Git development
 help / color / mirror / Atom feed
* [REPLACEMENT PATCH] Highlight keyboard shortcuts in git-add--interactive
From: Wincent Colaiuta @ 2007-12-01 14:29 UTC (permalink / raw)
  To: git; +Cc: gitster, dzwell, peff, Matthieu.Moy, Wincent Colaiuta
In-Reply-To: <697AB37F-784D-4374-A290-0E6290712B29@wincent.com>

The user interface provided by the command loop in git-add--interactive
gives the impression that subcommands can only be launched by entering
an integer identifier from 1 through 8.

A "hidden" feature is that any string can be entered, and an anchored
regex search is used to find the uniquely matching option.

This patch makes this feature a little more obvious by highlighting the
first character of each subcommand (for example "patch" is displayed as
"[p]atch").

A new function is added to detect the shortest unique prefix and this
is used to decide what to highlight. Highlighting is also applied when
choosing files.

In the case where the common prefix may be unreasonably large
highlighting is omitted; in this patch the soft limit (above which the
highlighting will be omitted for a particular item) is 0 (in other words,
there is no soft limit) and the hard limit (above which highlighting will
be omitted for all items) is 3, but this can be tweaked.

The actual highlighting is done by the highlight_prefix function, which
will enable us to implement ANSI color code-based highlighting (most
likely using underline or boldface) in the future.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---

I will go now and crawl under a rock.

 git-add--interactive.perl |  110 ++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb1e92a..0e358b5 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -44,7 +44,6 @@ my $status_fmt = '%12s %12s %s';
 my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
 
 # Returns list of hashes, contents of each of which are:
-# PRINT:	print message
 # VALUE:	pathname
 # BINARY:	is a binary path
 # INDEX:	is index different from HEAD?
@@ -122,8 +121,6 @@ sub list_modified {
 		}
 		push @return, +{
 			VALUE => $_,
-			PRINT => (sprintf $status_fmt,
-				  $it->{INDEX}, $it->{FILE}, $_),
 			%$it,
 		};
 	}
@@ -159,10 +156,95 @@ sub find_unique {
 	return $found;
 }
 
+# inserts string into trie and updates count for each character
+sub update_trie {
+	my ($trie, $string) = @_;
+	foreach (split //, $string) {
+		$trie = $trie->{$_} ||= {COUNT => 0};
+		$trie->{COUNT}++;
+	}
+}
+
+# returns an array of tuples (prefix, remainder)
+sub find_unique_prefixes {
+	my @stuff = @_;
+	my @return = ();
+
+	# any single prefix exceeding the soft limit is omitted
+	# if any prefix exceeds the hard limit all are omitted
+	# 0 indicates no limit
+	my $soft_limit = 0;
+	my $hard_limit = 3;
+
+	# build a trie modelling all possible options
+	my %trie;
+	foreach my $print (@stuff) {
+		if ((ref $print) eq 'ARRAY') {
+			$print = $print->[0];
+		}
+		elsif ((ref $print) eq 'HASH') {
+			$print = $print->{VALUE};
+		}
+		update_trie(\%trie, $print);
+		push @return, $print;
+	}
+
+	# use the trie to find the unique prefixes
+	for (my $i = 0; $i < @return; $i++) {
+		my $ret = $return[$i];
+		my @letters = split //, $ret;
+		my %search = %trie;
+		my ($prefix, $remainder);
+		my $j;
+		for ($j = 0; $j < @letters; $j++) {
+			my $letter = $letters[$j];
+			if ($search{$letter}{COUNT} == 1) {
+				$prefix = substr $ret, 0, $j + 1;
+				$remainder = substr $ret, $j + 1;
+				last;
+			}
+			else {
+				my $prefix = substr $ret, 0, $j;
+				return ()
+				    if ($hard_limit && $j + 1 > $hard_limit);
+			}
+			%search = %{$search{$letter}};
+		}
+		if ($soft_limit && $j + 1 > $soft_limit) {
+			$prefix = undef;
+			$remainder = $ret;
+		}
+		$return[$i] = [$prefix, $remainder];
+	}
+	return @return;
+}
+
+# filters out prefixes which have special meaning to list_and_choose()
+sub is_valid_prefix {
+	my $prefix = shift;
+	return (defined $prefix) &&
+	    !($prefix =~ /[\s,]/) && # separators
+	    !($prefix =~ /^-/) &&    # deselection
+	    !($prefix =~ /^\d+/) &&  # selection
+	    ($prefix ne '*');        # "all" wildcard
+}
+
+# given a prefix/remainder tuple return a string with the prefix highlighted
+# for now use square brackets; later might use ANSI colors (underline, bold)
+sub highlight_prefix {
+	my $prefix = shift;
+	my $remainder = shift;
+	return $remainder unless defined $prefix;
+	return is_valid_prefix($prefix) ?
+	    "[$prefix]$remainder" :
+	    "$prefix$remainder";
+}
+
 sub list_and_choose {
 	my ($opts, @stuff) = @_;
 	my (@chosen, @return);
 	my $i;
+	my @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};
 
       TOPLOOP:
 	while (1) {
@@ -177,13 +259,21 @@ sub list_and_choose {
 		for ($i = 0; $i < @stuff; $i++) {
 			my $chosen = $chosen[$i] ? '*' : ' ';
 			my $print = $stuff[$i];
-			if (ref $print) {
-				if ((ref $print) eq 'ARRAY') {
-					$print = $print->[0];
-				}
-				else {
-					$print = $print->{PRINT};
-				}
+			my $ref = ref $print;
+			my $highlighted = highlight_prefix(@{$prefixes[$i]})
+			    if @prefixes;
+			if ($ref eq 'ARRAY') {
+				$print = $highlighted || $print->[0];
+			}
+			elsif ($ref eq 'HASH') {
+				my $value = $highlighted || $print->{VALUE};
+				$print = sprintf($status_fmt,
+				    $print->{INDEX},
+				    $print->{FILE},
+				    $value);
+			}
+			else {
+				$print = $highlighted || $print;
 			}
 			printf("%s%2d: %s", $chosen, $i+1, $print);
 			if (($opts->{LIST_FLAT}) &&
-- 
1.5.3.6.953.gdffc

^ permalink raw reply related

* Re: [PATCH 1/2] Highlight keyboard shortcuts in git-add--interactive
From: Wincent Colaiuta @ 2007-12-01 14:15 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: git, gitster, dzwell, peff, Matthieu.Moy
In-Reply-To: <1196518040-85584-1-git-send-email-win@wincent.com>

El 1/12/2007, a las 15:07, Wincent Colaiuta escribió:

> +# filters out prefixes which have special meaning to  
> list_and_choose()
> +sub is_valid_prefix {
> +	my $prefix = shift;
> +	my $valid = (defined $prefix) &&
> +	    !($prefix =~ /[\s,]/) && # separators
> +	    !($prefix =~ /^-/) &&    # deselection
> +	    !($prefix =~ /^\d+/) &&  # selection
> +	    ($prefix ne '*');        # "all" wildcard
> +}

Doh, that's supposed to be:

	return (defined $prefix)...

Not:

	my $valid = (defined $prefix)...

It actually works as is, but I had changed the "return" while working  
on the patch (for debugging) and forgot to change it back afterwards.

And yes, I did proofread the patch before sending it. I just didn't  
notice the first time around.

Cheers,
Wincent

^ permalink raw reply

* Re: how to create v2 patch
From: Björn Steinbrink @ 2007-12-01 14:14 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Tilman Schmidt, git
In-Reply-To: <m3r6i6wm1g.fsf@roke.D-201>

On 2007.12.01 06:02:23 -0800, Jakub Narebski wrote:
> Tilman Schmidt <tilman@imap.cc> writes:
> 
> > Let's say that following the scheme laid out in
> > "Everyday GIT ...", chapter "Individual Developer (Participant)".
> > I have produced a patch, submitted it to LKML, received a few
> > comments, committed appropriate changes to my local git tree,
> > and now want to submit a revised patch. How do I do that?
> 
> If you have original commit and commit with corrections on top of it,
> do a squash rebase using "git rebase -i" (interactive), or do a squash
> merge.
> 
> In the future it would be better to just amend ("git commit --amend")
> original commit (or if you are using StGIT, "stg refresh" it).

For completeness:
To use "git commit --amend" for any but the latest commit, you use
rebase -i, too. Just change the "pick" for the commit you want to amend
to "edit". Rebasing will stop _after_ applying that commit and you can
amend it.

Björn

^ permalink raw reply

* [PATCH 2/2] Teach git-add--interactive to highlight untracked file prefixes
From: Wincent Colaiuta @ 2007-12-01 14:07 UTC (permalink / raw)
  To: git; +Cc: gitster, dzwell, peff, Matthieu.Moy, Wincent Colaiuta
In-Reply-To: <1196518040-85584-1-git-send-email-win@wincent.com>

Tweak the list_and_choose function so that untracked files will
use the automatic prefix detection machinery.

This works because while previously we handled arrays (command
menus), hashes (patch subcommand) now we explicitly handle strings
(add untracked subcommand).

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-add--interactive.perl |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 0fb808f..a1aee21 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -256,21 +256,21 @@ sub list_and_choose {
 		for ($i = 0; $i < @stuff; $i++) {
 			my $chosen = $chosen[$i] ? '*' : ' ';
 			my $print = $stuff[$i];
-			if (ref $print) {
-				if ((ref $print) eq 'ARRAY') {
-					$print = @prefixes ?
-					    highlight_prefix(@{$prefixes[$i]}) :
-					    $print->[0];
-				}
-				else {
-					my $value = @prefixes ?
-					    highlight_prefix(@{$prefixes[$i]}) :
-					    $print->{VALUE};
-					$print = sprintf($status_fmt,
-					    $print->{INDEX},
-					    $print->{FILE},
-					    $value);
-				}
+			my $ref = ref $print;
+			my $highlighted = highlight_prefix(@{$prefixes[$i]})
+			    if @prefixes;
+			if ($ref eq 'ARRAY') {
+				$print = $highlighted || $print->[0];
+			}
+			elsif ($ref eq 'HASH') {
+				my $value = $highlighted || $print->{VALUE};
+				$print = sprintf($status_fmt,
+				    $print->{INDEX},
+				    $print->{FILE},
+				    $value);
+			}
+			else {
+				$print = $highlighted || $print;
 			}
 			printf("%s%2d: %s", $chosen, $i+1, $print);
 			if (($opts->{LIST_FLAT}) &&
-- 
1.5.3.6.953.gdffc

^ permalink raw reply related

* [PATCH 1/2] Highlight keyboard shortcuts in git-add--interactive
From: Wincent Colaiuta @ 2007-12-01 14:07 UTC (permalink / raw)
  To: git; +Cc: gitster, dzwell, peff, Matthieu.Moy, Wincent Colaiuta
In-Reply-To: <7vy7cf87jz.fsf@gitster.siamese.dyndns.org>

The user interface provided by the command loop in git-add--interactive
gives the impression that subcommands can only be launched by entering
an integer identifier from 1 through 8.

A "hidden" feature is that any string can be entered, and an anchored
regex search is used to find the uniquely matching option.

This patch makes this feature a little more obvious by highlighting the
first character of each subcommand (for example "patch" is displayed as
"[p]atch").

A new function is added to detect the shortest unique prefix and this
is used to decide what to highlight. Highlighting is also applied when
choosing files.

In the case where the common prefix may be unreasonably large
highlighting is omitted; in this patch the soft limit (above which the
highlighting will be omitted for a particular item) is 0 (in other words,
there is no soft limit) and the hard limit (above which highlighting will
be omitted for all items) is 3, but this can be tweaked.

The actual highlighting is done by the highlight_prefix function, which
will enable us to implement ANSI color code-based highlighting (most
likely using underline or boldface) in the future.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---

Three things to note:

1. I don't actually find the "[p]atch" highlighting using brackets all
that attractive, especially when highlighting paths. This is
especially true when the common prefix is lengthy. This is why I've
set the "hard limit" to a very low 3 in this patch. I am basically
waiting on the stalled "color" series; once that's in "next" then I'd
like to switch to a highlight style that uses underlining.

2. The follow-up patch tweaks this so that the "Add untracked"
subcommand can benefit from it as well. Junio, you might want to squash
the two patches into one seeing as the second patch just refactors
something in the first patch; but I wanted to send them to the list as
two separate patches, at least for the purposes of review, because they
really are two separate behaviours.

3. I tried to find the patch that Junio sent out earlier allowing
multiple selection in the "Patch" subcommand, to see how this interplays
with that, but I couldn't locate it.

 git-add--interactive.perl |   97 ++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb1e92a..0fb808f 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -44,7 +44,6 @@ my $status_fmt = '%12s %12s %s';
 my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
 
 # Returns list of hashes, contents of each of which are:
-# PRINT:	print message
 # VALUE:	pathname
 # BINARY:	is a binary path
 # INDEX:	is index different from HEAD?
@@ -122,8 +121,6 @@ sub list_modified {
 		}
 		push @return, +{
 			VALUE => $_,
-			PRINT => (sprintf $status_fmt,
-				  $it->{INDEX}, $it->{FILE}, $_),
 			%$it,
 		};
 	}
@@ -159,10 +156,92 @@ sub find_unique {
 	return $found;
 }
 
+# inserts string into trie and updates count for each character
+sub update_trie {
+	my ($trie, $string) = @_;
+	foreach (split //, $string) {
+		$trie = $trie->{$_} ||= {COUNT => 0};
+		$trie->{COUNT}++;
+	}
+}
+
+# returns an array of tuples (prefix, remainder)
+sub find_unique_prefixes {
+	my @stuff = @_;
+	my @return = ();
+
+	# any single prefix exceeding the soft limit is omitted
+	# if any prefix exceeds the hard limit all are omitted
+	# 0 indicates no limit
+	my $soft_limit = 0;
+	my $hard_limit = 3;
+
+	# build a trie modelling all possible options
+	my %trie;
+	foreach my $print (@stuff) {
+		if ((ref $print) eq 'ARRAY') {
+			$print = $print->[0];
+		}
+		elsif ((ref $print) eq 'HASH') {
+			$print = $print->{VALUE};
+		}
+		update_trie(\%trie, $print);
+		push @return, $print;
+	}
+
+	# use the trie to find the unique prefixes
+	for (my $i = 0; $i < @return; $i++) {
+		my $ret = $return[$i];
+		my @letters = split //, $ret;
+		my %search = %trie;
+		my ($prefix, $remainder);
+		my $j;
+		for ($j = 0; $j < @letters; $j++) {
+			my $letter = $letters[$j];
+			if ($search{$letter}{COUNT} == 1) {
+				$prefix = substr $ret, 0, $j + 1;
+				$remainder = substr $ret, $j + 1;
+				last;
+			}
+			else {
+				my $prefix = substr $ret, 0, $j;
+				return ()
+				    if ($hard_limit && $j + 1 > $hard_limit);
+			}
+			%search = %{$search{$letter}};
+		}
+		if ($soft_limit && $j + 1 > $soft_limit) {
+			$prefix = undef;
+			$remainder = $ret;
+		}
+		$return[$i] = [$prefix, $remainder];
+	}
+	return @return;
+}
+
+# filters out prefixes which have special meaning to list_and_choose()
+sub is_valid_prefix {
+	my $prefix = shift;
+	my $valid = (defined $prefix) &&
+	    !($prefix =~ /[\s,]/) && # separators
+	    !($prefix =~ /^-/) &&    # deselection
+	    !($prefix =~ /^\d+/) &&  # selection
+	    ($prefix ne '*');        # "all" wildcard
+}
+
+# given a prefix/remainder tuple return a string with the prefix highlighted
+# for now use square brackets; later might use ANSI colors (underline, bold)
+sub highlight_prefix {
+	my $prefix = shift;
+	my $remainder = shift;
+	is_valid_prefix($prefix) ? "[$prefix]$remainder" : $remainder;
+}
+
 sub list_and_choose {
 	my ($opts, @stuff) = @_;
 	my (@chosen, @return);
 	my $i;
+	my @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};
 
       TOPLOOP:
 	while (1) {
@@ -179,10 +258,18 @@ sub list_and_choose {
 			my $print = $stuff[$i];
 			if (ref $print) {
 				if ((ref $print) eq 'ARRAY') {
-					$print = $print->[0];
+					$print = @prefixes ?
+					    highlight_prefix(@{$prefixes[$i]}) :
+					    $print->[0];
 				}
 				else {
-					$print = $print->{PRINT};
+					my $value = @prefixes ?
+					    highlight_prefix(@{$prefixes[$i]}) :
+					    $print->{VALUE};
+					$print = sprintf($status_fmt,
+					    $print->{INDEX},
+					    $print->{FILE},
+					    $value);
 				}
 			}
 			printf("%s%2d: %s", $chosen, $i+1, $print);
-- 
1.5.3.6.953.gdffc

^ permalink raw reply related

* Re: [PATCH] Highlight keyboard shortcuts in git-add--interactive
From: Wincent Colaiuta @ 2007-12-01 13:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, dzwell, peff, Matthieu.Moy
In-Reply-To: <7vy7cf87jz.fsf@gitster.siamese.dyndns.org>


El 1/12/2007, a las 3:36, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> A "hidden" feature is that any string can be entered, and an anchored
>> regex search is used to find the first matching option.
>
> I'd run s/the first/the uniquely/ here.
>
> When list_and_choose() function is letting you choose more than one
> items, its prompt becomes ">> ", instead of "> " that is used for a
> singleton choice.  To that prompt, you can say "3-7" (Add these 5  
> items
> to the choice), "*" (I want all of them), "-2-4" (exclude 2 and 3  
> and 4
> from the set I have chosen so far).  These are also "hidden", and need
> to be documented, but that would be a separate patch.

Agreed that it belongs in a separate patch.

But I'm glad you brought this up as it reminds me of the need to watch  
out for those characters which have special meaning for  
list_and_choose().

> I'd rewrite the last line to:
>
> 	return (defined $prefix) ? "[$prefix]$remainder" : $remainder;
>
> just in case the unique prefix is "0".  Otherwise you would lose the
> first letter from "00ReadMe" and show remainder "0ReadMe" alone.

Excellent catch. Crazy old perl; I didn't realize that "0" (the  
string, not the number) would evaluate to false.

Will send a separate mail with a revised, squashed patch with these  
changes:

- "s/the first/the uniquely/" in the commit message as you suggest

- filter out prefixes which contain characters with special meaning  
for list_and_choose()

- check "defined $prefix" rather than just "$prefix"

- also fixes a problem discovered while playing with this; it didn't  
play nicely with untracked files

Cheers,
Wincent

^ permalink raw reply

* Re: how to create v2 patch
From: Jakub Narebski @ 2007-12-01 14:02 UTC (permalink / raw)
  To: Tilman Schmidt; +Cc: git
In-Reply-To: <47515693.9070405@imap.cc>

Tilman Schmidt <tilman@imap.cc> writes:

> Let's say that following the scheme laid out in
> "Everyday GIT ...", chapter "Individual Developer (Participant)".
> I have produced a patch, submitted it to LKML, received a few
> comments, committed appropriate changes to my local git tree,
> and now want to submit a revised patch. How do I do that?

If you have original commit and commit with corrections on top of it,
do a squash rebase using "git rebase -i" (interactive), or do a squash
merge.

In the future it would be better to just amend ("git commit --amend")
original commit (or if you are using StGIT, "stg refresh" it).

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: cannot see git-fetch result in gitk
From: Jakub Narebski @ 2007-12-01 13:55 UTC (permalink / raw)
  To: Tilman Schmidt; +Cc: git
In-Reply-To: <475154DB.4040606@imap.cc>

Tilman Schmidt <tilman@imap.cc> writes:

> I'm still learning my way around git and trying to understand how it
> works. So after reading
>   howto/rebase-from-internal-branch.txt
> I wanted to try and understand the command sequence
> 
> git fetch origin
> git rebase FETCH_HEAD
> 
> by looking at what it does with gitk. But after the first command I do
> not see any change at all in gitk. Only after the second one do the
> newly fetched objects appear in the gitk display.
> 
> Simple question: why?

Did you run just "gitk"? By default gitk displays only current branch,
while git-fetch changes remote-tracking branches only. Try 
"gitk --all", or "gitk origin/master ..." enumerating explicitely all
remote branches.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: how to create v2 patch
From: Mike Hommey @ 2007-12-01 13:43 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Tilman Schmidt, git
In-Reply-To: <47515EF3.8010507@obry.net>

On Sat, Dec 01, 2007 at 02:17:39PM +0100, Pascal Obry wrote:
> Tilman Schmidt a écrit :
> > Let's say that following the scheme laid out in
> > http://www.kernel.org/pub/software/scm/git/docs/everyday.html#Individual%20Developer%20(Participant)
> > I have produced a patch, submitted it to LKML, received a few
> > comments, committed appropriate changes to my local git tree,
> > and now want to submit a revised patch. How do I do that?
> > If I just run git-format-patch again, it produces my original
> > patch plus a second one containing my updates, but what I need
> > is a single new patch replacing the first one.
> 
> Can't you merge both of your changes in your local repository? I would
> do that with an interactive rebase.

Or just git commit --amend when committing.

Mike

^ permalink raw reply

* Re: how to create v2 patch
From: Pascal Obry @ 2007-12-01 13:17 UTC (permalink / raw)
  To: Tilman Schmidt; +Cc: git
In-Reply-To: <47515693.9070405@imap.cc>

Tilman Schmidt a écrit :
> Let's say that following the scheme laid out in
> http://www.kernel.org/pub/software/scm/git/docs/everyday.html#Individual%20Developer%20(Participant)
> I have produced a patch, submitted it to LKML, received a few
> comments, committed appropriate changes to my local git tree,
> and now want to submit a revised patch. How do I do that?
> If I just run git-format-patch again, it produces my original
> patch plus a second one containing my updates, but what I need
> is a single new patch replacing the first one.

Can't you merge both of your changes in your local repository? I would
do that with an interactive rebase.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* how to create v2 patch
From: Tilman Schmidt @ 2007-12-01 12:41 UTC (permalink / raw)
  To: git

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

Let's say that following the scheme laid out in
http://www.kernel.org/pub/software/scm/git/docs/everyday.html#Individual%20Developer%20(Participant)
I have produced a patch, submitted it to LKML, received a few
comments, committed appropriate changes to my local git tree,
and now want to submit a revised patch. How do I do that?
If I just run git-format-patch again, it produces my original
patch plus a second one containing my updates, but what I need
is a single new patch replacing the first one.

Thanks,
Tilman

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Yes, I have searched Google!


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

^ permalink raw reply

* cannot see git-fetch result in gitk
From: Tilman Schmidt @ 2007-12-01 12:34 UTC (permalink / raw)
  To: git

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

I'm still learning my way around git and trying to understand how it
works. So after reading
http://www.kernel.org/pub/software/scm/git/docs/howto/rebase-from-internal-branch.txt
I wanted to try and understand the command sequence

git fetch origin
git rebase FETCH_HEAD

by looking at what it does with gitk. But after the first command I do
not see any change at all in gitk. Only after the second one do the
newly fetched objects appear in the gitk display.

Simple question: why?

Thanks,
Tilman

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Yes, I have searched Google!


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

^ permalink raw reply

* Re: Some git performance measurements..
From: Joachim B Haga @ 2007-12-01 11:36 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.LFD.0.9999.0711281852160.8458@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> The pack-files (both index and data) are accessed somewhat randomly, but 
> there is still enough locality that doing read-ahead and clustering really 
> does help.

They are dense enough that slurping them in whole is 20% faster, at 
least here. And much less noisy! These are both cache-cold tests.

$ time git read-tree -m -u HEAD HEAD

real    0m9.255s
user    0m0.832s
sys     0m0.196s

$ time (cat .git/objects/pack/* .git/index >/dev/null; git read-tree -m -u HEAD HEAD)

real    0m7.141s
user    0m0.936s
sys     0m1.912s


Now, I don't know how useful this is since git doesn't know if the
data are cached. Is it perhaps possible to give a hint to the
readahead logic that it should try to read as far as possible?


-j.

^ permalink raw reply

* git pack-objects input list
From: Mike Hommey @ 2007-12-01 10:45 UTC (permalink / raw)
  To: git

Hi,

While playing around with git-pack-objects, it seemed to me that the
input it can take is not a simple list of object SHA1s. Unfortunately,
the man page is not very verbose about that. While I'd happily send a
patch for that, I'd prefer to actually know what kind of input it can
take, and what it uses it for.

AFAICT, it can take the output of git-rev-list --all --objects (so,
SHA1s followed by file names for blobs), which seems to be the same as
what git-pack-objects --revs does internally, but it seems to have a
string impact on how deltas are calculated (not giving file names makes
it create a smaller pack in some cases, a bigger one in other cases).

Could someone knowing the delta calculation internals enlighten me ?

Thanks

Mike

^ permalink raw reply

* Re: [PATCH] transport.c: call dash-less form of receive-pack and upload-pack on remote
From: Johannes Schindelin @ 2007-12-01 10:17 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eyvind Bernhardsen, Nicolas Pitre, Nguyen Thai Ngoc Duy,
	Jan Hudec, git
In-Reply-To: <7vlk8f9m52.fsf@gitster.siamese.dyndns.org>

Hi,

On Fri, 30 Nov 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Since we plan to move the dash-form (git-<whatever>) into an execdir, it
> > make sense to prepare our git protocol users for it.
> >
> > Noticed by Eyvind Bernhardsen.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > 	On Fri, 30 Nov 2007, Eyvind Bernhardsen wrote:
> >
> > 	> - When pushing to my system over ssh, git-receive-pack and
> > 	> git-upload-pack are expected to be in $PATH.  I resolved the 
> > 	> problem by putting symlinks in /usr/local/bin.
> >
> > 	How about this?  (I only compile-tested it...)
> 
> I only eyeball-tested it and looks Okay, but that does not assure us
> much ;-).  Is this change easy to test using local transport?

Seems like it breaks down with git-shell.  But then, I think that we just 
have to fix execv_git_cmd() to call builtins via "git" instead.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Set OLD_ICONV on Cygwin.
From: Pascal Obry @ 2007-12-01  9:49 UTC (permalink / raw)
  To: git; +Cc: Pascal Obry

Cygwin still has old definition for the iconv() second
parameter. This patch fixes the last warning on Cygwin.
This has been tested with Cygwin 1.5.24.

Signed-off-by: Pascal Obry <pascal@obry.net>
---
 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index e869b85..fa3398c 100644
--- a/Makefile
+++ b/Makefile
@@ -446,6 +446,7 @@ ifeq ($(uname_O),Cygwin)
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
 	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
+	OLD_ICONV = UnfortunatelyYes
 	# There are conflicting reports about this.
 	# On some boxes NO_MMAP is needed, and not so elsewhere.
 	# Try commenting this out if you suspect MMAP is more efficient
-- 
1.5.3.6.985.g65c6a4

^ permalink raw reply related

* Re: [PATCH] git-cvsserver runs hooks/post-receive
From: Michael Witten @ 2007-12-01  9:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmysv87jm.fsf@gitster.siamese.dyndns.org>


On 30 Nov 2007, at 9:37:01 PM, Junio C Hamano wrote:

> I'll queue your patch, but I think it should be enhanced to support
> post-update for consistency.

I'll send another patch that includes support for post-update.

> I'll queue your patch,

Will the old patch already be in place?

Also, I explicitly decided to pipe input into post-receive
by hand rather than relying on a system() call that someone
might exploit maliciously.

Unfortunately, it turns out that open() with a pipe essentially
invokes system(); the solution is to fork a child process and
then to turn the child into the process with which communication
is desired via a call to exec().

Because the rest of git-cvsserver.perl uses explicit system()
calls, I have been wondering if I am being overly cautious.

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Eric Wong @ 2007-12-01  8:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Steven Grimm
In-Reply-To: <7vzlwv6sxr.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> * ew/svn (Thu Nov 22 13:44:42 2007 +0000) 4 commits
>  - git-svn: add support for pulling author from From: and Signed-off-
>    by:
>  - git-svn: add a show-externals command.
>  - git-svn now reads settings even if called in subdirectory
>  - git-svn: Remove unnecessary Git::SVN::Util package
> 
> Picked up from the list with Eric's Acks, but haven't merged, as my next
> pull from Eric would hopefully bring them in anyway.

Hi,

I've pushed the following out to git://git.bogomips.org/git-svn.git ,
along with Steven's patch:

Andy Whitcroft (1):
      git-svn: add support for pulling author from From: and Signed-off-by:

David D. Kilzer (1):
      git-svn: Remove unnecessary Git::SVN::Util package

Gustaf Hendeby (1):
      git-svn now reads settings even if called in subdirectory

Steven Grimm (1):
      git-svn: Don't create a "master" branch every time rebase is run

Vineet Kumar (1):
      git-svn: add a show-externals command.


-- 
Eric Wong

^ permalink raw reply

* Re: git guidance
From: Al Boldi @ 2007-12-01  6:50 UTC (permalink / raw)
  To: Linus Torvalds, Jing Xue; +Cc: linux-kernel, git
In-Reply-To: <alpine.LFD.0.9999.0711290810170.8458@woody.linux-foundation.org>

Jing Xue wrote:
> Quoting Al Boldi <a1426z@gawab.com>:
> > Sure, browsing is the easy part, but Version Control starts when things
> > become writable.
>
> But how is that supposed to work?  What happens when you make some
> changes to a file and save it?  Do you want the "git file system" to
> commit it right aways or wait until you to issue a "commit" command?
> The first behavior would obviously be wrong, and the second would make
> the "file system" not operationally transparent anyways. Right?

Not sure what you mean by operationally transparent?  It would be transparent 
for the updating client,  and the rest of the git-users would need to wait 
for the commit from the updating client; which is ok, as this transparency 
is not meant to change the server-side git-update semantic.

Linus Torvalds wrote:
> On Thu, 29 Nov 2007, Jing Xue wrote:
> > By the way, the only SCM I have worked with that tries to mount its
> > repository (or a view on top of it) as a file system is ClearCase with
> > its dynamic views. And, between the buggy file system implementation,
> > the intrusion on workflow, and the lack of scalability, at least in
> > the organization I worked for, it turned out to be a horrible,
> > horrible, horrible idea.

Judging an idea, based on a flawed implementation, doesn't prove that the 
idea itself is flawed.

And...
> Doing a read-only mount setup tends to be pretty easy, but it's largely
> pointless except for specialty uses. Ie it's obviously not useful for
> actual *development*, but it can be useful for some other cases.
>
> For example, a read-only revctrl filesystem can be a _very_ useful thing
> for test-farms, where you may have hundreds of clients that run tests on
> possibly different versions at the same time. In situations like that, the
> read-only mount can actually often be done as a user-space NFS server on
> some machine.
>
> The advantage is that you don't need to export close to infinite amounts
> of versions from a "real" filesystem, or make the clients have their own
> copies. And if you do it as a user-space NFS server (or samba, for that
> matter), it's even portable, unlike many other approaches. The read-only
> part also makes 99% of all the complexity go away, and it turns out to be
> a fairly easy exercise to do.
>
> So I don't think the filesystem approach is _wrong_ per se. But yes, doing
> it read-write is almost invariably a big mistake. On operatign systems
> that support a "union mount" approach, it's likely much better to have a
> read-only revctl thing, and then over-mount a regular filesystem on top of
> it.

You could probably do that, or you could instead use cp -al.  Both would 
require some hacks to allow some basic version control.

> Trying to make it read-write from the revctl engine standpoint is almost
> certainly totally insane.

Sure, you wouldn't want to change the git-engine update semantics, as that 
sits on the server and handles all users.  But what the git model is 
currently missing is a client manager.  Right now, this is being worked 
around by replicating the git tree on the client, which still doesn't 
provide the required transparency.

IOW, git currently only implements the server-side use-case, but fails to 
deliver on the client-side.  By introducing a git-client manager that 
handles the transparency needs of a single user, it should be possible to 
clearly isolate update semantics for both the client and the server, each 
handling their specific use-case.


Thanks!

--
Al

^ permalink raw reply

* Re: [PATCH] Do check_repository_format() early
From: Nguyen Thai Ngoc Duy @ 2007-12-01  6:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v3aunb0q4.fsf@gitster.siamese.dyndns.org>

On Dec 1, 2007 9:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Thu, 29 Nov 2007, Nguyen Thai Ngoc Duy wrote:
> >
> >> The comment is clearly not clear enough. Maybe this?
> >>
> >> +                     if (!work_tree_env) {
> >> +                             retval = set_work_tree(gitdirenv);
> >> +                             /* config may override worktree (see
> >> set_work_tree comment) */
> >> +                             check_repository_format();
> >> +                             return retval;
> >> +                     }
> >
> > Perfect.  Please make it so, and add my ACK.
>
> Looks sensible, but can this be accompanied with a trivial test to
> demonstrate the existing breakage?
>

How can I reliably check setup_git_directory_gently()? I can pick one
command that uses setup_git_directory_gently(). But commands change.
Once they turn to setup_git_directory(), the test will no longer be
valid.

-- 
Duy

^ permalink raw reply

* Re: [RFC] use typechange as rename source
From: Jeff King @ 2007-12-01  6:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7ce6hyb.fsf@gitster.siamese.dyndns.org>

On Fri, Nov 30, 2007 at 10:35:08PM -0800, Junio C Hamano wrote:

> Sorry for the earlier broken one.  Here is the correct one I wanted to
> send out.
>
> By the way, this breaks some tests in t4008 (break-rewrite test) because
> the test expects the behaviour you found counterintuitive (which was the
> reason we have this thread).

OK, this looks reasonable to me, and it produces diff results (with -B)
that make sense for typechanges. Looking at the failing test in t4008, I
like the new behavior better.

And maybe something like this on top for git-status?

diff --git a/wt-status.c b/wt-status.c
index 0e0439f..e77120d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -250,6 +250,7 @@ static void wt_status_print_updated(struct wt_status *s)
 	rev.diffopt.format_callback_data = s;
 	rev.diffopt.detect_rename = 1;
 	rev.diffopt.rename_limit = 100;
+	rev.diffopt.break_opt = 0;
 	wt_read_cache(s);
 	run_diff_index(&rev, 1);
 }

^ permalink raw reply related

* Re: [RFC] use typechange as rename source
From: Junio C Hamano @ 2007-12-01  6:35 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <7v8x4f6iyu.fsf@gitster.siamese.dyndns.org>

Sorry for the earlier broken one.  Here is the correct one I wanted to
send out.

By the way, this breaks some tests in t4008 (break-rewrite test) because
the test expects the behaviour you found counterintuitive (which was the
reason we have this thread).

-- >8 --
Subject: rename: Break filepairs with different types.

When we consider if a path has been totally rewritten, we did not
touch changes from symlinks to files or vice versa.  But a change
that modifies even the type of a blob surely should count as a
complete rewrite.

While we are at it, modernise diffcore-break to be aware of gitlinks (we
do not want to touch them).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h                           |    7 +++
 diffcore-break.c                  |   12 +++--
 t/t4023-diff-rename-typechange.sh |   86 +++++++++++++++++++++++++++++++++++++
 tree-walk.h                       |    7 ---
 4 files changed, 101 insertions(+), 11 deletions(-)
 create mode 100755 t/t4023-diff-rename-typechange.sh

diff --git a/cache.h b/cache.h
index aaa135b..d0e7a71 100644
--- a/cache.h
+++ b/cache.h
@@ -192,6 +192,13 @@ enum object_type {
 	OBJ_MAX,
 };
 
+static inline enum object_type object_type(unsigned int mode)
+{
+	return S_ISDIR(mode) ? OBJ_TREE :
+		S_ISGITLINK(mode) ? OBJ_COMMIT :
+		OBJ_BLOB;
+}
+
 #define GIT_DIR_ENVIRONMENT "GIT_DIR"
 #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
 #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
diff --git a/diffcore-break.c b/diffcore-break.c
index c71a226..31cdcfe 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -52,8 +52,10 @@ static int should_break(struct diff_filespec *src,
 			     * is the default.
 			     */
 
-	if (!S_ISREG(src->mode) || !S_ISREG(dst->mode))
-		return 0; /* leave symlink rename alone */
+	if (S_ISREG(src->mode) != S_ISREG(dst->mode)) {
+		*merge_score_p = (int)MAX_SCORE;
+		return 1; /* even their types are different */
+	}
 
 	if (src->sha1_valid && dst->sha1_valid &&
 	    !hashcmp(src->sha1, dst->sha1))
@@ -168,11 +170,13 @@ void diffcore_break(int break_score)
 		struct diff_filepair *p = q->queue[i];
 		int score;
 
-		/* We deal only with in-place edit of non directory.
+		/*
+		 * We deal only with in-place edit of blobs.
 		 * We do not break anything else.
 		 */
 		if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two) &&
-		    !S_ISDIR(p->one->mode) && !S_ISDIR(p->two->mode) &&
+		    object_type(p->one->mode) == OBJ_BLOB &&
+		    object_type(p->two->mode) == OBJ_BLOB &&
 		    !strcmp(p->one->path, p->two->path)) {
 			if (should_break(p->one, p->two,
 					 break_score, &score)) {
diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh
new file mode 100755
index 0000000..255604e
--- /dev/null
+++ b/t/t4023-diff-rename-typechange.sh
@@ -0,0 +1,86 @@
+#!/bin/sh
+
+test_description='typechange rename detection'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	rm -f foo bar &&
+	cat ../../COPYING >foo &&
+	ln -s linklink bar &&
+	git add foo bar &&
+	git commit -a -m Initial &&
+	git tag one &&
+
+	rm -f foo bar &&
+	cat ../../COPYING >bar &&
+	ln -s linklink foo &&
+	git add foo bar &&
+	git commit -a -m Second &&
+	git tag two &&
+
+	rm -f foo bar &&
+	cat ../../COPYING >foo &&
+	git add foo &&
+	git commit -a -m Third &&
+	git tag three &&
+
+	mv foo bar &&
+	ln -s linklink foo &&
+	git add foo bar &&
+	git commit -a -m Fourth &&
+	git tag four &&
+
+	# This is purely for sanity check
+
+	rm -f foo bar &&
+	cat ../../COPYING >foo &&
+	cat ../../Makefile >bar &&
+	git add foo bar &&
+	git commit -a -m Fifth &&
+	git tag five &&
+
+	rm -f foo bar &&
+	cat ../../Makefile >foo &&
+	cat ../../COPYING >bar &&
+	git add foo bar &&
+	git commit -a -m Sixth &&
+	git tag six
+
+'
+
+test_expect_success 'cross renames to be detected for regular files' '
+
+	git diff-tree five six -r --name-status -B -M | sort >actual &&
+	{
+		echo "R100	foo	bar"
+		echo "R100	bar	foo"
+	} | sort >expect &&
+	diff -u expect actual
+
+'
+
+test_expect_success 'cross renames to be detected for typechange' '
+
+	git diff-tree one two -r --name-status -B -M | sort >actual &&
+	{
+		echo "R100	foo	bar"
+		echo "R100	bar	foo"
+	} | sort >expect &&
+	diff -u expect actual
+
+'
+
+test_expect_success 'moves and renames' '
+
+	git diff-tree three four -r --name-status -B -M | sort >actual &&
+	{
+		echo "R100	foo	bar"
+		echo "T100	foo"
+	} | sort >expect &&
+	diff -u expect actual
+
+'
+
+test_done
diff --git a/tree-walk.h b/tree-walk.h
index 903a7b0..db0fbdc 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -7,13 +7,6 @@ struct name_entry {
 	unsigned int mode;
 };
 
-static inline enum object_type object_type(unsigned int mode)
-{
-	return S_ISDIR(mode) ? OBJ_TREE :
-		S_ISGITLINK(mode) ? OBJ_COMMIT :
-		OBJ_BLOB;
-}
-
 struct tree_desc {
 	const void *buffer;
 	struct name_entry entry;
-- 
1.5.3.6.2090.g4ece0

^ permalink raw reply related

* Re: [RFC] use typechange as rename source
From: Jeff King @ 2007-12-01  6:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhcj36j6e.fsf@gitster.siamese.dyndns.org>

On Fri, Nov 30, 2007 at 10:08:41PM -0800, Junio C Hamano wrote:

> I think it does make a difference, if you have a cross rename that
> swaps:
> 
> 	$ ls -F foo bar
>         bar	foo@
>         $ mv foo tmp; mv bar foo; mv tmp bar
>         $ ls -F foo bar
>         bar@	foo

OK, I see now what you were saying before. Yes, we do want them actually
broken, and my initial patch is not right in that sense.

> > -	if (!S_ISREG(src->mode) || !S_ISREG(dst->mode))
> > -		return 0; /* leave symlink rename alone */
> > +	if (object_type(src->mode) != object_type(dst->mode))
> > +		return 1; /* even their types are different */
> 
> Oops, this part is wrong.  It should be checking ISREG and stuff.

OK, good. I was sitting there puzzling over what in the world this
change meant.

-Peff

^ permalink raw reply

* Re: [RFC] use typechange as rename source
From: Junio C Hamano @ 2007-12-01  6:13 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <7vhcj36j6e.fsf@gitster.siamese.dyndns.org>

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

> diff --git a/diffcore-break.c b/diffcore-break.c
> index c71a226..69afc07 100644
> --- a/diffcore-break.c
> +++ b/diffcore-break.c
> @@ -52,8 +52,8 @@ static int should_break(struct diff_filespec *src,
>  			     * is the default.
>  			     */
>  
> -	if (!S_ISREG(src->mode) || !S_ISREG(dst->mode))
> -		return 0; /* leave symlink rename alone */
> +	if (object_type(src->mode) != object_type(dst->mode))
> +		return 1; /* even their types are different */

Oops, this part is wrong.  It should be checking ISREG and stuff.

^ permalink raw reply

* Re: [PATCH/RFC] "color.diff = true" is not "always" anymore.
From: Junio C Hamano @ 2007-12-01  6:10 UTC (permalink / raw)
  To: Jeff King; +Cc: git list
In-Reply-To: <20071201041549.GB30725@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Nov 30, 2007 at 06:36:44PM -0800, Junio C Hamano wrote:
>
>> > It would be nice to have a "git config --colorbool" option, but it has
>> > the unfortunate problem that the stdout of "git config" is piped back to
>> > the caller, so the isatty check is meaningless (and the "pager in use"
>> > is similarly tricky). Perhaps it should go in Git.pm, so it at least
>> > only needs to be written once.
>> 
>> About the isatty(3) check, you do not have to use the stdout to report
>> the result, though.  IOW, you could use the exit code from the command.
>
> I thought about that, but it feels a little wrong since it is so unlike
> all of the other interfaces to git-config.

Yeah, that is why I did not seriously suggest it.  The message you were
responding to was sitting in my "I do not know if this should go out"
box for a few days and was sent out purely by accident ;-)

^ 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