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

Jakub Narebski <jnareb@gmail.com> writes:

> The problem was with duplicated _maybe_self(@_), which should be run only
> once.  t9700-perl-git.sh passed because it uses only (recommended) object
> form, and not procedural form like git-svn.

Thanks.

I have a suspicion that a more standard implementation of _maybe_self()
would instead unshift a dummy singleton Git() instance instead of undef to
lift such a limitation, but perhaps the code relies on undef'ness of self
when called as a vanilla subroutine.

^ permalink raw reply

* Re: [PATCH 2/4] git-gui: add smart case search mode in searchbar
From: Pat Thoyts @ 2011-10-17 23:29 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: Andrew Ardill, git
In-Reply-To: <CAKPyHN2XQYrGDhfjX4G12Ggw6DuJasuYbLQvfbmikBbMezp4=g@mail.gmail.com>

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

>On Mon, Oct 17, 2011 at 00:32, Andrew Ardill <andrew.ardill@gmail.com> wrote:
>> I don't really know tcl so I'm not certain, but it looks like you
>> never reset the case sensitive flag once it has been set by entering
>> an upper case letter. If I accidentally enter an upper case letter and
>> have set smartcase true, I would expect that deleting that character
>> would turn case sensitivity off again.
>>
>
>I never reset it, because your case is a way to search case-sensitive
>for an expression in all lower-case. For example, if you would like to
>search for 'git' case-sensitively, you would type: 'gitA^H'. A direct
>shortcut to toggle the case flag could also be of use. The other idea
>which come to mind, is to reset the case flag, if you clear the input
>field. I.e. type 'G^Hgit' would still seach case-insensitive.
>
>Bert

I don't really like the way it works at the moment. It seems very keen
to enable the case sensitive mode and you must use the mouse to disable
that. I see why you chose to make it work that way though. I think
resetting it in clearing the field makes sense.

The rest of the series looks good. The history recording is nice to have.

-- 
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

* Re: [PATCH/RFC 1/2] gitweb: change format_diff_line() to remove leading SP from $diff_class
From: Kato Kazuyoshi @ 2011-10-17 23:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v62jnmog3.fsf@alter.siamese.dyndns.org>

On Tue, Oct 18, 2011 at 4:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Kato Kazuyoshi <kato.kazuyoshi@gmail.com> writes:
>
>> The format_diff_line() will return $diff_class and HTML in upcoming changes.
>
> An auxiliary piece of information like this is fine at the end of the
> commit log message, but the patch itself wants to be justified
> standalone.  Perhaps this should be sufficient:
>
>        The $diff_class variable to classify the kind of line in the diff
>        output was prefixed with a SP, only so that the code to synthesize
>        value for "class" attribute can blindly concatenate it with
>        another value "diff". This made the code unnecessarily ugly.
>
>        Instead, add SP that separates the value of $diff_class from
>        another class value "diff" where <div class="..."> string is
>        created and drop the leading SP from the value of $diff_class.
>
> Explained this way, it does not even have to mention that the return value
> will be changed in a different patch.
>

Thanks. I couldn't write a good summary for my patch because it was just
an "adjust" for me. However your summary is really clear!

>>  gitweb/gitweb.perl |   24 +++++++++++++-----------
>>  1 files changed, 13 insertions(+), 11 deletions(-)
>>
>> 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 {
>> ...
>> +
>> +     my $div_open = '<div class="' . (join ' ', ('diff', $diff_class)) . '">';
>
> I think using a separate helper variable like this is a good change.  You
> do not have to worry about the issue in three different places.
>
> But doesn't join(" ", ("frotz", "")) still give you "frotz "?  It is OK to
> punt and say
>
>        my $div_open = '<div class="diff $diff_class">';
>
> which would be far easier to read. It may sacrifice a bit of tidiness in
> the resulting HTML but the tidiness of the source outweighs it.
>
> Of course, if you have tons of classes, it may be worth doing something
> like
>
>        join(" ", grep { defined $_ && $_ ne ""}  @diff_classes);
>
> but I do not think it is worth it in this particular case.
>

Yeah, I want to remove unnecessary SP that you mentioned before.
But well, join(" ", ("frotz", "")) give me "frotz ".
I will add some per-function test cases to gitweb before this patch series.

Thanks,

-- 
Kato Kazuyoshi

^ permalink raw reply

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

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
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH 2/4] git-gui: add smart case search mode in searchbar
From: Andrew Ardill @ 2011-10-18  3:33 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: Bert Wesarg, git
In-Reply-To: <87ipnngpsj.fsf@fox.patthoyts.tk>

I agree that clearing the field to reset case sensitivity is a nice
compromise between easily turning smartcase off (by typing a capital)
and easily turning it on again if you accidentally turned it off, or
decided you don't want it on.

In most cases you would think an accidental capital would be the first
letter typed anyhow.

Regards,

Andrew Ardill

^ permalink raw reply

* Compiling on Windows
From: Andrew Ardill @ 2011-10-18  4:08 UTC (permalink / raw)
  To: git

Hi list, I have been searching for details on what is required to
compile on Windows, but haven't found anything conclusive. Perhaps
there is something on the wiki, but unfortunately it is down at the
moment.

Can anyone point me in the right direction? I would like to be able to
compile and test topic branches, and perhaps even do some dev work on
my windows machine.

Regards,

Andrew Ardill

^ permalink raw reply

