Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFC 3/2 (fixed)] Refactor Git::config_*
From: Junio C Hamano @ 2011-10-18 19:52 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Eric Wong, Cord Seele, Matthieu Moy, git, Cord Seele
In-Reply-To: <201110181147.02397.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> From: Junio C Hamano <gitster@pobox.com>
>
> There is, especially with addition of Git::config_path(), much code
> repetition in the Git::config_* family of subroutines.
>
> Refactor common parts of Git::config(), Git::config_bool(),
> Git::config_int() and Git::config_path() into _config_common()
> helper method, reducing code duplication.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> Jakub Narebski wrote:
>>
>> I'll resend amended commit.
>
> Here it is.

Well, this breaks t9001 and I ended up spending an hour and half figuring
out why. Admittedly, I was doing something else on the side, so it is not
like the whole 90 minutes is the billable hour for reviewing this patch,
but as far as the Git project is concerned, I didn't have any other branch
checked out in my working tree, so that whole time was what ended up
costing.

The real problem was here.

> @@ -609,25 +592,10 @@ This currently wraps command('config') so it is not so fast.
>  
>  sub config_bool {
>  	my ($self, $var) = _maybe_self(@_);
> -
> -	try {
> -		my @cmd = ('config', '--bool', '--get', $var);
> -		unshift @cmd, $self if $self;
> -		my $val = command_oneline(@cmd);
> -		return undef unless defined $val;
> -		return $val eq 'true';
> ...
> -	};
> +	my $val = scalar _config_common($self, $var, {'kind' => '--bool'});
> +	return (defined $val && $val eq 'true');
>  }

Can you spot the difference?

This is the reason why I do not particularly like a rewrite for the sake
of rewriting.

^ permalink raw reply