* [PATCH 0/3] stupid git tricks
From: Jeff King @ 2011-10-18  4:49 UTC (permalink / raw)
  To: git

Here are a few contrib tidbits that I find invaluable when working with
git. They're pretty simple ideas, but I've been using them all for
at least a year[1], and they've made my life more pleasant and
productive. So I thought I'd share.

  [1/3]: contrib: add diff highlight script
  [2/3]: contrib: add git-jump script
  [3/3]: completion: match ctags symbol names in grep patterns

-Peff

[1] Actually, they all needed significant cleanup for me not to be
    embarrassed to share them, which I just did recently. Undoubtedly I
    introduced scores of new bugs during the cleanup, so take my "at
    least a year" with a grain of salt. The _concepts_ have been proven,
    at least. :)

^ permalink raw reply

* [PATCH 1/3] contrib: add diff highlight script
From: Jeff King @ 2011-10-18  4:52 UTC (permalink / raw)
  To: git
In-Reply-To: <20111018044955.GA8976@sigill.intra.peff.net>

This is a simple and stupid script for highlighting
differing parts of lines in a unified diff. See the README
for a discussion of the limitations.

Signed-off-by: Jeff King <peff@peff.net>
---
I posted a similar script before, but not as a contrib patch. Many of
the comments were that this could be done more accurately inside git
itself. Undoubtedly. But this is already written, and I have found it
very useful in practice. If somebody wants to come along with an
implementation inside git-diff, I'd be happy to compare output.

 contrib/diff-highlight/README         |   57 +++++++++++++++
 contrib/diff-highlight/diff-highlight |  124 +++++++++++++++++++++++++++++++++
 2 files changed, 181 insertions(+), 0 deletions(-)
 create mode 100644 contrib/diff-highlight/README
 create mode 100755 contrib/diff-highlight/diff-highlight

diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
new file mode 100644
index 0000000..1b7b6df
--- /dev/null
+++ b/contrib/diff-highlight/README
@@ -0,0 +1,57 @@
+diff-highlight
+==============
+
+Line oriented diffs are great for reviewing code, because for most
+hunks, you want to see the old and the new segments of code next to each
+other. Sometimes, though, when an old line and a new line are very
+similar, it's hard to immediately see the difference.
+
+You can use "--color-words" to highlight only the changed portions of
+lines. However, this can often be hard to read for code, as it loses
+the line structure, and you end up with oddly formatted bits.
+
+Instead, this script post-processes the line-oriented diff, finds pairs
+of lines, and highlights the differing segments.  It's currently very
+simple and stupid about doing these tasks. In particular:
+
+  1. It will only highlight a pair of lines if they are the only two
+     lines in a hunk.  It could instead try to match up "before" and
+     "after" lines for a given hunk into pairs of similar lines.
+     However, this may end up visually distracting, as the paired
+     lines would have other highlighted lines in between them. And in
+     practice, the lines which most need attention called to their
+     small, hard-to-see changes are touching only a single line.
+
+  2. It will find the common prefix and suffix of two lines, and
+     consider everything in the middle to be "different". It could
+     instead do a real diff of the characters between the two lines and
+     find common subsequences. However, the point of the highlight is to
+     call attention to a certain area. Even if some small subset of the
+     highlighted area actually didn't change, that's OK. In practice it
+     ends up being more readable to just have a single blob on the line
+     showing the interesting bit.
+
+The goal of the script is therefore not to be exact about highlighting
+changes, but to call attention to areas of interest without being
+visually distracting.  Non-diff lines and existing diff coloration is
+preserved; the intent is that the output should look exactly the same as
+the input, except for the occasional highlight.
+
+Use
+---
+
+You can try out the diff-highlight program with:
+
+---------------------------------------------
+git log -p --color | /path/to/diff-highlight
+---------------------------------------------
+
+If you want to use it all the time, drop it in your $PATH and put the
+following in your git configuration:
+
+---------------------------------------------
+[pager]
+	log = diff-highlight | less
+	show = diff-highlight | less
+	diff = diff-highlight | less
+---------------------------------------------
diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight
new file mode 100755
index 0000000..d893898
--- /dev/null
+++ b/contrib/diff-highlight/diff-highlight
@@ -0,0 +1,124 @@
+#!/usr/bin/perl
+
+# Highlight by reversing foreground and background. You could do
+# other things like bold or underline if you prefer.
+my $HIGHLIGHT   = "\x1b[7m";
+my $UNHIGHLIGHT = "\x1b[27m";
+my $COLOR = qr/\x1b\[[0-9;]*m/;
+
+my @window;
+
+while (<>) {
+	# We highlight only single-line changes, so we need
+	# a 4-line window to make a decision on whether
+	# to highlight.
+	push @window, $_;
+	next if @window < 4;
+	if ($window[0] =~ /^$COLOR*(\@| )/ &&
+	    $window[1] =~ /^$COLOR*-/ &&
+	    $window[2] =~ /^$COLOR*\+/ &&
+	    $window[3] !~ /^$COLOR*\+/) {
+		print shift @window;
+		show_pair(shift @window, shift @window);
+	}
+	else {
+		print shift @window;
+	}
+
+	# Most of the time there is enough output to keep things streaming,
+	# but for something like "git log -Sfoo", you can get one early
+	# commit and then many seconds of nothing. We want to show
+	# that one commit as soon as possible.
+	#
+	# Since we can receive arbitrary input, there's no optimal
+	# place to flush. Flushing on a blank line is a heuristic that
+	# happens to match git-log output.
+	if (!length) {
+		local $| = 1;
+	}
+}
+
+# Special case a single-line hunk at the end of file.
+if (@window == 3 &&
+    $window[0] =~ /^$COLOR*(\@| )/ &&
+    $window[1] =~ /^$COLOR*-/ &&
+    $window[2] =~ /^$COLOR*\+/) {
+	print shift @window;
+	show_pair(shift @window, shift @window);
+}
+
+# And then flush any remaining lines.
+while (@window) {
+	print shift @window;
+}
+
+exit 0;
+
+sub show_pair {
+	my @a = split_line(shift);
+	my @b = split_line(shift);
+
+	# Find common prefix, taking care to skip any ansi
+	# color codes.
+	my $seen_plusminus;
+	my ($pa, $pb) = (0, 0);
+	while ($pa < @a && $pb < @b) {
+		if ($a[$pa] =~ /$COLOR/) {
+			$pa++;
+		}
+		elsif ($b[$pb] =~ /$COLOR/) {
+			$pb++;
+		}
+		elsif ($a[$pa] eq $b[$pb]) {
+			$pa++;
+			$pb++;
+		}
+		elsif (!$seen_plusminus && $a[$pa] eq '-' && $b[$pb] eq '+') {
+			$seen_plusminus = 1;
+			$pa++;
+			$pb++;
+		}
+		else {
+			last;
+		}
+	}
+
+	# Find common suffix, ignoring colors.
+	my ($sa, $sb) = ($#a, $#b);
+	while ($sa >= $pa && $sb >= $pb) {
+		if ($a[$sa] =~ /$COLOR/) {
+			$sa--;
+		}
+		elsif ($b[$sb] =~ /$COLOR/) {
+			$sb--;
+		}
+		elsif ($a[$sa] eq $b[$sb]) {
+			$sa--;
+			$sb--;
+		}
+		else {
+			last;
+		}
+	}
+
+	print highlight(\@a, $pa, $sa);
+	print highlight(\@b, $pb, $sb);
+}
+
+sub split_line {
+	local $_ = shift;
+	return map { /$COLOR/ ? $_ : (split //) }
+	       split /($COLOR*)/;
+}
+
+sub highlight {
+	my ($line, $prefix, $suffix) = @_;
+
+	return join('',
+		@{$line}[0..($prefix-1)],
+		$HIGHLIGHT,
+		@{$line}[$prefix..$suffix],
+		$UNHIGHLIGHT,
+		@{$line}[($suffix+1)..$#$line]
+	);
+}
-- 
1.7.7.rc3.37.gc4dc8

^ permalink raw reply related

* [PATCH 2/3] contrib: add git-jump script
From: Jeff King @ 2011-10-18  4:54 UTC (permalink / raw)
  To: git
In-Reply-To: <20111018044955.GA8976@sigill.intra.peff.net>

This is a small script for helping your editor jump to
specific points of interest. See the README for details.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/git-jump/README   |   92 +++++++++++++++++++++++++++++++++++++++++++++
 contrib/git-jump/git-jump |   68 +++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+), 0 deletions(-)
 create mode 100644 contrib/git-jump/README
 create mode 100755 contrib/git-jump/git-jump

diff --git a/contrib/git-jump/README b/contrib/git-jump/README
new file mode 100644
index 0000000..1cebc32
--- /dev/null
+++ b/contrib/git-jump/README
@@ -0,0 +1,92 @@
+git-jump
+========
+
+Git-jump is a script for helping you jump to "interesting" parts of your
+project in your editor. It works by outputting a set of interesting
+spots in the "quickfix" format, which editors like vim can use as a
+queue of places to visit (this feature is usually used to jump to errors
+produced by a compiler). For example, given a diff like this:
+
+------------------------------------
+diff --git a/foo.c b/foo.c
+index a655540..5a59044 100644
+--- a/foo.c
++++ b/foo.c
+@@ -1,3 +1,3 @@
+ int main(void) {
+-  printf("hello word!\n");
++  printf("hello world!\n");
+ }
+-----------------------------------
+
+git-jump will feed this to the editor:
+
+-----------------------------------
+foo.c:2: printf("hello word!\n");
+-----------------------------------
+
+Obviously this trivial case isn't that interesting; you could just open
+`foo.c` yourself. But when you have many changes scattered across a
+project, you can use the editor's support to "jump" from point to point.
+
+Git-jump can generate three types of interesting lists:
+
+  1. The beginning of any diff hunks.
+
+  2. The beginning of any merge conflict markers.
+
+  3. Any grep matches.
+
+
+Using git-jump
+--------------
+
+To use it, just drop git-jump in your PATH, and then invoke it like
+this:
+
+--------------------------------------------------
+# jump to changes not yet staged for commit
+git jump diff
+
+# jump to changes that are staged for commit; you can give
+# arbitrary diff options
+git jump diff --cached
+
+# jump to merge conflicts
+git jump merge
+
+# jump to all instances of foo_bar
+git jump grep foo_bar
+
+# same as above, but case-insensitive; you can give
+# arbitrary grep options
+git jump grep -i foo_bar
+--------------------------------------------------
+
+
+Related Programs
+----------------
+
+You can accomplish some of the same things with individual tools. For
+example, you can use `git mergetool` to start vimdiff on each unmerged
+file. `git jump merge` is for the vim-wielding luddite who just wants to
+jump straight to the conflict text with no fanfare.
+
+As of git v1.7.2, `git grep` knows the `--open-files-in-pager` option,
+which does something similar to `git jump grep`. However, it is limited
+to positioning the cursor to the correct line in only the first file,
+leaving you to locate subsequent hits in that file or other files using
+the editor or pager. By contrast, git-jump provides the editor with a
+complete list of files and line numbers for each match.
+
+
+Limitations
+-----------
+
+This scripts was written and tested with vim. Given that the quickfix
+format is the same as what gcc produces, I expect emacs users have a
+similar feature for iterating through the list, but I know nothing about
+how to activate it.
+
+The shell snippets to generate the quickfix lines will almost certainly
+choke on filenames with exotic characters (like newlines).
diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
new file mode 100755
index 0000000..13b8e9f
--- /dev/null
+++ b/contrib/git-jump/git-jump
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+usage() {
+	cat <<\EOF
+usage: git jump <mode> [<args>]
+
+Jump to interesting elements in an editor.
+The <mode> parameter is one of:
+
+diff: elements are diff hunks. Arguments are given to diff.
+
+merge: elements are merge conflicts. Arguments are ignored.
+
+grep: elements are grep hits. Arguments are given to grep.
+EOF
+}
+
+open_editor() {
+	editor=`git var GIT_EDITOR`
+	eval "$editor -q \$1"
+}
+
+mode_diff() {
+	git diff --relative "$@" |
+	perl -ne '
+	if (m{^\+\+\+ b/(.*)}) { $file = $1; next }
+	defined($file) or next;
+	if (m/^@@ .*\+(\d+)/) { $line = $1; next }
+	defined($line) or next;
+	if (/^ /) { $line++; next }
+	if (/^[-+]\s*(.*)/) {
+		print "$file:$line: $1\n";
+		$line = undef;
+	}
+	'
+}
+
+mode_merge() {
+	git ls-files -u |
+	perl -pe 's/^.*?\t//' |
+	sort -u |
+	while IFS= read fn; do
+		grep -Hn '^<<<<<<<' "$fn"
+	done
+}
+
+# Grep -n generates nice quickfix-looking lines by itself,
+# but let's clean up extra whitespace, so they look better if the
+# editor shows them to us in the status bar.
+mode_grep() {
+	git grep -n "$@" |
+	perl -pe '
+	s/[ \t]+/ /g;
+	s/^ *//;
+	'
+}
+
+if test $# -lt 1; then
+	usage >&2
+	exit 1
+fi
+mode=$1; shift
+
+trap 'rm -f "$tmp"' 0 1 2 3 15
+tmp=`mktemp -t git-jump.XXXXXX` || exit 1
+"mode_$mode" "$@" >"$tmp" || { usage >&2; exit 1; }
+test -s "$tmp" || exit 0
+open_editor "$tmp"
-- 
1.7.7.rc3.37.gc4dc8

^ permalink raw reply related

* [PATCH 3/3] completion: match ctags symbol names in grep patterns
From: Jeff King @ 2011-10-18  5:01 UTC (permalink / raw)
  To: git
In-Reply-To: <20111018044955.GA8976@sigill.intra.peff.net>

A common thing to grep for is the name of a symbol. This
patch teaches the completion for "git grep" to look in
a 'tags' file, if present, to complete a pattern. For
example, in git.git:

  $ make tags
  $ git grep get_sha1<Tab><Tab>
  get_sha1                 get_sha1_oneline
  get_sha1_1               get_sha1_with_context
  get_sha1_basic           get_sha1_with_context_1
  get_sha1_hex             get_sha1_with_mode
  get_sha1_hex_segment     get_sha1_with_mode_1
  get_sha1_mb

Signed-off-by: Jeff King <peff@peff.net>
---
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.

Undoubtedly you can do something similar with cscope, or with an editor
plugin, but using grep feels very natural and simple to me.

For using with "git jump grep", I do this:

  # much easier to type
  git config --global alias.vgrep 'jump grep'

  # and set up completion for it, too
  cat >>~/.bashrc
  _git_vgrep() {
          _git_grep
  }

 contrib/completion/git-completion.bash |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8648a36..f4ab13d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1432,6 +1432,10 @@ _git_gitk ()
 	_gitk
 }
 
+_git_grep_ctag_match() {
+	awk -v ORS=' ' "/^${1////\\/}/ { print \$1 }" "$2"
+}
+
 _git_grep ()
 {
 	__git_has_doubledash && return
@@ -1454,6 +1458,13 @@ _git_grep ()
 		;;
 	esac
 
+	case "$COMP_CWORD,$prev" in
+	2,*|*,-*)
+		test -r tags || return
+		COMPREPLY=( $(compgen -W "`_git_grep_ctag_match "$cur" tags`") )
+		return
+	esac
+
 	__gitcomp "$(__git_refs)"
 }
 
-- 
1.7.7.rc3.37.gc4dc8

^ permalink raw reply related

* [PATCH 1/4] pack-objects: mark add_to_write_order() as inline
From: Dan McGee @ 2011-10-18  5:21 UTC (permalink / raw)
  To: git

This function is a whole 26 bytes when compiled on x86_64, but is
currently invoked over 1.037 billion times when running pack-objects on
the Linux kernel git repository. This is hitting the point where
micro-optimizations do make a difference, and inlining it only increases
the object file size by 38 bytes.

As reported by perf, this dropped task-clock from 84183 to 83373 ms, and
total cycles from 223.5 billion to 221.6 billion. Not astronomical, but
worth getting for adding one word.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 builtin/pack-objects.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 2b18de5..0ab3a3b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -454,7 +454,7 @@ static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
 	return 0;
 }
 
-static void add_to_write_order(struct object_entry **wo,
+static inline void add_to_write_order(struct object_entry **wo,
 			       int *endp,
 			       struct object_entry *e)
 {
-- 
1.7.7

^ permalink raw reply related

* [PATCH 2/4] pack-objects: use unsigned int for counter and offset values
From: Dan McGee @ 2011-10-18  5:21 UTC (permalink / raw)
  To: git
In-Reply-To: <1318915284-6361-1-git-send-email-dpmcgee@gmail.com>

This is done in some of the new pack layout code introduced in commit
1b4bb16b9ec331c. This more closely matches the nr_objects global that is
unsigned that these variables are based off of and bounded by.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 builtin/pack-objects.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0ab3a3b..0de10d2 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -455,7 +455,7 @@ static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
 }
 
 static inline void add_to_write_order(struct object_entry **wo,
-			       int *endp,
+			       unsigned int *endp,
 			       struct object_entry *e)
 {
 	if (e->filled)
@@ -465,7 +465,7 @@ static inline void add_to_write_order(struct object_entry **wo,
 }
 
 static void add_descendants_to_write_order(struct object_entry **wo,
-					   int *endp,
+					   unsigned int *endp,
 					   struct object_entry *e)
 {
 	struct object_entry *child;
@@ -477,7 +477,7 @@ static void add_descendants_to_write_order(struct object_entry **wo,
 }
 
 static void add_family_to_write_order(struct object_entry **wo,
-				      int *endp,
+				      unsigned int *endp,
 				      struct object_entry *e)
 {
 	struct object_entry *root;
@@ -490,7 +490,7 @@ static void add_family_to_write_order(struct object_entry **wo,
 
 static struct object_entry **compute_write_order(void)
 {
-	int i, wo_end;
+	unsigned int i, wo_end;
 
 	struct object_entry **wo = xmalloc(nr_objects * sizeof(*wo));
 
@@ -506,8 +506,8 @@ static struct object_entry **compute_write_order(void)
 	 * Make sure delta_sibling is sorted in the original
 	 * recency order.
 	 */
-	for (i = nr_objects - 1; 0 <= i; i--) {
-		struct object_entry *e = &objects[i];
+	for (i = nr_objects; i > 0;) {
+		struct object_entry *e = &objects[--i];
 		if (!e->delta)
 			continue;
 		/* Mark me as the first child */
-- 
1.7.7

^ permalink raw reply related

* [PATCH 4/4] pack-objects: rewrite add_descendants_to_write_order() iteratively
From: Dan McGee @ 2011-10-18  5:21 UTC (permalink / raw)
  To: git
In-Reply-To: <1318915284-6361-1-git-send-email-dpmcgee@gmail.com>

This removes the need to call this function recursively, shinking the
code size slightly and netting a small performance increase.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 builtin/pack-objects.c |   44 +++++++++++++++++++++++++++++++++++++-------
 1 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d9fb202..9efd1a7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -468,12 +468,43 @@ static void add_descendants_to_write_order(struct object_entry **wo,
 					   unsigned int *endp,
 					   struct object_entry *e)
 {
-	struct object_entry *child;
-
-	for (child = e->delta_child; child; child = child->delta_sibling)
-		add_to_write_order(wo, endp, child);
-	for (child = e->delta_child; child; child = child->delta_sibling)
-		add_descendants_to_write_order(wo, endp, child);
+	int add_to_order = 1;
+	while (e) {
+		if (add_to_order) {
+			struct object_entry *s;
+			/* add this node... */
+			add_to_write_order(wo, endp, e);
+			/* all its siblings... */
+			for (s = e->delta_sibling; s; s = s->delta_sibling) {
+				add_to_write_order(wo, endp, s);
+			}
+		}
+		/* drop down a level to add left subtree nodes if possible */
+		if (e->delta_child) {
+			add_to_order = 1;
+			e = e->delta_child;
+		} else {
+			add_to_order = 0;
+			/* our sibling might have some children, it is next */
+			if (e->delta_sibling) {
+				e = e->delta_sibling;
+				continue;
+			}
+			/* go back to our parent node */
+			e = e->delta;
+			while (e && !e->delta_sibling) {
+				/* we're on the right side of a subtree, keep
+				 * going up until we can go right again */
+				e = e->delta;
+			}
+			if (!e) {
+				/* done- we hit our original root node */
+				return;
+			}
+			/* pass it off to sibling at this level */
+			e = e->delta_sibling;
+		}
+	};
 }
 
 static void add_family_to_write_order(struct object_entry **wo,
@@ -484,7 +515,6 @@ static void add_family_to_write_order(struct object_entry **wo,
 
 	for (root = e; root->delta; root = root->delta)
 		; /* nothing */
-	add_to_write_order(wo, endp, root);
 	add_descendants_to_write_order(wo, endp, root);
 }
 
-- 
1.7.7

^ permalink raw reply related

* [PATCH 3/4] pack-objects: don't traverse objects unnecessarily
From: Dan McGee @ 2011-10-18  5:21 UTC (permalink / raw)
  To: git
In-Reply-To: <1318915284-6361-1-git-send-email-dpmcgee@gmail.com>

This brings back some of the performance lost in optimizing recency
order inside pack objects. We were doing extreme amounts of object
re-traversal: for the 2.14 million objects in the Linux kernel
repository, we were calling add_to_write_order() over 1.03 billion times
(a 0.2% hit rate, making 99.8% of of these calls extraneous).

Two optimizations take place here- we can start our objects array
iteration from a known point where we left off before we started trying
to find our tags, and we don't need to do the deep dives required by
add_family_to_write_order() if the object has already been marked as
filled.

These two optimizations bring some pretty spectacular results via `perf
stat`:

task-clock:   83373 ms        --> 43800 ms         (50% faster)
cycles:       221,633,461,676 --> 116,307,209,986  (47% fewer)
instructions: 149,299,179,939 --> 122,998,800,184  (18% fewer)

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 builtin/pack-objects.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0de10d2..d9fb202 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -490,7 +490,7 @@ static void add_family_to_write_order(struct object_entry **wo,
 
 static struct object_entry **compute_write_order(void)
 {
-	unsigned int i, wo_end;
+	unsigned int i, wo_end, last_untagged;
 
 	struct object_entry **wo = xmalloc(nr_objects * sizeof(*wo));
 
@@ -521,7 +521,7 @@ static struct object_entry **compute_write_order(void)
 	for_each_tag_ref(mark_tagged, NULL);
 
 	/*
-	 * Give the commits in the original recency order until
+	 * Give the objects in the original recency order until
 	 * we see a tagged tip.
 	 */
 	for (i = wo_end = 0; i < nr_objects; i++) {
@@ -529,6 +529,7 @@ static struct object_entry **compute_write_order(void)
 			break;
 		add_to_write_order(wo, &wo_end, &objects[i]);
 	}
+	last_untagged = i;
 
 	/*
 	 * Then fill all the tagged tips.
@@ -541,7 +542,7 @@ static struct object_entry **compute_write_order(void)
 	/*
 	 * And then all remaining commits and tags.
 	 */
-	for (i = 0; i < nr_objects; i++) {
+	for (i = last_untagged; i < nr_objects; i++) {
 		if (objects[i].type != OBJ_COMMIT &&
 		    objects[i].type != OBJ_TAG)
 			continue;
@@ -551,7 +552,7 @@ static struct object_entry **compute_write_order(void)
 	/*
 	 * And then all the trees.
 	 */
-	for (i = 0; i < nr_objects; i++) {
+	for (i = last_untagged; i < nr_objects; i++) {
 		if (objects[i].type != OBJ_TREE)
 			continue;
 		add_to_write_order(wo, &wo_end, &objects[i]);
@@ -560,8 +561,13 @@ static struct object_entry **compute_write_order(void)
 	/*
 	 * Finally all the rest in really tight order
 	 */
-	for (i = 0; i < nr_objects; i++)
-		add_family_to_write_order(wo, &wo_end, &objects[i]);
+	for (i = last_untagged; i < nr_objects; i++) {
+		if (!objects[i].filled)
+			add_family_to_write_order(wo, &wo_end, &objects[i]);
+	}
+
+	if(wo_end != nr_objects)
+		die("ordered %u objects, expected %u", wo_end, nr_objects);
 
 	return wo;
 }
-- 
1.7.7

^ permalink raw reply related

* Re: Compiling on Windows
From: Frans Klaver @ 2011-10-18  5:41 UTC (permalink / raw)
  To: git, Andrew Ardill
In-Reply-To: <CAH5451=7Em7sPzknVx8i2VBSAZxZwg1Awr8s3Nr2W=A6SDEZEw@mail.gmail.com>

On Tue, 18 Oct 2011 06:08:16 +0200, Andrew Ardill  
<andrew.ardill@gmail.com> wrote:

> Hi list, I have been searching for details on what is required to
> compile on Windows, but haven't found anything conclusive. Perhaps
> there is something on the wiki, but unfortunately it is down at the
> moment.

There's this project called msysgit[0]. The website depends on the wiki  
for explanation though.

Cheers,
Frans

[0]http://code.google.com/p/msysgit/

^ permalink raw reply

* Re: Compiling on Windows
From: Andrew Ardill @ 2011-10-18  6:17 UTC (permalink / raw)
  To: Frans Klaver; +Cc: git
In-Reply-To: <op.v3i8rauz0aolir@keputer.lokaal>

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).

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

Thanks again,

Andrew Ardill

^ permalink raw reply

* Re: [PATCH] git-gui: fix multi selected file operation
From: Bert Wesarg @ 2011-10-18  6:31 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: git
In-Reply-To: <877h43l0f3.fsf@fox.patthoyts.tk>

On Tue, Oct 18, 2011 at 00:26, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>>Hi,
>>
>>On Sun, Oct 16, 2011 at 00:48, Pat Thoyts
>><patthoyts@users.sourceforge.net> wrote:
>>> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>>>
>>>>The current path for what we see the diff is not in the list of selected
>>>>paths. But when we add single paths (with Ctrl) to the set the current path
>>>>would not be used when the action is performed.
>>>>
>>>>Fix this by explicitly putting the path into the list before we start
>>>>showing the diff.
>>>>
>>>>Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>>>---
>>>> git-gui.sh |    1 +
>>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>>diff --git a/git-gui.sh b/git-gui.sh
>>>>index f897160..e5dd8bc 100755
>>>>--- a/git-gui.sh
>>>>+++ b/git-gui.sh
>>>>@@ -2474,6 +2474,7 @@ proc toggle_or_diff {w x y} {
>>>>                               [concat $after [list ui_ready]]
>>>>               }
>>>>       } else {
>>>>+              set selected_paths($path) 1
>>>>               show_diff $path $w $lno
>>>>       }
>>>> }
>>>
>>> It is not clear what I should be looking for to test this. Can you
>>> re-write the commit message to be more clear about what you are
>>> fixing. Is this multiple unstaged files in the staging box? If so I
>>> don't see what path display is changing.
>>
>>Sorry, for this bad description. I will give you a recipe here what to
>>do to expose the problem. I try later to form this into a new commit
>>message:
>>
>>You have 2 modified, not staged files A and B. Your current view shows
>>the diff for A. Adding B to the selection via Ctrl+Button1 and than
>>perform the "Stage To Commit" action from the "Commit" menu results
>>only in the staging of B.
>>
>>Note, using Shift+Button1 (i.e. 'adding a range of files to the
>>selection') results in the staging of both files A and B.
>>
>>Bert
>
> Ah ok - that explains things and I can see the issue now. I think
> something like:
>
> "When staging a selection of files using Shift-Click to choose a range
> of files then using Ctrl-T or the Stage To Commit menu item will stage
> all the selected files. However if a non-sequential range is selected
> using Ctrl-Click then only the last name selected gets staged. This
> commit fixes this to properly stage all selected files by explicitly
> adding the path to the list before showing the diff."

Thanks for this. A slight, but important, change to the second last sentence:

"...using Ctrl-Click then all but the first name selected gets staged."

Its the first which does not get staged. Ie. that one, which was
selected just by a Click to view the diff.

Bert

>
> will do.
>
> --
> 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

* Re: [Bug] git pull doesn't recognize --work-tree parameter
From: Junio C Hamano @ 2011-10-18  6:34 UTC (permalink / raw)
  To: Jeff King; +Cc: Kirill Likhodedov, git
In-Reply-To: <20111013194432.GA20082@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>>   $ mkdir .git/tmp
>>   $ cd .git/tmp
>>   $ git rebase
>>   fatal: fatal: /home/peff/local/git/private/libexec/git-core/git-rebase
>>   cannot be used without a working tree.
>> 
>> So that case is already broken. The only change this would make is that
>> what used to fail would not actually take them to the top-level of the
>> working tree[1].
>
> Ugh. It does work if you do:
>
>   mkdir .git/tmp
>   cd .git/tmp
>   GIT_DIR=$PWD/.. git rebase

Actually, this one is OK. Presense of GIT_DIR combined with the lack of
GIT_WORK_TREE means that .git/tmp must be the top of the working tree, so
it is the script's responsibility to populate the directory with what
matches to $GIT_DIR/index between "mkdir" and "rebase" in the above
sequence.

So I suspect we won't have to worry about this case either.

^ permalink raw reply

* Re: [PATCH 3/3] git-gui: new config to control staging of untracked files
From: Bert Wesarg @ 2011-10-18  6:34 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: Heiko Voigt, git
In-Reply-To: <87r52bgrky.fsf@fox.patthoyts.tk>

On Tue, Oct 18, 2011 at 00:51, Pat Thoyts
<patthoyts@users.sourceforge.net> wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
>
>>Hi,
>>
>>On Mon, Oct 17, 2011 at 08:47:50PM +0200, Bert Wesarg wrote:
>>> On Mon, Oct 17, 2011 at 20:34, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>>> > Here I am wondering whether we have a similar mechanism in git gui like
>>> > in core git that makes yes,true,1 equivalents (and similar with other
>>> > values) ?
>>>
>>> But it is not only yes,true,1 or no,false,0 its a tristate with the
>>> third state 'ask'. For booleans, there is such functionality in git
>>> gui. See is_config_true and is_config_false. Reusing these for this
>>> tristate wouldn't work. The current check here is indeed very strict
>>> and should be loosen by at least ignoring the case, surrounding
>>> spaces, and allow also true/false. But also note, that this variable
>>> can be set via the Options menu, so you can't mistype it.
>>
>>Well if using git config you can ;-). I just wanted to ask whether we
>>may already have machinery which supports such tristate.
>>If we do not I think the current "strict" configuration is fine. In most
>>cases the user will use the gui itself to configure such behavior so
>>thats no big deal.
>>If someone needs that it can be added later on.
>>
>>Thanks, Heiko
>>
>
> This set of 3 patches looks fine. I was a bit dubious of the new
> phrasing for the ask condition but it is growing on me. I wonder it it
> might be worth including the number of untracked files to be staged too
> eg: "Stage 15 untracked files?"
>
>   set reply [ask_popup [mc "Stage %d untracked files?" \
>                             [llength $untracked_paths]]]

I thought about to list the untracked files in the dialog, but
couldn't find a good template dialog for this. But the number is
definitely worth I think.

>
> Loosening the check we can do using
>  switch -glob -- [get_config gui.stageuntracked] {
>    [Nn]* { set reply 0}
>    [Yy]* { set reply 1}
>    default { ... }
>  }

I think this is too loose ;-)

Bert

>
> --
> 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

* Re: [PATCH 1/4] git-gui: handle config booleans without value
From: Bert Wesarg @ 2011-10-18  6:39 UTC (permalink / raw)
  To: Pat Thoyts; +Cc: git
In-Reply-To: <87mxczgqiw.fsf@fox.patthoyts.tk>

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

>
> --
> 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

* Re: Compiling on Windows
From: Alexey Shumkin @ 2011-10-18  6:39 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: git
In-Reply-To: <CAH5451=7Em7sPzknVx8i2VBSAZxZwg1Awr8s3Nr2W=A6SDEZEw@mail.gmail.com>

Take a look at Cygwin.
On Windows I use Git under Cygwin.
And, of course, Git can be compiled there (with dependencies installed 
like openssl-dev, curl-dev, etc)

> Hi list, I have been searching for details on what is required to
> compile on Windows, but haven't found anything conclusive. Perhaps
> there is something on the wiki, but unfortunately it is down at the
> moment.
> 
> Can anyone point me in the right direction? I would like to be able to
> compile and test topic branches, and perhaps even do some dev work on
> my windows machine.
> 
> Regards,
> 
> Andrew Ardill

^ permalink raw reply

* Re: [PATCH 3/3] completion: match ctags symbol names in grep patterns
From: Junio C Hamano @ 2011-10-18  7:15 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111018050105.GC9008@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> A common thing to grep for is the name of a symbol. This
> patch teaches the completion for "git grep" to look in
> a 'tags' file, if present, to complete a pattern. For
> example, in git.git:
>
>   $ make tags
>   $ git grep get_sha1<Tab><Tab>
>   get_sha1                 get_sha1_oneline
>   get_sha1_1               get_sha1_with_context
>   get_sha1_basic           get_sha1_with_context_1
>   get_sha1_hex             get_sha1_with_mode
>   get_sha1_hex_segment     get_sha1_with_mode_1
>   get_sha1_mb
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> 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.

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?

 Makefile |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 17404c4..b38f55b 100644
--- a/Makefile
+++ b/Makefile
@@ -2127,17 +2127,25 @@ po/git.pot: $(LOCALIZED_C)
 
 pot: po/git.pot
 
+git_check = $(shell git ls-files >/dev/null 2>&1; echo $$?)
+ifeq ($(git_check),0)
+FIND_SOURCE_FILES = git ls-files '*.[hcS]'
+else
+FIND_SOURCE_FILES = $(FIND) . \( -name .git -type d -prune \) \
+		-o \( -name '*.[hcS]' -type f -print \)
+endif
+
 $(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)):\

^ permalink raw reply related

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

Junio C Hamano wrote:

> Perhaps something like this is in order?
[...]
> +++ b/Makefile
> @@ -2127,17 +2127,25 @@ po/git.pot: $(LOCALIZED_C)
>  
>  pot: po/git.pot
>  
> +git_check = $(shell git ls-files >/dev/null 2>&1; echo $$?)
> +ifeq ($(git_check),0)
> +FIND_SOURCE_FILES = git ls-files '*.[hcS]'
> +else
> +FIND_SOURCE_FILES = $(FIND) . \( -name .git -type d -prune \) \
> +		-o \( -name '*.[hcS]' -type f -print \)
> +endif

Neat.  I'd prefer something like

	FIND_SOURCE_FILES = \
		git ls-files '*.[hcS]' 2>/dev/null || \
		$(FIND) . -name .git -prune -o -name '*.[hcS]' -type f -print

that avoid punishing people who were using the makefile for some
purpose unrelated to tags and cscope, though. ;)

^ permalink raw reply

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

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>
>> Perhaps something like this is in order?
> [...]
>> +++ b/Makefile
>> @@ -2127,17 +2127,25 @@ po/git.pot: $(LOCALIZED_C)
>>  
>>  pot: po/git.pot
>>  
>> +git_check = $(shell git ls-files >/dev/null 2>&1; echo $$?)
>> +ifeq ($(git_check),0)
>> +FIND_SOURCE_FILES = git ls-files '*.[hcS]'
>> +else
>> +FIND_SOURCE_FILES = $(FIND) . \( -name .git -type d -prune \) \
>> +		-o \( -name '*.[hcS]' -type f -print \)
>> +endif
>
> Neat.  I'd prefer something like
>
> 	FIND_SOURCE_FILES = \
> 		git ls-files '*.[hcS]' 2>/dev/null || \
> 		$(FIND) . -name .git -prune -o -name '*.[hcS]' -type f -print
>
> that avoid punishing people who were using the makefile for some
> purpose unrelated to tags and cscope, though. ;)

Hmm, how would this punish anybody exactly (I just took the structure
from the way how the auto-depend is done)?

Besides, you would need to have the whole thing in a subshell or
something, as this is used as the upstream to "| xargs".

^ permalink raw reply

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

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

>>> +git_check = $(shell git ls-files >/dev/null 2>&1; echo $$?)
>>> +ifeq ($(git_check),0)

> Hmm, how would this punish anybody exactly (I just took the structure
> from the way how the auto-depend is done)?

The "shell git ls-files" is ran unconditionnally, hence a small
performance penality even if you don't run ctags.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ 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