* Re: [GUILT] handle branches with slashes in guilt-graph
From: Junio C Hamano @ 2011-10-18 19:15 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Per Cederqvist, Jeff Sipek, git, ceder
In-Reply-To: <7v39eqglle.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> Alternatively, you could change the delimiter to `,':
>>
>>   \,^$current refs/patches/$branch, {
>
> Isn't a comma still valid character in a branch name?
>
> The vertical var | is available, though ;-)

Nah, sorry, '|' is not.

^ permalink raw reply

* Re: [GUILT] handle branches with slashes in guilt-graph
From: Junio C Hamano @ 2011-10-18 19:12 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Per Cederqvist, Jeff Sipek, git, ceder
In-Reply-To: <m2r52adu65.fsf@igel.home>

Andreas Schwab <schwab@linux-m68k.org> writes:

> Per Cederqvist <cederp@opera.com> writes:
>
>> Avoid sed errors when the branch name contains a slash.
>>
>> Signed-off-by: Per Cederqvist <cederp@opera.com>
>>
>> --- /usr/bin/guilt-graph~	2011-01-25 20:15:50.000000000 +0100
>> +++ /usr/bin/guilt-graph	2011-10-18 12:30:31.000000000 +0200
>> @@ -37,9 +37,10 @@ disp "digraph G {"
>>
>>  current="$top"
>>
>> +safebranch=`echo "$branch"|sed 's%/%\\\\/%g'`
>>  while [ "$current" != "$base" ]; do
>>  	pname=`git show-ref | sed -n -e "
>> -/^$current refs\/patches\/$branch/ {
>> +/^$current refs\/patches\/$safebranch/ {
>
> Alternatively, you could change the delimiter to `,':
>
>   \,^$current refs/patches/$branch, {

Isn't a comma still valid character in a branch name?

The vertical var | is available, though ;-)

^ permalink raw reply

* Re: [GUILT] handle branches with slashes in guilt-graph
From: Andreas Schwab @ 2011-10-18 18:35 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: Jeff Sipek, git, ceder
In-Reply-To: <4E9D57BB.2030707@opera.com>

Per Cederqvist <cederp@opera.com> writes:

> Avoid sed errors when the branch name contains a slash.
>
> Signed-off-by: Per Cederqvist <cederp@opera.com>
>
> --- /usr/bin/guilt-graph~	2011-01-25 20:15:50.000000000 +0100
> +++ /usr/bin/guilt-graph	2011-10-18 12:30:31.000000000 +0200
> @@ -37,9 +37,10 @@ disp "digraph G {"
>
>  current="$top"
>
> +safebranch=`echo "$branch"|sed 's%/%\\\\/%g'`
>  while [ "$current" != "$base" ]; do
>  	pname=`git show-ref | sed -n -e "
> -/^$current refs\/patches\/$branch/ {
> +/^$current refs\/patches\/$safebranch/ {

Alternatively, you could change the delimiter to `,':

  \,^$current refs/patches/$branch, {

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* [PATCH] gitweb: Refactor diff body line classification
From: Jakub Narebski @ 2011-10-18 18:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kato Kazuyoshi, git
In-Reply-To: <7v62jnl3ef.fsf@alter.siamese.dyndns.org>

Simplify classification of diff line body in format_diff_line(),
replacing two if-elsif chains (one for ordinary diff and one for
combined diff of a merge commit) with regexp match.  Refactor this
code into diff_line_class() function.

While at it:

* Fix an artifact in that $diff_class included leading space to be
  able to compose classes like this "class=\"diff$diff_class\"', even
  when $diff_class was an empty string.  This made code unnecessary
  ugly: $diff_class is now just class name or an empty string.

* Introduce "ctx" class for context lines ($diff_class was set to ""
  in this case before this commit).

Idea and initial code by Junio C Hamano, polish and testing by Jakub
Narebski.  Inspired by patch adding side-by-side diff by Kato Kazuyoshi,
which required $diff_class to be name of class without extra space.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> Kato Kazuyoshi wrote:

>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl 
>>> index 85d64b2..095adda 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -2235,28 +2235,30 @@ sub format_diff_line {
>>>              # combined diff
>>>              my $prefix = substr($line, 0, scalar @{$from->{'href'}});
>>>              if ($line =~ m/^\@{3}/) {
>>> -                    $diff_class = " chunk_header";
>>> +                    $diff_class = "chunk_header";
>>>              } elsif ($line =~ m/^\\/) {
>>> -                    $diff_class = " incomplete";
>>> +                    $diff_class = "incomplete";
>>>              } elsif ($prefix =~ tr/+/+/) {
>>> -                    $diff_class = " add";
>>> +                    $diff_class = "add";
>>>              } elsif ($prefix =~ tr/-/-/) {
>>> -                    $diff_class = " rem";
>>> +                    $diff_class = "rem";
>>>              }
>>
>> Hmmm... that reminds me: this if-elsif chain is a bit ugly, but would
>> be hard to replace without given ... when, I think.
> 
> I wasn't reading the existing context line, but now that you mention it,
> they are not just ugly but are borderline of being incorrect (e.g. it does
> not catch a broken hunk-header that begins with "@@@@" for a two-way
> merge).
> 
> Assuming that $from->{'href'} has all the parents (i.e. 2 for two-way
> merge), shouldn't the code be more like this?
> 
>         # combined diff
>         my $num_sign = @{$from->{'href'}} + 1;
>         my @cc_classifier = (
>                 ["\@{$num_sign} ", "chunk_header"],
>                 ['\\', "incomplete"],
>                 [" {$num_sign}", ""],
>                 ["[+ ]{$num_sign}", "add"],
>                 ["[- ]{$num_sign}", "rem"],
>         );
>         for my $cls (@cc_classifier) {
>                 if ($line =~ /^$cls->[0]$/) {
>                         $diff_class = $cls->[1];
>                         last;
>                 }
>         }
> 
> Also don't we want to use "context" or something for the css class for the
> context lines, instead of assuming that we won't want to paint it in any
> special color?

How about this?

Kato Kazuyoshi, this should replace your

  [PATCH/RFC 1/2] gitweb: change format_diff_line() to remove leading SP from $diff_class

 gitweb/gitweb.perl |   67 ++++++++++++++++++++++++++++-----------------------
 1 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b0eaad7..b3284d4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2247,40 +2247,47 @@ sub format_diff_cc_simplified {
 	return $result;
 }
 
+sub diff_line_class {
+	my ($line, $from, $to) = @_;
+
+	# ordinary diff
+	my $num_sign = 1;
+	# combined diff
+	if ($from && $to && ref($from->{'href'}) eq "ARRAY") {
+		$num_sign = scalar @{$from->{'href'}};
+	}
+
+	my @diff_line_classifier = (
+		{ regexp => qr/^\@\@{$num_sign} /, class => "chunk_header"},
+		{ regexp => qr/^\\/,               class => "incomplete"  },
+		{ regexp => qr/^ {$num_sign}/,     class => "ctx" },
+		# classifier for context must come before classifier add/rem,
+		# or we would have to use more complicated regexp, for example
+		# qr/(?= {0,$m}\+)[+ ]{$num_sign}/, where $m = $num_sign - 1;
+		{ regexp => qr/^[+ ]{$num_sign}/,   class => "add" },
+		{ regexp => qr/^[- ]{$num_sign}/,   class => "rem" },
+	);
+	for my $clsfy (@diff_line_classifier) {
+		return $clsfy->{'class'}
+			if ($line =~ $clsfy->{'regexp'});
+	}
+
+	# fallback
+	return "";
+}
+
 # format patch (diff) line (not to be used for diff headers)
 sub format_diff_line {
 	my $line = shift;
 	my ($from, $to) = @_;
-	my $diff_class = "";
 
-	chomp $line;
+	my $diff_class = diff_line_class($line, $from, $to);
+	my $diff_classes = "diff";
+	$diff_classes .= " $diff_class" if ($diff_class);
 
-	if ($from && $to && ref($from->{'href'}) eq "ARRAY") {
-		# combined diff
-		my $prefix = substr($line, 0, scalar @{$from->{'href'}});
-		if ($line =~ m/^\@{3}/) {
-			$diff_class = " chunk_header";
-		} elsif ($line =~ m/^\\/) {
-			$diff_class = " incomplete";
-		} elsif ($prefix =~ tr/+/+/) {
-			$diff_class = " add";
-		} elsif ($prefix =~ tr/-/-/) {
-			$diff_class = " rem";
-		}
-	} else {
-		# assume ordinary diff
-		my $char = substr($line, 0, 1);
-		if ($char eq '+') {
-			$diff_class = " add";
-		} elsif ($char eq '-') {
-			$diff_class = " rem";
-		} elsif ($char eq '@') {
-			$diff_class = " chunk_header";
-		} elsif ($char eq "\\") {
-			$diff_class = " incomplete";
-		}
-	}
+	chomp $line;
 	$line = untabify($line);
+
 	if ($from && $to && $line =~ m/^\@{2} /) {
 		my ($from_text, $from_start, $from_lines, $to_text, $to_start, $to_lines, $section) =
 			$line =~ m/^\@{2} (-(\d+)(?:,(\d+))?) (\+(\d+)(?:,(\d+))?) \@{2}(.*)$/;
@@ -2298,7 +2305,7 @@ sub format_diff_line {
 		}
 		$line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
 		        "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
-		return "<div class=\"diff$diff_class\">$line</div>\n";
+		return "<div class=\"$diff_classes\">$line</div>\n";
 	} elsif ($from && $to && $line =~ m/^\@{3}/) {
 		my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
 		my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
@@ -2331,9 +2338,9 @@ sub format_diff_line {
 		}
 		$line .= " $prefix</span>" .
 		         "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
-		return "<div class=\"diff$diff_class\">$line</div>\n";
+		return "<div class=\"$diff_classes\">$line</div>\n";
 	}
-	return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1) . "</div>\n";
+	return "<div class=\"$diff_classes\">" . esc_html($line, -nbsp=>1) . "</div>\n";
 }
 
 # Generates undef or something like "_snapshot_" or "snapshot (_tbz2_ _zip_)",
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH] inet_ntop.c: Work around GCC 4.6's detection of uninitialized variables
From: Sebastian Schuberth @ 2011-10-18 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, mschub
In-Reply-To: <7vehyagq82.fsf@alter.siamese.dyndns.org>

GCC 4.6 claims that

    error: 'best.len' may be used uninitialized in this function

so silence that warning which is treated as an error by also initializing
the "len" members of the struct.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 compat/inet_ntop.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index ea249c6..9e9f6fb 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -98,7 +98,8 @@ inet_ntop6(const u_char *src, char *dst, size_t size)
 	for (i = 0; i < NS_IN6ADDRSZ; i++)
 		words[i / 2] |= (src[i] << ((1 - (i % 2)) << 3));
 	best.base = -1;
-	cur.base = -1;
+	best.len = 0;
+	cur = best;
 	for (i = 0; i < (NS_IN6ADDRSZ / NS_INT16SZ); i++) {
 		if (words[i] == 0) {
 			if (cur.base == -1)
-- 
1.7.7.msysgit.1

^ permalink raw reply related

* Re: What's cooking in git.git (Oct 2011, #06; Tue, 18)
From: Junio C Hamano @ 2011-10-18 18:09 UTC (permalink / raw)
  To: Phil Hord; +Cc: git
In-Reply-To: <7vmxcygs1m.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Phil Hord <phil.hord@gmail.com> writes:
>
>> On Tue, Oct 18, 2011 at 3:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> What's cooking in git.git (Oct 2011, #06; Tue, 18)
>>> --------------------------------------------------
>> [...]
>>> * ph/transport-with-gitfile (2011-10-11) 5 commits
>>>  (merged to 'next' on 2011-10-12 at 6d58417)
>>>  + Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
>>>  (merged to 'next' on 2011-10-06 at 891b8b6)
>>>  + Add test showing git-fetch groks gitfiles
>>>  + Teach transport about the gitfile mechanism
>>>  + Learn to handle gitfiles in enter_repo
>>>  + enter_repo: do not modify input
>>>
>>> Will merge to 'master' in the fifth wave.
>>
>> Do you want a re-roll of this with your is_bundle() changes added in?
>> I do like them better.
>
> Hmm, you may be right. I'll try to queue an update on top of the series
> and see what happens.

Actually there is nothing that needs re-rolling. We can just make the
jc/unseekable-bundle topic graduate at the same time as this one, and let
the merge remove the is_gitfile() helper that becomes unnecessary.

^ permalink raw reply

* Re: git-p4.skipSubmitEdit
From: Luke Diamand @ 2011-10-18 17:53 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: L. A. Linden Levy, git
In-Reply-To: <20111018004500.GA31768@arf.padd.com>

On 18/10/11 01:45, Pete Wyckoff wrote:
> alevy@mobitv.com wrote on Mon, 12 Sep 2011 10:12 -0700:
>> I agree with almost all your points. I have answered them each in-line
>> below.
>>
>> On Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:
>>> On 08/09/11 21:40, L. A. Linden Levy wrote:
>>>> Hi All,
>>>>
>>>> I have been using git-p4 for a while and it has allowed me to completely
>>>> change the way I develop and still be able to use perforce which my
>>>> company has for its main VCS. One thing that was driving me nuts was
>>>> that "git p4 submit" cycles through all of my individual commits and
>>>> asks me if I want to change them. The way I develop I often am checking
>>>> in 20 to 50 different small commits each with a descriptive git comment.
>>>> I felt like I was doing double duty by having emacs open on every commit
>>>> into perforce. So I modified git-p4 to have an option to skip the
>>>> editor. This option coupled with git-p4.skipSubmitEditCheck will make
>>>> the submission non-interactive for "git p4 submit".
>>>
>>>
>>> Sorry - I've not had a chance to look at this before now. But a couple
>>> of comments:
>>>
>>>    - Is there a line wrap problem in the patch? It doesn't seem to want
>>> to apply for me.
>>
>> Probably. Below are the results from following the patch submission
>> instructions.
>
> Sorry I sat on this for a month.  It is a good idea.  Your
> patches were good in content, but they didn't apply well due to
> being line-wrapped and having one duplicate.
>
> Reading the code, though, I decided that the whole
> skipSubmitEdit* checking was a bit convoluted even before you got
> to it.  So I moved it all to a separate function.  And added a
> unit test.
>
> Tell me if you think this is okay instead.  If I got a bit too
> wordy in the doc, please help with that too.

Looks good, one minor nit (see below) and a comment.

Ack.

Luke


>
> 		-- Pete
>
> --- 8<  ---
>
> Subject: [PATCH] git-p4: introduce skipSubmitEdit
>
> Add a configuration variable to skip invoking the editor in the
> submit path.
>
> The existing variable skipSubmitEditCheck continues to make sure
> that the submit template was indeed modified by the editor; but,
> it is not considered is skipSubmitEdit is true.
>
> Reported-by: Loren A. Linden Levy<lindenle@gmail.com>
> Signed-off-by: Pete Wyckoff<pw@padd.com>
> ---
>   contrib/fast-import/git-p4     |   60 +++++++++++++++++++----------
>   contrib/fast-import/git-p4.txt |   19 ++++++++-
>   t/t9804-skip-submit-edit.sh    |   82 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 137 insertions(+), 24 deletions(-)
>   create mode 100755 t/t9804-skip-submit-edit.sh
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index f885d70..abd6778 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -847,6 +847,39 @@ class P4Submit(Command, P4UserMap):
>
>           return template
>
> +    def edit_template(self, template_file):
> +        """Invoke the editor to let the user change the submission
> +           message.  Return true if okay to continue with the submit."""
> +
> +        # if configured to skip the editing part, just submit
> +        if gitConfig("git-p4.skipSubmitEdit") == "true":
> +            return True
> +
> +        # look at the modification time, to check later if the user saved
> +        # the file
> +        mtime = os.stat(template_file).st_mtime
> +
> +        # invoke the editor
> +        if os.environ.has_key("P4EDITOR"):
> +            editor = os.environ.get("P4EDITOR")
> +        else:
> +            editor = read_pipe("git var GIT_EDITOR").strip()
> +        system(editor + " " + template_file)

This is where we should really check the return code. However, doing so 
seems to break lots of the existing tests so it's not as easy as it looks.

> +
> +        # If the file was not saved, prompt to see if this patch should
> +        # be skipped.  But skip this verification step if configured so.
> +        if gitConfig("git-p4.skipSubmitEditCheck") == "true":
> +            print "return true for skipSubmitEditCheck"

You print a helpful/annoying(?) message here, but not further up at 
skipSubmitEdit?


> +            return True
> +
> +        if os.stat(template_file).st_mtime<= mtime:
> +            while True:
> +                response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
> +                if response == 'y':
> +                    return True
> +                if response == 'n':
> +                    return False
> +
>       def applyCommit(self, id):
>           print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
>
> @@ -1001,7 +1034,7 @@ class P4Submit(Command, P4UserMap):
>
>               separatorLine = "######## everything below this line is just the diff #######\n"
>
> -            [handle, fileName] = tempfile.mkstemp()
> +            (handle, fileName) = tempfile.mkstemp()
>               tmpFile = os.fdopen(handle, "w+")
>               if self.isWindows:
>                   submitTemplate = submitTemplate.replace("\n", "\r\n")
> @@ -1009,25 +1042,9 @@ class P4Submit(Command, P4UserMap):
>                   newdiff = newdiff.replace("\n", "\r\n")
>               tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
>               tmpFile.close()
> -            mtime = os.stat(fileName).st_mtime
> -            if os.environ.has_key("P4EDITOR"):
> -                editor = os.environ.get("P4EDITOR")
> -            else:
> -                editor = read_pipe("git var GIT_EDITOR").strip()
> -            system(editor + " " + fileName)
>
> -            if gitConfig("git-p4.skipSubmitEditCheck") == "true":
> -                checkModTime = False
> -            else:
> -                checkModTime = True
> -
> -            response = "y"
> -            if checkModTime and (os.stat(fileName).st_mtime<= mtime):
> -                response = "x"
> -                while response != "y" and response != "n":
> -                    response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
> -
> -            if response == "y":
> +            if self.edit_template(fileName):
> +                # read the edited message and submit
>                   tmpFile = open(fileName, "rb")
>                   message = tmpFile.read()
>                   tmpFile.close()
> @@ -1039,11 +1056,12 @@ class P4Submit(Command, P4UserMap):
>                   if self.preserveUser:
>                       if p4User:
>                           # Get last changelist number. Cannot easily get it from
> -                        # the submit command output as the output is unmarshalled.
> +                        # the submit command output as the output is
> +                        # unmarshalled.
>                           changelist = self.lastP4Changelist()
>                           self.modifyChangelistUser(changelist, p4User)
> -
>               else:
> +                # skip this patch
>                   for f in editedFiles:
>                       p4_revert(f)
>                   for f in filesToAdd:
> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
> index 52003ae..5044a12 100644
> --- a/contrib/fast-import/git-p4.txt
> +++ b/contrib/fast-import/git-p4.txt
> @@ -202,11 +202,24 @@ able to find the relevant client.  This client spec will be used to
>   both filter the files cloned by git and set the directory layout as
>   specified in the client (this implies --keep-path style semantics).
>
> -git-p4.skipSubmitModTimeCheck
> +git-p4.skipSubmitEdit
>
> -  git config [--global] git-p4.skipSubmitModTimeCheck false
> +  git config [--global] git-p4.skipSubmitEdit false
>
> -If true, submit will not check if the p4 change template has been modified.
> +Normally, git-p4 invokes an editor after each commit is applied so
> +that you can make changes to the submit message.  Setting this
> +variable to true will skip the editing step, submitting the change as is.
> +
> +git-p4.skipSubmitEditCheck
> +
> +  git config [--global] git-p4.skipSubmitEditCheck false
> +
> +After the editor is invoked, git-p4 normally makes sure you saved the
> +change description, as an indication that you did indeed read it over
> +and edit it.  You can quit without saving to abort the submit (or skip
> +this change and continue).  Setting this variable to true will cause
> +git-p4 not to check if you saved the change description.  This variable
> +only matters if git-p4.skipSubmitEdit has not been set to true.
>
>   git-p4.preserveUser
>
> diff --git a/t/t9804-skip-submit-edit.sh b/t/t9804-skip-submit-edit.sh
> new file mode 100755
> index 0000000..734ccf2
> --- /dev/null
> +++ b/t/t9804-skip-submit-edit.sh
> @@ -0,0 +1,82 @@
> +#!/bin/sh
> +
> +test_description='git-p4 skipSubmitEdit config variables'
> +
> +. ./lib-git-p4.sh
> +
> +test_expect_success 'start p4d' '
> +	start_p4d
> +'
> +
> +test_expect_success 'init depot' '
> +	(
> +		cd "$cli"&&
> +		echo file1>file1&&
> +		p4 add file1&&
> +		p4 submit -d "change 1"
> +	)
> +'
> +
> +# this works because EDITOR is set to :
> +test_expect_success 'no config, unedited, say yes' '
> +	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
> +	(
> +		cd "$git"&&
> +		echo line>>file1&&
> +		git commit -a -m "change 2"&&
> +		echo y | "$GITP4" submit&&
> +		p4 changes //depot/...>wc&&
> +		test_line_count = 2 wc
> +	)
> +'
> +
> +test_expect_success 'no config, unedited, say no' '
> +	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
> +	(
> +		cd "$git"&&
> +		echo line>>file1&&
> +		git commit -a -m "change 3 (not really)"&&
> +		printf "bad response\nn\n" | "$GITP4" submit
> +		p4 changes //depot/...>wc&&
> +		test_line_count = 2 wc
> +	)
> +'
> +
> +test_expect_success 'skipSubmitEdit' '
> +	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
> +	(
> +		cd "$git"&&
> +		git config git-p4.skipSubmitEdit true&&
> +		# will fail if editor is even invoked
> +		git config core.editor /bin/false&&
> +		echo line>>file1&&
> +		git commit -a -m "change 3"&&
> +		"$GITP4" submit&&
> +		p4 changes //depot/...>wc&&
> +		test_line_count = 3 wc
> +	)
> +'
> +
> +test_expect_success 'skipSubmitEditCheck' '
> +	"$GITP4" clone --dest="$git" //depot&&
> +	test_when_finished cleanup_git&&
> +	(
> +		cd "$git"&&
> +		git config git-p4.skipSubmitEditCheck true&&
> +		echo line>>file1&&
> +		git commit -a -m "change 4"&&
> +		"$GITP4" submit&&
> +		p4 changes //depot/...>wc&&
> +		test_line_count = 4 wc
> +	)
> +'
> +
> +
> +test_expect_success 'kill p4d' '
> +	kill_p4d
> +'
> +
> +test_done

^ permalink raw reply

* Re: git-p4.skipSubmitEdit
From: Pete Wyckoff @ 2011-10-18 17:35 UTC (permalink / raw)
  To: L. A. Linden Levy; +Cc: Luke Diamand, git
In-Reply-To: <1318956684.2717.7.camel@uncle-pecos>

alevy@mobitv.com wrote on Tue, 18 Oct 2011 09:51 -0700:
> This is just fine for me. Like I said initially I did not approach the
> problem from an engineering perspective, I just changed it to prevent
> the patch questions so I could commit faster. Looking forward to seeing
> the update in the repo.

Okay, thanks.  I'll see if I get some review comments, and will
gradually shepherd this toward the next release.

		-- Pete

^ permalink raw reply

* Re: [PATCH] inet_ntop.c: Work around GCC 4.6's detection of uninitialized variables
From: Junio C Hamano @ 2011-10-18 17:32 UTC (permalink / raw)
  To: Sebastian Schuberth; +Cc: git, mschub
In-Reply-To: <4E9DA88E.40500@gmail.com>

Sebastian Schuberth <sschuberth@gmail.com> writes:

> GCC 4.6 claims that
>
>     error: 'best.len' may be used uninitialized in this function
>
> so silence that warning which is treated as an error by also initializing
> the "len" members of the struct.
>
> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
>  compat/inet_ntop.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
> index ea249c6..60b5a1d 100644
> --- a/compat/inet_ntop.c
> +++ b/compat/inet_ntop.c
> @@ -98,7 +98,9 @@ inet_ntop6(const u_char *src, char *dst, size_t size)
>  	for (i = 0; i < NS_IN6ADDRSZ; i++)
>  		words[i / 2] |= (src[i] << ((1 - (i % 2)) << 3));
>  	best.base = -1;
> +	best.len = 0;
>  	cur.base = -1;
> +	cur.len = 0;

It may be just the "taste" thing, but I wonder if

	best.base = -1;
        best.len = 0;
        cur = best;

might be easier on the eyes.

Will queue as-is anyway. Thanks.

>  	for (i = 0; i < (NS_IN6ADDRSZ / NS_INT16SZ); i++) {
>  		if (words[i] == 0) {
>  			if (cur.base == -1)

^ permalink raw reply

* Re: [PATCH 3/3] completion: match ctags symbol names in grep patterns
From: Junio C Hamano @ 2011-10-18 17:14 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jeff King, git
In-Reply-To: <20111018075521.GB20976@elie.hsd1.il.comcast.net>

Jonathan Nieder <jrnieder@gmail.com> writes:

> In this new tags/cscope example, one could make an argument that
> running exactly once is similarly better than running as needed (as in
> (ii) above), by pointing out that
>
> 	make tags TAGS cscope
>
> would have to check for a working "git ls-files" once instead of three
> times.

I tend to agree that once instead of three times is not such an
improvement, especially given how cheap builtin-invocation of ls-files is,
but then once every invocation of $(MAKE) would also be negligible (it is
not once every internal target evaluation).

In any case, here is what I'll queue. I like the fact that your suggestion
avoids an extra "test invocation" that spends cycles to read the full
index without contributing to the end result.

By the way, did anybody know that "git ls-files >/dev/null" is more
expensive than "git ls-files '~/' >/dev/null" as a way to see if there is
ls-files command available?

-- >8 --
Subject: [PATCH] Makefile: ask "ls-files" to list source files if available

The [ce]tags and cscope targets used to run "find" looking for any paths
that match '*.[chS]' to feed the list of source files to downstream xargs.

Use "git ls-files" if it is already available to us, and otherwise use a
tighter "find" expression that does not list directories and does not go
into our .git directory.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 8d6d451..badfb77 100644
--- a/Makefile
+++ b/Makefile
@@ -2115,17 +2115,21 @@ po/git.pot: $(LOCALIZED_C)
 
 pot: po/git.pot
 
+FIND_SOURCE_FILES = ( git ls-files '*.[hcS]' 2>/dev/null || \
+			$(FIND) . \( -name .git -type d -prune \) \
+				-o \( -name '*.[hcS]' -type f -print \) )
+
 $(ETAGS_TARGET): FORCE
 	$(RM) $(ETAGS_TARGET)
-	$(FIND) . -name '*.[hcS]' -print | xargs etags -a -o $(ETAGS_TARGET)
+	$(FIND_SOURCE_FILES) | xargs etags -a -o $(ETAGS_TARGET)
 
 tags: FORCE
 	$(RM) tags
-	$(FIND) . -name '*.[hcS]' -print | xargs ctags -a
+	$(FIND_SOURCE_FILES) | xargs ctags -a
 
 cscope:
 	$(RM) cscope*
-	$(FIND) . -name '*.[hcS]' -print | xargs cscope -b
+	$(FIND_SOURCE_FILES) | xargs cscope -b
 
 ### Detect prefix changes
 TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):\
-- 
1.7.7.388.g3a4b7

^ permalink raw reply related

* Re: git-p4.skipSubmitEdit
From: L. A. Linden Levy @ 2011-10-18 16:51 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: Luke Diamand, git@vger.kernel.org
In-Reply-To: <20111018004500.GA31768@arf.padd.com>

[-- Attachment #1: Type: text/plain, Size: 11341 bytes --]

Hi Peter,

This is just fine for me. Like I said initially I did not approach the
problem from an engineering perspective, I just changed it to prevent
the patch questions so I could commit faster. Looking forward to seeing
the update in the repo.

Cheers,
Alex

On Mon, 2011-10-17 at 20:45 -0400, Pete Wyckoff wrote:
> alevy@mobitv.com wrote on Mon, 12 Sep 2011 10:12 -0700:
> > I agree with almost all your points. I have answered them each in-line
> > below.
> > 
> > On Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:
> > > On 08/09/11 21:40, L. A. Linden Levy wrote:
> > > > Hi All,
> > > >
> > > > I have been using git-p4 for a while and it has allowed me to completely
> > > > change the way I develop and still be able to use perforce which my
> > > > company has for its main VCS. One thing that was driving me nuts was
> > > > that "git p4 submit" cycles through all of my individual commits and
> > > > asks me if I want to change them. The way I develop I often am checking
> > > > in 20 to 50 different small commits each with a descriptive git comment.
> > > > I felt like I was doing double duty by having emacs open on every commit
> > > > into perforce. So I modified git-p4 to have an option to skip the
> > > > editor. This option coupled with git-p4.skipSubmitEditCheck will make
> > > > the submission non-interactive for "git p4 submit".
> > > 
> > > 
> > > Sorry - I've not had a chance to look at this before now. But a couple 
> > > of comments:
> > > 
> > >   - Is there a line wrap problem in the patch? It doesn't seem to want 
> > > to apply for me.
> > 
> > Probably. Below are the results from following the patch submission
> > instructions. 
> 
> Sorry I sat on this for a month.  It is a good idea.  Your
> patches were good in content, but they didn't apply well due to
> being line-wrapped and having one duplicate.
> 
> Reading the code, though, I decided that the whole
> skipSubmitEdit* checking was a bit convoluted even before you got
> to it.  So I moved it all to a separate function.  And added a
> unit test.
> 
> Tell me if you think this is okay instead.  If I got a bit too
> wordy in the doc, please help with that too.
> 
> 		-- Pete
> 
> --- 8< ---
> 
> Subject: [PATCH] git-p4: introduce skipSubmitEdit
> 
> Add a configuration variable to skip invoking the editor in the
> submit path.
> 
> The existing variable skipSubmitEditCheck continues to make sure
> that the submit template was indeed modified by the editor; but,
> it is not considered is skipSubmitEdit is true.
> 
> Reported-by: Loren A. Linden Levy <lindenle@gmail.com>
> Signed-off-by: Pete Wyckoff <pw@padd.com>
> ---
>  contrib/fast-import/git-p4     |   60 +++++++++++++++++++----------
>  contrib/fast-import/git-p4.txt |   19 ++++++++-
>  t/t9804-skip-submit-edit.sh    |   82 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 137 insertions(+), 24 deletions(-)
>  create mode 100755 t/t9804-skip-submit-edit.sh
> 
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index f885d70..abd6778 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -847,6 +847,39 @@ class P4Submit(Command, P4UserMap):
>  
>          return template
>  
> +    def edit_template(self, template_file):
> +        """Invoke the editor to let the user change the submission
> +           message.  Return true if okay to continue with the submit."""
> +
> +        # if configured to skip the editing part, just submit
> +        if gitConfig("git-p4.skipSubmitEdit") == "true":
> +            return True
> +
> +        # look at the modification time, to check later if the user saved
> +        # the file
> +        mtime = os.stat(template_file).st_mtime
> +
> +        # invoke the editor
> +        if os.environ.has_key("P4EDITOR"):
> +            editor = os.environ.get("P4EDITOR")
> +        else:
> +            editor = read_pipe("git var GIT_EDITOR").strip()
> +        system(editor + " " + template_file)
> +
> +        # If the file was not saved, prompt to see if this patch should
> +        # be skipped.  But skip this verification step if configured so.
> +        if gitConfig("git-p4.skipSubmitEditCheck") == "true":
> +            print "return true for skipSubmitEditCheck"
> +            return True
> +
> +        if os.stat(template_file).st_mtime <= mtime:
> +            while True:
> +                response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
> +                if response == 'y':
> +                    return True
> +                if response == 'n':
> +                    return False
> +
>      def applyCommit(self, id):
>          print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
>  
> @@ -1001,7 +1034,7 @@ class P4Submit(Command, P4UserMap):
>  
>              separatorLine = "######## everything below this line is just the diff #######\n"
>  
> -            [handle, fileName] = tempfile.mkstemp()
> +            (handle, fileName) = tempfile.mkstemp()
>              tmpFile = os.fdopen(handle, "w+")
>              if self.isWindows:
>                  submitTemplate = submitTemplate.replace("\n", "\r\n")
> @@ -1009,25 +1042,9 @@ class P4Submit(Command, P4UserMap):
>                  newdiff = newdiff.replace("\n", "\r\n")
>              tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
>              tmpFile.close()
> -            mtime = os.stat(fileName).st_mtime
> -            if os.environ.has_key("P4EDITOR"):
> -                editor = os.environ.get("P4EDITOR")
> -            else:
> -                editor = read_pipe("git var GIT_EDITOR").strip()
> -            system(editor + " " + fileName)
>  
> -            if gitConfig("git-p4.skipSubmitEditCheck") == "true":
> -                checkModTime = False
> -            else:
> -                checkModTime = True
> -
> -            response = "y"
> -            if checkModTime and (os.stat(fileName).st_mtime <= mtime):
> -                response = "x"
> -                while response != "y" and response != "n":
> -                    response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
> -
> -            if response == "y":
> +            if self.edit_template(fileName):
> +                # read the edited message and submit
>                  tmpFile = open(fileName, "rb")
>                  message = tmpFile.read()
>                  tmpFile.close()
> @@ -1039,11 +1056,12 @@ class P4Submit(Command, P4UserMap):
>                  if self.preserveUser:
>                      if p4User:
>                          # Get last changelist number. Cannot easily get it from
> -                        # the submit command output as the output is unmarshalled.
> +                        # the submit command output as the output is
> +                        # unmarshalled.
>                          changelist = self.lastP4Changelist()
>                          self.modifyChangelistUser(changelist, p4User)
> -
>              else:
> +                # skip this patch
>                  for f in editedFiles:
>                      p4_revert(f)
>                  for f in filesToAdd:
> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
> index 52003ae..5044a12 100644
> --- a/contrib/fast-import/git-p4.txt
> +++ b/contrib/fast-import/git-p4.txt
> @@ -202,11 +202,24 @@ able to find the relevant client.  This client spec will be used to
>  both filter the files cloned by git and set the directory layout as
>  specified in the client (this implies --keep-path style semantics).
>  
> -git-p4.skipSubmitModTimeCheck
> +git-p4.skipSubmitEdit
>  
> -  git config [--global] git-p4.skipSubmitModTimeCheck false
> +  git config [--global] git-p4.skipSubmitEdit false
>  
> -If true, submit will not check if the p4 change template has been modified.
> +Normally, git-p4 invokes an editor after each commit is applied so
> +that you can make changes to the submit message.  Setting this
> +variable to true will skip the editing step, submitting the change as is.
> +
> +git-p4.skipSubmitEditCheck
> +
> +  git config [--global] git-p4.skipSubmitEditCheck false
> +
> +After the editor is invoked, git-p4 normally makes sure you saved the
> +change description, as an indication that you did indeed read it over
> +and edit it.  You can quit without saving to abort the submit (or skip
> +this change and continue).  Setting this variable to true will cause
> +git-p4 not to check if you saved the change description.  This variable
> +only matters if git-p4.skipSubmitEdit has not been set to true.
>  
>  git-p4.preserveUser
>  
> diff --git a/t/t9804-skip-submit-edit.sh b/t/t9804-skip-submit-edit.sh
> new file mode 100755
> index 0000000..734ccf2
> --- /dev/null
> +++ b/t/t9804-skip-submit-edit.sh
> @@ -0,0 +1,82 @@
> +#!/bin/sh
> +
> +test_description='git-p4 skipSubmitEdit config variables'
> +
> +. ./lib-git-p4.sh
> +
> +test_expect_success 'start p4d' '
> +	start_p4d
> +'
> +
> +test_expect_success 'init depot' '
> +	(
> +		cd "$cli" &&
> +		echo file1 >file1 &&
> +		p4 add file1 &&
> +		p4 submit -d "change 1"
> +	)
> +'
> +
> +# this works because EDITOR is set to :
> +test_expect_success 'no config, unedited, say yes' '
> +	"$GITP4" clone --dest="$git" //depot &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		echo line >>file1 &&
> +		git commit -a -m "change 2" &&
> +		echo y | "$GITP4" submit &&
> +		p4 changes //depot/... >wc &&
> +		test_line_count = 2 wc
> +	)
> +'
> +
> +test_expect_success 'no config, unedited, say no' '
> +	"$GITP4" clone --dest="$git" //depot &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		echo line >>file1 &&
> +		git commit -a -m "change 3 (not really)" &&
> +		printf "bad response\nn\n" | "$GITP4" submit
> +		p4 changes //depot/... >wc &&
> +		test_line_count = 2 wc
> +	)
> +'
> +
> +test_expect_success 'skipSubmitEdit' '
> +	"$GITP4" clone --dest="$git" //depot &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		git config git-p4.skipSubmitEdit true &&
> +		# will fail if editor is even invoked
> +		git config core.editor /bin/false &&
> +		echo line >>file1 &&
> +		git commit -a -m "change 3" &&
> +		"$GITP4" submit &&
> +		p4 changes //depot/... >wc &&
> +		test_line_count = 3 wc
> +	)
> +'
> +
> +test_expect_success 'skipSubmitEditCheck' '
> +	"$GITP4" clone --dest="$git" //depot &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		git config git-p4.skipSubmitEditCheck true &&
> +		echo line >>file1 &&
> +		git commit -a -m "change 4" &&
> +		"$GITP4" submit &&
> +		p4 changes //depot/... >wc &&
> +		test_line_count = 4 wc
> +	)
> +'
> +
> +
> +test_expect_success 'kill p4d' '
> +	kill_p4d
> +'
> +
> +test_done

-- 
Alex Linden Levy
Senior Software Engineer
MobiTV, Inc.
6425 Christie Avenue, 5th Floor, Emeryville, CA 94608
phone 510.450.5190 mobile 720.352.8394
email alevy@mobitv.com  web www.mobitv.com


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2011, #06; Tue, 18)
From: Junio C Hamano @ 2011-10-18 16:53 UTC (permalink / raw)
  To: Phil Hord; +Cc: git
In-Reply-To: <CABURp0po3C9-a4_cGm8gq71=gq2ELzHWBK0y7H=OEcY1=DUdsw@mail.gmail.com>

Phil Hord <phil.hord@gmail.com> writes:

> On Tue, Oct 18, 2011 at 3:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> What's cooking in git.git (Oct 2011, #06; Tue, 18)
>> --------------------------------------------------
> [...]
>> * ph/transport-with-gitfile (2011-10-11) 5 commits
>>  (merged to 'next' on 2011-10-12 at 6d58417)
>>  + Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
>>  (merged to 'next' on 2011-10-06 at 891b8b6)
>>  + Add test showing git-fetch groks gitfiles
>>  + Teach transport about the gitfile mechanism
>>  + Learn to handle gitfiles in enter_repo
>>  + enter_repo: do not modify input
>>
>> Will merge to 'master' in the fifth wave.
>
> Do you want a re-roll of this with your is_bundle() changes added in?
> I do like them better.

Hmm, you may be right. I'll try to queue an update on top of the series
and see what happens.

Thanks.

^ permalink raw reply

* [PATCH] inet_ntop.c: Work around GCC 4.6's detection of uninitialized variables
From: Sebastian Schuberth @ 2011-10-18 16:25 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, mschub

GCC 4.6 claims that

    error: 'best.len' may be used uninitialized in this function

so silence that warning which is treated as an error by also initializing
the "len" members of the struct.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 compat/inet_ntop.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index ea249c6..60b5a1d 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -98,7 +98,9 @@ inet_ntop6(const u_char *src, char *dst, size_t size)
 	for (i = 0; i < NS_IN6ADDRSZ; i++)
 		words[i / 2] |= (src[i] << ((1 - (i % 2)) << 3));
 	best.base = -1;
+	best.len = 0;
 	cur.base = -1;
+	cur.len = 0;
 	for (i = 0; i < (NS_IN6ADDRSZ / NS_INT16SZ); i++) {
 		if (words[i] == 0) {
 			if (cur.base == -1)
-- 
1.7.7.msysgit.1

^ permalink raw reply related

* Re: Compiling on Windows
From: Sebastian Schuberth @ 2011-10-18 16:07 UTC (permalink / raw)
  To: kusmabite; +Cc: Andrew Ardill, Frans Klaver, git
In-Reply-To: <CABPQNSZhWOa5wken4vh6Hcza8EM4VnekE3VUFJNmaEJvWME=ew@mail.gmail.com>

On 18.10.2011 16:02, Erik Faye-Lund wrote:

>> there. Foolishly, I had glossed over the msysgit installers on the
>> project home (I think I thought they were Git for Windows installers).
> 
> Yeah, the installer sets up a fully self-contained MSYS-based
> development environment to hack on Git for Windows.

I keep advertizing my mingwGitDevEnv [1] project which aims to become an alternative to the current msysGit net installer [2]. The advantage of mingwGitDevEnv will be that it comes with an mingw-get based environment, i.e. you can very easily update / add new MinGW / MSYS tools directly from upstream.

Just today I reached a state were mingwGitDevEnv can successfully build git.exe, but git-gui etc. are still missing due to a lack of mingw-get compatible Tcl/Tk packages.

[1] https://github.com/sschuberth/mingwGitDevEnv
[2] http://msysgit.googlecode.com/files/msysGit-netinstall-1.7.7-preview20111014.exe

-- 
Sebastian Schuberth

^ permalink raw reply

* Re: [PATCH 3/3] completion: match ctags symbol names in grep patterns
From: Jeff King @ 2011-10-18 15:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd3duixdg.fsf@alter.siamese.dyndns.org>

On Tue, Oct 18, 2011 at 12:15:23AM -0700, Junio C Hamano wrote:

> > It's debatable whether this belongs in the generic completion code, as
> > it really only works if your project uses ctags. But I find it to be a
> > huge timesaver for finding callsites of functions, especially when
> > coupled with "git jump grep" from the previous patch.
> 
> Could you elaborate a bit more on how this would help for finding
> callsites? You are looking at a function and do not want to break its
> callers, so at that point presumably you already know the name of the
> function, no?
> 
> Ahh, Ok, you do not necessarily want to type the long function name.

Exactly. Actually, it is often not so much "do not want to type" as
"cannot remember the exact name", but the effect is the same. :)

I use the same completion for "vim -t" which will jump to the
definition.

> By the way, I notice that "make tags" runs "find ." looking for any files
> and directories that match "*.[hcS]" (so do $(ETAGS_TARGET) and cscope),
> without even excluding .git metadirectory.
> 
> Perhaps something like this is in order?
> [...]
> +FIND_SOURCE_FILES = git ls-files '*.[hcS]'

Makes sense to me. I doubt it matters much in practice, though, as we
don't tend to have random untracked source files lying around.

My version of ctags will actually do the recursion itself with "ctags
-R", picking out any files with with languages it knows about. But maybe
not all versions do that.

Also, I have often found myself trying to do completion on shell
functions. I wonder if it's worth adding them to the list (again, my
version of ctags understands bourne shell just fine, but I don't know if
all do).

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2011, #06; Tue, 18)
From: Phil Hord @ 2011-10-18 14:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzkgyhh6n.fsf@alter.siamese.dyndns.org>

On Tue, Oct 18, 2011 at 3:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> What's cooking in git.git (Oct 2011, #06; Tue, 18)
> --------------------------------------------------
[...]
> * ph/transport-with-gitfile (2011-10-11) 5 commits
>  (merged to 'next' on 2011-10-12 at 6d58417)
>  + Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
>  (merged to 'next' on 2011-10-06 at 891b8b6)
>  + Add test showing git-fetch groks gitfiles
>  + Teach transport about the gitfile mechanism
>  + Learn to handle gitfiles in enter_repo
>  + enter_repo: do not modify input
>
> Will merge to 'master' in the fifth wave.

Do you want a re-roll of this with your is_bundle() changes added in?
I do like them better.

Phil

^ permalink raw reply

* Re: Compiling on Windows
From: Erik Faye-Lund @ 2011-10-18 14:02 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: Frans Klaver, git
In-Reply-To: <CAH5451=VPzkFZyyUUdj+=dXDCHKQbWTTob_=JJFBCwaDsp1n7Q@mail.gmail.com>

On Tue, Oct 18, 2011 at 8:17 AM, Andrew Ardill <andrew.ardill@gmail.com> wrote:
> Thanks for the replies all - I think my main issue was that the wiki
> is down and msysgit has very little use-able documentation otherwise.
> I had cloned the msysgit project, but was lost on what to do from
> there. Foolishly, I had glossed over the msysgit installers on the
> project home (I think I thought they were Git for Windows installers).
>

Yeah, the installer sets up a fully self-contained MSYS-based
development environment to hack on Git for Windows.

> I have now installed everything, and am ready to hack. Perhaps the
> first port of call is adding a README to msysgit :D

Not at all a bad idea!

It'd be awesome if it even worked with the GitHub markup-stuff as
well, since we moved the project there :)

^ permalink raw reply

* Re: [GUILT] handle branches with slashes in guilt-graph
From: Jeff Sipek @ 2011-10-18 13:30 UTC (permalink / raw)
  To: Per Cederqvist; +Cc: git, ceder
In-Reply-To: <4E9D57BB.2030707@opera.com>

On Tue, Oct 18, 2011 at 12:40:59PM +0200, Per Cederqvist wrote:
> Avoid sed errors when the branch name contains a slash.

Makes sense.  I'll test it and add it to my patch queue.

Thanks,

Jeff.

> Signed-off-by: Per Cederqvist <cederp@opera.com>
> 
> --- /usr/bin/guilt-graph~	2011-01-25 20:15:50.000000000 +0100
> +++ /usr/bin/guilt-graph	2011-10-18 12:30:31.000000000 +0200
> @@ -37,9 +37,10 @@ disp "digraph G {"
> 
>  current="$top"
> 
> +safebranch=`echo "$branch"|sed 's%/%\\\\/%g'`
>  while [ "$current" != "$base" ]; do
>  	pname=`git show-ref | sed -n -e "
> -/^$current refs\/patches\/$branch/ {
> +/^$current refs\/patches\/$safebranch/ {
>  	s,^$current refs/patches/$branch/,,
>  	p
>  	q

-- 
Once you have their hardware. Never give it back.
(The First Rule of Hardware Acquisition)

^ permalink raw reply

* Problems after deleting submodule
From: Howard Miller @ 2011-10-18 11:54 UTC (permalink / raw)
  To: git

I included a submodule in my project then decided I didn't like
submodules and deleted it again. I followed the advice of delting
.gitmodules, the bit from .git/config and then git rm'ing the
submodule. Seemed to work. I then copied files with the same directory
name into where the submodule was. However, I can't add them.

Doing git add /path/to/old/submodule - does nothing, files are not
staged, no error messages no nothing.
If I try to git rm /path/to/old/submodule - it just says 'did not
match any files'.

It simply does not seem to want to add anything to the old submodule
location. I had a grep around and could not see any obvious references
in the repo.

I'm a bit stuck... any suggestions for things I can try much appreciated.

^ permalink raw reply

* Repo seems broken after deleting submodule
From: Howard Miller @ 2011-10-18 11:05 UTC (permalink / raw)
  To: git

I included a submodule in my project then decided I didn't like
submodules and deleted it again. I followed the advice of delting
.gitmodules, the bit from .git/config and then git rm'ing the
submodule. Seemed to work. I then copied files with the same directory
name into where the submodule was. However, I can't add them.

Doing git add /path/to/old/submodule - does nothing, files are not
staged, no error messages no nothing.
If I try to git rm /path/to/old/submodule - it just says 'did not
match any files'.

I'm a bit stuck... any suggestions for things I can try much appreciated.

^ permalink raw reply

* [GUILT] handle branches with slashes in guilt-graph
From: Per Cederqvist @ 2011-10-18 10:40 UTC (permalink / raw)
  To: Jeff Sipek; +Cc: git, ceder

Avoid sed errors when the branch name contains a slash.

Signed-off-by: Per Cederqvist <cederp@opera.com>

--- /usr/bin/guilt-graph~	2011-01-25 20:15:50.000000000 +0100
+++ /usr/bin/guilt-graph	2011-10-18 12:30:31.000000000 +0200
@@ -37,9 +37,10 @@ disp "digraph G {"

  current="$top"

+safebranch=`echo "$branch"|sed 's%/%\\\\/%g'`
  while [ "$current" != "$base" ]; do
  	pname=`git show-ref | sed -n -e "
-/^$current refs\/patches\/$branch/ {
+/^$current refs\/patches\/$safebranch/ {
  	s,^$current refs/patches/$branch/,,
  	p
  	q

^ permalink raw reply

* Re: [PATCH 2/2] gitweb: add a feature to show side-by-side diff
From: Jakub Narebski @ 2011-10-18 10:36 UTC (permalink / raw)
  To: Kato Kazuyoshi; +Cc: git, Jakub Narebski
In-Reply-To: <m34nz771mj.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Kato Kazuyoshi <kato.kazuyoshi@gmail.com> writes:
> 
> > gitweb currently has a feature to show diff but it doesn't support
> > "side-by-side" style. This modification introduces:
> > 
> >  * The "ds" query parameter to specify the style of diff.
> >  * The format_diff_chunk() to reorganize an output of diff.
> >  * The diff_nav() for form.
> 
> I would state it a bit differently.
> 
> I would say that this patch introduces support for side-by-side diff
> in git_patchset_body, and that style of diff is controlled by newly
> introduces 'diff_style' ("ds") parameter.
> 
> I would say a bit later that navigation bar got extended to make it
> easy to switch between 'inline' and 'sidebyside' diff.

I think it would be good idea to explain algorithm here, and perhaps
also layout used.

When I was thinking about implementing side-by-side diff in gitweb, I
was always stopped by the problem of aligning changes.  In your
solution changes are always aligned to top, which is a simple
solution.

> > +sub format_diff_chunk {
> > +	my @chunk = @_;
> > +
> > +	my $first_class = $chunk[0]->[0];
> > +	my @partial = map { $_->[1] } grep { $_->[0] eq $first_class } @chunk;
> > +
> > +	if (scalar @partial < scalar @chunk) {
> > +		return join '', ("<div class='chunk'><div class='old'>",
> > +		             @partial,
> > +		             "</div>",
> > +		             "<div class='new'>",
> > +		             (map {
> > +		                 $_->[1];
> > +		             } @chunk[scalar @partial..scalar @chunk-1]),
> > +		             "</div></div>");
> > +	} else {
> > +		return join '', ("<div class='chunk'><div class='",
> > +		             ($first_class eq 'add' ? 'new' : 'old'),
> > +		             "'>",
> > +		             @partial,
> > +		             "</div></div>");
> > +	}
> > +}

This is I think unnecessary complicated.  What you do here is
separating removed and added lines (either can be empty), and putting
removed on left (as "old"), and added on right (as "new").

It means that the following unified diff:

     --- a/foo
     +++ b/foo
     @@ -1,5 +1,4 @@
     -foo
     -FOO
      bar
     -baz
     +b
     +baZ
      quux

would be turned into following side by side diff:

      foo             | 
      FOO             |
      bar             |  bar
      baz             |  b
                      |  baZ
      quux            |  quux

It's a bit strange that context is put line by line, and changed lines
are put in "blocks".

> > @@ -4940,12 +4967,31 @@ sub git_patchset_body {
> > 
> >  		# the patch itself
> >  	LINE:
> > +		my @chunk;
> >  		while ($patch_line = <$fd>) {
> >  			chomp $patch_line;
> > 
> >  			next PATCH if ($patch_line =~ m/^diff /);
> > 
> > -			print format_diff_line($patch_line, \%from, \%to);
> > +			my ($class, $line) = format_diff_line($patch_line, \%from, \%to);
> > +			if ($is_inline) {
> > +				print $line;
> > +			} elsif ($class eq 'add' || $class eq 'rem') {
> > +				push @chunk, [ $class, $line ];
> > +			} else {
> > +				if (@chunk) {
> > +					print format_diff_chunk(@chunk);
> > +					@chunk = ();
> > +				} elsif ($class eq 'chunk_header') {
> > +					print $line;
> > +				} else {
> > +					print '<div class="chunk"><div class="old">',
> > +					      $line,
> > +					      '</div><div class="new">',
> > +					      $line,
> > +					      '</div></div>';
> > +				}
> > +			}

Note that your side by side diff wouldn't work for combined diff,
which gitweb supports.  You should show 'unified' / 'inline' format
for combined diff (more than one parent / from).

-- 
Jakub Narębski

^ permalink raw reply

* [PATCH/RFC 3/2 (fixed)] Refactor Git::config_*
From: Jakub Narebski @ 2011-10-18  9:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, Cord Seele, Matthieu Moy, git, Cord Seele
In-Reply-To: <201110172347.42568.jnareb@gmail.com>

From: Junio C Hamano <gitster@pobox.com>

There is, especially with addition of Git::config_path(), much code
repetition in the Git::config_* family of subroutines.

Refactor common parts of Git::config(), Git::config_bool(),
Git::config_int() and Git::config_path() into _config_common()
helper method, reducing code duplication.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Jakub Narebski wrote:
>
> I'll resend amended commit.

Here it is.

 perl/Git.pm |   74 ++++++++++++++--------------------------------------------
 1 files changed, 18 insertions(+), 56 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index c775b4f..8e1e2fd 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -576,24 +576,7 @@ This currently wraps command('config') so it is not so fast.
 
 sub config {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config');
-		unshift @cmd, $self if $self;
-		if (wantarray) {
-			return command(@cmd, '--get-all', $var);
-		} else {
-			return command_oneline(@cmd, '--get', $var);
-		}
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return;
-		} else {
-			throw $E;
-		}
-	};
+	return _config_common($self, $var);
 }
 
 
@@ -609,25 +592,10 @@ This currently wraps command('config') so it is not so fast.
 
 sub config_bool {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--bool', '--get', $var);
-		unshift @cmd, $self if $self;
-		my $val = command_oneline(@cmd);
-		return undef unless defined $val;
-		return $val eq 'true';
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	my $val = scalar _config_common($self, $var, {'kind' => '--bool'});
+	return (defined $val && $val eq 'true');
 }
 
-
 =item config_path ( VARIABLE )
 
 Retrieve the path configuration C<VARIABLE>. The return value
@@ -639,24 +607,7 @@ This currently wraps command('config') so it is not so fast.
 
 sub config_path {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--path');
-		unshift @cmd, $self if $self;
-		if (wantarray) {
-			return command(@cmd, '--get-all', $var);
-		} else {
-			return command_oneline(@cmd, '--get', $var);
-		}
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	return _config_common($self, $var, {'kind' => '--path'});
 }
 
 
@@ -705,16 +656,27 @@ This currently wraps command('config') so it is not so fast.
 
 sub config_int {
 	my ($self, $var) = _maybe_self(@_);
+	return scalar _config_common($self, $var, {'kind' => '--int'});
+}
+
+# Common subroutine to implement bulk of what the config* family of methods
+# do. This curently wraps command('config') so it is not so fast.
+sub _config_common {
+	my ($self, $var, $opts) = @_;
 
 	try {
-		my @cmd = ('config', '--int', '--get', $var);
+		my @cmd = ('config', $opts->{'kind'} ? $opts->{'kind'} : ());
 		unshift @cmd, $self if $self;
-		return command_oneline(@cmd);
+		if (wantarray) {
+			return command(@cmd, '--get-all', $var);
+		} else {
+			return command_oneline(@cmd, '--get', $var);
+		}
 	} catch Git::Error::Command with {
 		my $E = shift;
 		if ($E->value() == 1) {
 			# Key not found.
-			return undef;
+			return;
 		} else {
 			throw $E;
 		}
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH 1/4] git-gui: handle config booleans without value
From: Pat Thoyts @ 2011-10-18  8:25 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: git
In-Reply-To: <CAKPyHN3dQ+qWeJtGzigEm=fZe62ZeRW-QGq0jnycKEBVaWn=mA@mail.gmail.com>

Bert Wesarg <bert.wesarg@googlemail.com> writes:

>On Tue, Oct 18, 2011 at 01:13, Pat Thoyts
><patthoyts@users.sourceforge.net> wrote:
>> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>>
>>>When git interprets a config variable without a value as bool it is considered
>>>as true. But git-gui doesn't so until yet.
>>>
>>>The value for boolean configs are also case-insensitive.
>>>
>>>Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>>---
>>> git-gui.sh |   16 ++++++++++++++--
>>> 1 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>>diff --git a/git-gui.sh b/git-gui.sh
>>>index f897160..d687871 100755
>>>--- a/git-gui.sh
>>>+++ b/git-gui.sh
>>>@@ -299,7 +299,9 @@ proc is_config_true {name} {
>>>       global repo_config
>>>       if {[catch {set v $repo_config($name)}]} {
>>>               return 0
>>>-      } elseif {$v eq {true} || $v eq {1} || $v eq {yes}} {
>>>+      }
>>>+      set v [string tolower $v]
>>>+      if {$v eq {} || $v eq {true} || $v eq {1} || $v eq {yes} || $v eq {on}} {
>>>               return 1
>>>       } else {
>>>               return 0
>>
>> This looks ok - we actually have a [string is boolean $v] test we could
>> use eg:
>>  if {[string is boolean $v]} {
>>    return [expr {$v eq {} ? 1 : !!$v}]
>>  }
>> although I'm not sure it gains us much. This would match everything Tcl
>> believes to be a boolean - yes/no, on/off, true/false and 1/0. Without
>> -strict the [string is] test will consider {} to be a boolean.
>>
>>
>>>@@ -310,7 +312,9 @@ proc is_config_false {name} {
>>>       global repo_config
>>>       if {[catch {set v $repo_config($name)}]} {
>>>               return 0
>>>-      } elseif {$v eq {false} || $v eq {0} || $v eq {no}} {
>>>+      }
>>>+      set v [string tolower $v]
>>>+      if {$v eq {false} || $v eq {0} || $v eq {no} || $v eq {off}} {
>>>               return 1
>>>       } else {
>>>               return 0
>>>@@ -1060,6 +1064,10 @@ git-version proc _parse_config {arr_name args} {
>>>                               } else {
>>>                                       set arr($name) $value
>>>                               }
>>>+                      } elseif {[regexp {^([^\n]+)$} $line line name]} {
>>>+                              # no value given, but interpreting them as
>>>+                              # boolean will be handled as true
>>>+                              set arr($name) {}
>>>                       }
>>>               }
>>>       }
>>>@@ -1075,6 +1083,10 @@ git-version proc _parse_config {arr_name args} {
>>>                                       } else {
>>>                                               set arr($name) $value
>>>                                       }
>>>+                              } elseif {[regexp {^([^=]+)$} $line line name]} {
>>>+                                      # no value given, but interpreting them as
>>>+                                      # boolean will be handled as true
>>>+                                      set arr($name) {}
>>>                               }
>>>                       }
>>>                       close $fd_rc
>>
>> Is this really how git treats boolean config values? I can't seem to
>> demonstrate that to myself:
>> pat@frog:/opt/src/git-gui$ git config --unset core.xyzzy
>> pat@frog:/opt/src/git-gui$ git config --bool --get core.xyzzy
>> pat@frog:/opt/src/git-gui$ git config --bool core.xyzzy 1
>> pat@frog:/opt/src/git-gui$ git config --bool --get core.xyzzy
>> true
>>
>> I assume I'm using the wrong test for this.
>
>It looks like you can't set it with git-config. But I know, that writing:
>
>[core]
>        xyyzy
>
>into the git config file and than calling git config --bool --get
>core.xyzzy, will give you true.
>
>Bert

OK thanks for explaining.
-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox