Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Stop making git-merge-recur
From: A Large Angry SCM @ 2006-10-02  4:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vu02nz5w6.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
> 
>> Git-merge-recur is no longer installed and make clean no longer
>> removes it, so creating making it.
> 
> Thanks for noticing.
> 
> The bug is actually not installing nor cleaning it.  People who
> wanted to stay on the bleeding edge could add "-s recur" on the
> command line, or have "merge.twohead = recur" in the config;
> this is what we actually encouraged people for wider testing.
> We do not suddenly break them.  I planned the removal for the
> release after 1.4.3.
> 
> This should fix it -- could you eyeball it please?

+1

^ permalink raw reply

* Re: [BUG] 'git mv a/fileA b/fileB' causes 'c/fileC' to be deleted
From: Junio C Hamano @ 2006-10-02  5:05 UTC (permalink / raw)
  To: Michael Cassar; +Cc: git
In-Reply-To: <750170aa0610011833n39271704q349d86af76832783@mail.gmail.com>

Thanks.  I see it reproduces.

It is a bug in git-mv that fails to update cache-tree data
structure properly.  This patch should fix it.

-- >8 --
git-mv: do not write out a bogus cache-tree in the index

The command updates the cache without invalidating the cache
tree entries.  Since this is not as performance critical as
one-tree and two-tree git-read-tree and git-apply, disable
use of cache-tree entirely.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff --git a/builtin-mv.c b/builtin-mv.c
index 4d21d88..9a4e2e0 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -83,6 +83,9 @@ int cmd_mv(int argc, const char **argv, 
 	if (read_cache() < 0)
 		die("index file corrupt");
 
+	/* This does not manage cache-tree properly */
+	cache_tree_free(&active_cache_tree);
+
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 

^ permalink raw reply related

* Re: [BUG] 'git mv a/fileA b/fileB' causes 'c/fileC' to be deleted
From: Junio C Hamano @ 2006-10-02  5:20 UTC (permalink / raw)
  To: Michael Cassar; +Cc: git
In-Reply-To: <7vven3xq7l.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Thanks.  I see it reproduces.
>
> It is a bug in git-mv that fails to update cache-tree data
> structure properly.  This patch should fix it.

The previous patch was safer but unnecessarily was a bit heavy
handed.  This should fix it nicer.

-- >8 --
git-mv: invalidate the removed path properly in cache-tree

The command updated the cache without invalidating the cache
tree entries while removing an existing entry.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff --git a/builtin-mv.c b/builtin-mv.c
index 4d21d88..54dd3bf 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -278,6 +278,7 @@ int cmd_mv(int argc, const char **argv, 
 		for (i = 0; i < deleted.nr; i++) {
 			const char *path = deleted.items[i].path;
 			remove_file_from_cache(path);
+			cache_tree_invalidate_path(active_cache_tree, path);
 		}
 
 		if (active_cache_changed) {

^ permalink raw reply related

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Andreas Ericsson @ 2006-10-02  7:15 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, Luben Tuikov, git
In-Reply-To: <200610012127.37113.jnareb@gmail.com>

Jakub Narebski wrote:
> Junio C Hamano wrote:
> 
>> Doesn't leading drwxr-xr-x mean anything?
> 
> Neither d in drwxr-xr-x, nor trailing / in the name are significant
> enough indicators of directory vs file (tree vs blob) indicator,
> especially that by default git-ls-tree output mixes trees with blobs.
>  

I beg to differ. The trailing slash would indeed help, as peoples eyes 
are generally focused on the mouse-pointer rather than some 5-10" to the 
left of it. If we're worrying that windows folks use \ for 
path-delimiter, I think they'll pick up the difference quick enough.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Junio C Hamano @ 2006-10-02  7:34 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <7vd59b27c4.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Jakub Narebski <jnareb@gmail.com> writes:
>
>> ... That means that we cannot distinguish really well (at 
>> least color) between tree and blob entries.
>
> Do we even say links are blue and underlined by forcing that in
> our css?
>
> Doesn't leading drwxr-xr-x mean anything?
>
> Why is making the distinction important in the first place?

Anyhow, I was too tired to sleep after an unscheduled day-job on
Sunday X-<, and whipped this up for fun.

-- >8 --
[PATCH] gitweb: remove UNIXy mode bits from tree display

and replace it with an image icon for cuteness ;-).

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 gitweb/gitweb.css  |   20 ++++++++++++++++++++
 gitweb/gitweb.perl |   21 ++++++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index eb9fc38..c4aaf7c 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -211,6 +211,26 @@ td.selflink {
 	padding-right: 0px;
 }
 
+td.executable {
+  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAACVJREFUCNdjYGBgYGdgMEhgUFBgYGJgcBKAIiAbKAIUB8oyMAAANBcCqbivEbgAAAAASUVORK5CYII=);
+  background-repeat: no-repeat;
+}
+
+td.folder {
+  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAAB1JREFUCNdjYGBgkGFgUGJgcPzA4CCABdV/ACoBAFTTBQ822ZerAAAAAElFTkSuQmCC);
+  background-repeat: no-repeat;
+}
+
+td.regular {
+  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAABtJREFUCNdjqD/A4JDA4BDA4FDB4MCBHdX/AACO5wbfUNnbqwAAAABJRU5ErkJggg==);
+  background-repeat: no-repeat;
+}
+
+td.symlink {
+  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAACVJREFUCNdjYGBg4G9gMEhgUFBgcBAAIQYYYlFg4ElgkG8AKgEARSsDX750+Y0AAAAASUVORK5CYII=);
+  background-repeat: no-repeat;
+}
+
 td.sha1 {
 	font-family: monospace;
 }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7e4ec8d..8c54a5d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -567,6 +567,23 @@ sub age_string {
 }
 
 # convert file mode in octal to symbolic file mode string
+sub kind_class {
+	my ($type, $mode) = @_;
+	$mode = oct $mode;
+	if (S_ISDIR($mode & S_IFMT)) {
+		return 'folder';
+	} elsif (S_ISLNK($mode)) {
+		return 'symlink';
+	} elsif (S_ISREG($mode)) {
+		# git cares only about the executable bit
+		if ($mode & S_IXUSR) {
+			return 'executable';
+		} else {
+			return 'regular';
+		};
+	}
+}
+
 sub mode_str {
 	my $mode = oct shift;
 
@@ -1651,7 +1668,9 @@ sub git_print_tree_entry {
 	# the mode of the entry, list is the name of the entry, an href,
 	# and link is the action links of the entry.
 
-	print "<td class=\"mode\">" . mode_str($t->{'mode'}) . "</td>\n";
+	my $kind = kind_class($t->{'type'}, $t->{'mode'});
+	print "<td class=\"$kind\">&nbsp;</td>\n";
+
 	if ($t->{'type'} eq "blob") {
 		print "<td class=\"list\">" .
 			$cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},
-- 
1.4.2.2.g91c5

^ permalink raw reply related

* Re: ANSWER
From: Junio C Hamano @ 2006-10-02  7:40 UTC (permalink / raw)
  To: Alan Chandler; +Cc: git
In-Reply-To: <200610011253.03169.alan@chandlerfamily.org.uk>

Alan Chandler <alan@chandlerfamily.org.uk> writes:

> On Sunday 01 October 2006 11:38, Alan Chandler wrote:
>
>> I have a .git/remotes/public file which contains
>>
>> URL:roo.home:/var/lib/git/akclib.git
>> Push:master
>
> The URL: and Push: need spaces before the detail
>
> This used to work, so somewhere along the line (when the builtin was 
> introduced?) the need for the space has arisen.

True.  I did not notice it before but this indeed is a breakage
when builtin-push.c was done.

Totally untested, but it looks obvious enough...

-- >8 --
git-push: .git/remotes/ file does not require SP after colon

Although most people would have one after colon if only for
readability, we never required it in git-parse-remote, so let's
not require one only in git-push.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/builtin-push.c b/builtin-push.c
index c43f256..f5150ed 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -78,12 +78,12 @@ static int get_remotes_uri(const char *r
 		int is_refspec;
 		char *s, *p;
 
-		if (!strncmp("URL: ", buffer, 5)) {
+		if (!strncmp("URL:", buffer, 4)) {
 			is_refspec = 0;
-			s = buffer + 5;
-		} else if (!strncmp("Push: ", buffer, 6)) {
+			s = buffer + 4;
+		} else if (!strncmp("Push:", buffer, 5)) {
 			is_refspec = 1;
-			s = buffer + 6;
+			s = buffer + 5;
 		} else
 			continue;
 

^ permalink raw reply related

* [ANNOUNCE] GIT 1.4.2.3
From: Junio C Hamano @ 2006-10-02  8:17 UTC (permalink / raw)
  To: git; +Cc: linux-kernel

The latest maintenance release GIT 1.4.2.3 is available at the
usual places:

  http://www.kernel.org/pub/software/scm/git/

  git-1.4.2.3.tar.{gz,bz2}			(tarball)
  git-htmldocs-1.4.2.3.tar.{gz,bz2}		(preformatted docs)
  git-manpages-1.4.2.3.tar.{gz,bz2}		(preformatted docs)
  RPMS/$arch/git-*-1.4.2.3-1.$arch.rpm	(RPM)

Sorry to be doing two maintenance releases in rapid succession,
but git-mv breakage causes random tree corruption and is rather
serious.

----------------------------------------------------------------

Changes since v1.4.2.2 are as follows:

Junio C Hamano:
      git-mv: invalidate the removed path properly in cache-tree
      git-push: .git/remotes/ file does not require SP after colon

^ permalink raw reply

* problem with git-cvsimport
From: picca @ 2006-10-02  9:15 UTC (permalink / raw)
  To: git

Hello

I tryed to import this sourceforge project with git-cvsimport
The command was:

git-cvsimport -v -d :pserver:anonymous@tango-cs.cvs.sourceforge.net:/cvsroot/tango-cs -C tango.git tango

unfortunetly the command stop with this error:

skip patchset 3713: 1159522283 before 1159522323
skip patchset 3714: 1159522323 before 1159522323
DONE.
fatal: Needed a single revision
fatal: Needed a single revision
Usage: /usr/bin/git-merge [-n] [--no-commit] [--squash] [-s <strategy>]... <merg
e-message> <head> <remote>+
Could not merge origin into the current branch.

I am using the debian testing git-core version aka 1.4.1.1

Why ?


Have a nice day.

^ permalink raw reply

* Re: Multiple checkouts of the same repository
From: Johannes Schindelin @ 2006-10-02 10:16 UTC (permalink / raw)
  To: Matt McCutchen; +Cc: git
In-Reply-To: <3bbc18d20610011404x52d8ba15ib51ac2a7060bd25e@mail.gmail.com>

Hi,

On Sun, 1 Oct 2006, Matt McCutchen wrote:

> On 10/1/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > The fact is, your setup is fragile, and I think that is hard to fix if you
> > do not make A and B git repositories in their own right.
> 
> I think my setup is much simpler than having two separate repositories
> that update each other using hooks.  What can go wrong, besides the
> scenario I mentioned where one working tree doesn't get updated when I
> commit through the other one?

I do not know, but it feels wrong. Git does not expect something like 
this, much like an operating system does not expect another computer to 
access its root hard disk. So _I_ think your setup is fragile.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Jakub Narebski @ 2006-10-02 10:56 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git
In-Reply-To: <4520BC84.5090800@op5.se>

Andreas Ericsson wrote:
> Jakub Narebski wrote:
>> Junio C Hamano wrote:
>> 
>>> Doesn't leading drwxr-xr-x mean anything?
>> 
>> Neither d in drwxr-xr-x, nor trailing / in the name are significant
>> enough indicators of directory vs file (tree vs blob) indicator,
>> especially that by default git-ls-tree output mixes trees with blobs.
>>  
> 
> I beg to differ. The trailing slash would indeed help, as peoples eyes 
> are generally focused on the mouse-pointer rather than some 5-10" to the 
> left of it. If we're worrying that windows folks use \ for 
> path-delimiter, I think they'll pick up the difference quick enough.

I agree that it would help, but it is not enough when tree (directory)
and blob (file) would have the same color and the same decoration.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Jakub Narebski @ 2006-10-02 11:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhcynxjbc.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> 
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>> ... That means that we cannot distinguish really well (at 
>>> least color) between tree and blob entries.
>>
>> Do we even say links are blue and underlined by forcing that in
>> our css?
>>
>> Doesn't leading drwxr-xr-x mean anything?
>>
>> Why is making the distinction important in the first place?
> 
> Anyhow, I was too tired to sleep after an unscheduled day-job on
> Sunday X-<, and whipped this up for fun.
> 
> -- >8 --
> [PATCH] gitweb: remove UNIXy mode bits from tree display
> 
> and replace it with an image icon for cuteness ;-).

[...]
> +td.executable {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAACVJREFUCNdjYGBgYGdgMEhgUFBgYGJgcBKAIiAbKAIUB8oyMAAANBcCqbivEbgAAAAASUVORK5CYII=);
> +  background-repeat: no-repeat;
> +}
> +
> +td.folder {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAAB1JREFUCNdjYGBgkGFgUGJgcPzA4CCABdV/ACoBAFTTBQ822ZerAAAAAElFTkSuQmCC);
> +  background-repeat: no-repeat;
> +}
> +
> +td.regular {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAABtJREFUCNdjqD/A4JDA4BDA4FDB4MCBHdX/AACO5wbfUNnbqwAAAABJRU5ErkJggg==);
> +  background-repeat: no-repeat;
> +}
> +
> +td.symlink {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAACVJREFUCNdjYGBg4G9gMEhgUFBgcBAAIQYYYlFg4ElgkG8AKgEARSsDX750+Y0AAAAASUVORK5CYII=);
> +  background-repeat: no-repeat;
> +}
> +

Wouldn't it be better to use out-of-line images? I'm not sure if all browsers
support CSS embedded images, and if all browsers can cache such images.

Perhaps we could reuse Apache's MultiView/index.html.var... err, gitweb
tries to be web server agnostic...

And what about text browsers? It would be better to use <img> element, with
alt attribute set either to old UNIX-y mode bits, and title set to the file
type... or perhaps both alt and title attributes set to file type.

>  # convert file mode in octal to symbolic file mode string
> +sub kind_class {
> +	my ($type, $mode) = @_;
> +	$mode = oct $mode;
> +	if (S_ISDIR($mode & S_IFMT)) {
> +		return 'folder';
> +	} elsif (S_ISLNK($mode)) {
> +		return 'symlink';
> +	} elsif (S_ISREG($mode)) {
> +		# git cares only about the executable bit
> +		if ($mode & S_IXUSR) {
> +			return 'executable';
> +		} else {
> +			return 'regular';
> +		};
> +	}
> +}
> +

We have file_type subroutine, which does almost the same work. It doesn't
mark file as "executable", and it uses "directory" instead of non-standard
"folder".

By the way, $type argument (parameter) is not used at all.

> @@ -1651,7 +1668,9 @@ sub git_print_tree_entry {
>  	# the mode of the entry, list is the name of the entry, an href,
>  	# and link is the action links of the entry.
>  
> -	print "<td class=\"mode\">" . mode_str($t->{'mode'}) . "</td>\n";
> +	my $kind = kind_class($t->{'type'}, $t->{'mode'});
> +	print "<td class=\"$kind\">&nbsp;</td>\n";
> +
>  	if ($t->{'type'} eq "blob") {
>  		print "<td class=\"list\">" .
>  			$cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] lock_ref_sha1_basic does not remove empty directories on BSD
From: Dennis Stosberg @ 2006-10-02 17:23 UTC (permalink / raw)
  To: git

lock_ref_sha1_basic relies on errno beeing set to EISDIR by the
call to read() in resolve_ref() to detect directories.  But calling
read() on a directory under NetBSD returns EPERM, and even succeeds
for local filesystems on FreeBSD.

Signed-off-by: Dennis Stosberg <dennis@stosberg.net>
---
 refs.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index aa4c4e0..305c1a9 100644
--- a/refs.c
+++ b/refs.c
@@ -234,6 +234,12 @@ const char *resolve_ref(const char *ref,
 			}
 		}
 
+		/* Is it a directory? */
+		if (S_ISDIR(st.st_mode)) {
+			errno = EISDIR;
+			return NULL;
+		}
+
 		/*
 		 * Anything else, just open it and try to use it as
 		 * a ref
-- 
1.4.2

^ permalink raw reply related

* [PATCH] escape tilde in Documentation/git-rev-parse.txt
From: Stefan Richter @ 2006-10-02 18:55 UTC (permalink / raw)
  To: git

fixes a failure to build the git-rev-parse manpage,
seen with asciidoc 8.0.0

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index b761b4b..671b4e3 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -138,7 +138,7 @@ syntax.
   'rev{caret}0' means the commit itself and is used when 'rev' is the
   object name of a tag object that refers to a commit object.
 
-* A suffix '~<n>' to a revision parameter means the commit
+* A suffix '$$~$$<n>' to a revision parameter means the commit
   object that is the <n>th generation grand-parent of the named
   commit object, following only the first parent.  I.e. rev~3 is
   equivalent to rev{caret}{caret}{caret} which is equivalent to\

^ permalink raw reply related

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Luben Tuikov @ 2006-10-02 19:11 UTC (permalink / raw)
  To: Jakub Narebski, Junio C Hamano; +Cc: Luben Tuikov, git
In-Reply-To: <200610012041.15296.jnareb@gmail.com>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> But that is plain impossible without compromising interface usability.
> Let me explain. 
> 
> In the case of tree view (directory listing) we have blobs (files and 
> symlinks) and trees (directories). To mark item unequivocably as link 
> it has to have default link color (blue) and default link decoration 
> (underline). That means that we cannot distinguish really well (at 
> least color) between tree and blob entries. I'd rather have redundant 
> "blob"/"tree" (self)links, clearly marked as links, and tree entries 
> using link decoration (blue, underlined) while blob entries have 
> default text decoration (black, no underline).
> 
> In the case of shortlog/log/history/heads/tags view, to clearly mark 
> subject/title of a commit or tag as link, we would have to use default 
> link decoration. Let's for a while abandon link-within-link, i.e. using 
> some of committags also in commit title (in shortlog/history view)...
> But underlined text is harder to read, and blue underlined text even 
> more so (as for example it is hard to read italics, commonly used for 
> emphasis). I'd rather have additional "commit" link, clearly marked as 
> link, and leave subject as is, as hidden link, as a shortcut.
> 
> I think that redundancy in a visual interface (and not only visual, as 
> seen in the example of Perl programming language) is a good idea, 
> contrary to the redundancy in code or data (database).

Jakub,

Your opinion here is highly subjective.

Years of experience make certain things "make sense" and other
"make less sense".  Note that that is in itself subjective.

Give it 10 years, your opinion will change.

     Luben

^ permalink raw reply

* Re: [PATCH] gitweb: make leftmost column of blame less cluttered.
From: Luben Tuikov @ 2006-10-02 19:29 UTC (permalink / raw)
  To: Junio C Hamano, git
In-Reply-To: <7vwt7k4clr.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:

> Instead of labelling each and every line with clickable commit
> object name, this makes the blame output to show them only on
> the first line of each group of lines from the same revision.
> 
> Also it makes mouse-over to show the minimum authorship and
> authordate information for extra cuteness ;-).
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---

ACK.  Please commit.

   Luben

> 
>  * I've been staying away from the party to paint the bikeshed,
>    but I had a bit of time to kill tonight.  Let's see if people
>    might like this...
> 
>  gitweb/gitweb.perl |   67 +++++++++++++++++++++++++++++++++++++++++----------
>  1 files changed, 54 insertions(+), 13 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 44991b1..7e4ec8d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2429,6 +2429,41 @@ sub git_tag {
>  	git_footer_html();
>  }
>  
> +sub git_blame_flush_chunk {
> +	my ($name, $revdata, $color, $rev, @line) = @_;
> +	my $label = substr($rev, 0, 8);
> +	my $line = scalar(@line);
> +	my $cnt = 0;
> +	my $pop = '';
> +
> +	if ($revdata->{$rev} ne '') {
> +		$pop = ' title="' . esc_html($revdata->{$rev}) . '"';
> +	}
> +
> +	for (@line) {
> +		my ($lineno, $data) = @$_;
> +		$cnt++;
> +		print "<tr class=\"$color\">\n";
> +		if ($cnt == 1) {
> +			print "<td class=\"sha1\"$pop";
> +			if ($line > 1) {
> +				print " rowspan=\"$line\"";
> +			}
> +			print ">";
> +			print $cgi->a({-href => href(action=>"commit",
> +						     hash=>$rev,
> +						     file_name=>$name)},
> +				      $label);
> +			print "</td>\n";
> +		}
> +		print "<td class=\"linenr\">".
> +		    "<a id=\"l$lineno\" href=\"#l$lineno\" class=\"linenr\">" .
> +		    esc_html($lineno) . "</a></td>\n";
> +		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
> +		print "</tr>\n";
> +	}
> +}
> +
>  sub git_blame2 {
>  	my $fd;
>  	my $ftype;
> @@ -2474,27 +2509,33 @@ sub git_blame2 {
>  <table class="blame">
>  <tr><th>Commit</th><th>Line</th><th>Data</th></tr>
>  HTML
> +	my @chunk = ();
> +	my %revdata = ();
>  	while (<$fd>) {
>  		/^([0-9a-fA-F]{40}).*?(\d+)\)\s{1}(\s*.*)/;
> -		my $full_rev = $1;
> -		my $rev = substr($full_rev, 0, 8);
> -		my $lineno = $2;
> -		my $data = $3;
> -
> +		my ($full_rev, $author, $date, $lineno, $data) =
> +		    /^([0-9a-f]{40}).*?\s\((.*?)\s+([-\d]+ [:\d]+ [-+\d]+)\s+(\d+)\)\s(.*)/;
> +		if (!exists $revdata{$full_rev}) {
> +			$revdata{$full_rev} = "$author, $date";
> +		}
>  		if (!defined $last_rev) {
>  			$last_rev = $full_rev;
>  		} elsif ($last_rev ne $full_rev) {
> +			git_blame_flush_chunk($file_name,
> +					      \%revdata,
> +					      $rev_color[$current_color],
> +					      $last_rev, @chunk);
> +			@chunk = ();
>  			$last_rev = $full_rev;
>  			$current_color = ++$current_color % $num_colors;
>  		}
> -		print "<tr class=\"$rev_color[$current_color]\">\n";
> -		print "<td class=\"sha1\">" .
> -			$cgi->a({-href => href(action=>"commit", hash=>$full_rev, file_name=>$file_name)},
> -			        esc_html($rev)) . "</td>\n";
> -		print "<td class=\"linenr\"><a id=\"l$lineno\" href=\"#l$lineno\" class=\"linenr\">" .
> -		      esc_html($lineno) . "</a></td>\n";
> -		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
> -		print "</tr>\n";
> +		push @chunk, [$lineno, $data];
> +	}
> +	if (@chunk) {
> +		git_blame_flush_chunk($file_name,
> +				      \%revdata,
> +				      $rev_color[$current_color],
> +				      $last_rev, @chunk);
>  	}
>  	print "</table>\n";
>  	print "</div>";
> -- 
> 1.4.2.1.gc9fffe
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH] gitweb: Add history and blame to git_difftree_body()
From: Luben Tuikov @ 2006-10-02 19:32 UTC (permalink / raw)
  To: Jakub Narebski, git
In-Reply-To: <efor2e$d0i$1@sea.gmane.org>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Luben Tuikov wrote:
> 
> > Add blame and history to Deleted files.
> > Add blame and history to Modified or Type changed files.
> > Add blame and history to Renamed or Copied files.
> 
> Blame has to be added conditionally (gitweb_have_blame).

Can you please not edit the CC field when you reply?
I can see that only the GIT mailing list was left,
but my email had been removed.

Thanks,
   Luben

^ permalink raw reply

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Luben Tuikov @ 2006-10-02 19:46 UTC (permalink / raw)
  To: Junio C Hamano, Jakub Narebski; +Cc: git
In-Reply-To: <7vd59b27c4.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > ... That means that we cannot distinguish really well (at 
> > least color) between tree and blob entries.
> 
> Do we even say links are blue and underlined by forcing that in
> our css?
> 
> Doesn't leading drwxr-xr-x mean anything?
> 
> Why is making the distinction important in the first place?
> 
> > In the case of shortlog/log/history/heads/tags view, to clearly mark 
> > subject/title of a commit or tag as link, we would have to use default 
> > link decoration....
> > But underlined text is harder to read, and blue underlined text even 
> > more so...
> 
> This is something in which I can see some more sense than tree
> vs blob issue, but only as a principle issue.  In practice, the
> list of commit subjects is the first thing users encounter, and
> as long as there is some visual indication (e.g. mousing over it
> makes it obvious it is something that is clickable), I think
> users will quickly pick up that it will lead to the commit's
> detail.

Very well put.

   Luben

^ permalink raw reply

* Re: [PATCH] gitweb: Remove excessively redundant entries from git_difftree_body
From: Luben Tuikov @ 2006-10-02 19:48 UTC (permalink / raw)
  To: Jakub Narebski, git
In-Reply-To: <efp4d6$a7b$1@sea.gmane.org>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Junio C Hamano wrote:
> 
> > Luben Tuikov <ltuikov@yahoo.com> writes:
> > 
> >> 1) All entries on the left are blobs and clicking on them
> >> leads to blobs.  No more diff or blob depending on what
> >> happened (modified or mode changed) to the file -- this goes
> >> to the right, in the "link" column.
> >>
> >> 2) Remove redundant "blob" from the link column on the right.
> >> This can now be had by clicking on the entry itself.
> >>
> >> This reduces and simplifies the code.
> >>
> >> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> > 
> > We lost blobdiff from commitdiff page with this patch, which I
> > think it probably is a good change.  We can say "when we display
> > a pathname you can click it to get the blob or tree".
> 
> In my opinion it is a bad change. We now have "patch" inner-link
> to anchor in commitdiff view, but it's equivalent in commit view
> i.e. diff aka blobdiff vanished.

All which can be easily had.

    Luben

^ permalink raw reply

* Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive
From: Luben Tuikov @ 2006-10-02 19:51 UTC (permalink / raw)
  To: Jakub Narebski, git
In-Reply-To: <efpfc3$765$1@sea.gmane.org>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Luben Tuikov wrote:
> 
> > What this patch does is simply set "h" and/or "hb" to
> > the string "HEAD" instead of the string "<SHA1 of HEAD>" when
> > h/hb is not defined.
> 
> I guess there it is a good idea, but we should always think
> over if we want "constant" link, always showing the same thing,
> or a "variable" (or "news") link, showing current version.
> 
> Explicit sha1 hash as 'h'/'hb' gives persistent, cacheable link,
> while e.g. "HEAD" gives "variable" link.

Exactly what is wanted by people using git in a work environment.

   Luben
P.S. Please don't edit out the CC list leaving only git in. Click
"Reply-all" please, just like everyone else does.

^ permalink raw reply

* Re: [PATCH] gitweb: Add history and blame to git_difftree_body()
From: Jakub Narebski @ 2006-10-02 19:55 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: git
In-Reply-To: <20061002193217.86165.qmail@web31814.mail.mud.yahoo.com>

Luben Tuikov wrote:
> --- Jakub Narebski <jnareb@gmail.com> wrote:
> > Luben Tuikov wrote:
> > 
> > > Add blame and history to Deleted files.
> > > Add blame and history to Modified or Type changed files.
> > > Add blame and history to Renamed or Copied files.
> > 
> > Blame has to be added conditionally (gitweb_have_blame).
> 
> Can you please not edit the CC field when you reply?
I do not edit Cc: field. I reply via GMane NNTP (Usenet)
interface.

> I can see that only the GIT mailing list was left,
> but my email had been removed.
It was not removed, it was not added.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] gitweb: tree view: eliminate redundant "blob"
From: Jakub Narebski @ 2006-10-02 20:03 UTC (permalink / raw)
  To: Luben Tuikov; +Cc: git
In-Reply-To: <20061002191115.84730.qmail@web31811.mail.mud.yahoo.com>

Luben Tuikov wrote:
> --- Jakub Narebski <jnareb@gmail.com> wrote:
>> I think that redundancy in a visual interface (and not only visual, as 
>> seen in the example of Perl programming language) is a good idea, 
>> contrary to the redundancy in code or data (database).
> 
> Jakub,
> 
> Your opinion here is highly subjective.

Yours too.
 
> Years of experience make certain things "make sense" and other
> "make less sense".  Note that that is in itself subjective.
> 
> Give it 10 years, your opinion will change.

Perhaps. Perhaps not.

I guess we have to agree to disagree. It's Junio opinion that
matters (which patches would get accepted).

I'd like to mention that there are if I remember correctly
some other places, like "commit" view which have this hated by
you redundancy.
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: gitweb: using quotemeta
From: Luben Tuikov @ 2006-10-02 20:12 UTC (permalink / raw)
  To: Jakub Narebski, git
In-Reply-To: <efpme9$o13$1@sea.gmane.org>

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Luben Tuikov wrote:
> 
> > --- Junio C Hamano <junkio@cox.net> wrote:
> >> 
> >> Ouch, that was a sloppy planning and coding, and sloppier
> >> reviewing.  Sorry.
> >> 
> >> What is the right quoting there?  Just quoting double-quotes?
> > 
> > I'm not sure.  What undesired character could we have in $filename
> > of a snapshot?  The commit ab41dfbfd4f message gives this
> > justification: "Just in case filename contains end of line character."
> > 
> > It looks like $filename is constructed by well defined strings:
> > basename($project), $hash and $suffix all of which should be ok.
> > 
> > I'd say we don't need quotemeta for $filename of snapshot.
> 
> But we do need quoting for blob_plain and perhaps blobdiff_plain
> views, although not quotemeta, but perhaps the reverse of unescape,
> i.e. quote '"', EOLN (end of line) and perhaps also TAB.

Escaping for the purposes of HTML _view_ and URL generation is ok,
but it is not ok when _saving_ the file with a file name.

A file name is just a string of chars, and I want to _save_ the file
name as its name is. No changes or interpretations please.  I don't
care what the string is, what chars it is made of, etc.

Please don't interpret file names and their characters when the files
are _saved_ by the user's browser.

The file name in my filesystem should be the exact same file name
as it appears on any other filesystem hosting the same git repo.

I don't want this translation:
Server FS: linux-2.6.git-5c2d97cb31fb77981797fec46230ca005b865799.tar.gz
Quotemeta: linux\-2\.6\.git\-5c2d97cb31fb77981797fec46230ca005b865799\.tar\.gz
User FS: linux\-2\.6\.git\-5c2d97cb31fb77981797fec46230ca005b865799\.tar\.gz

When you comitted ab41dfbfd4f3f9fedac71550027e9813b11abe3d, it extended
quotemeta to where it shouldn't have been applied.

    Luben
P.S. When replying please don't redact the CC field.

^ permalink raw reply

* Re: gitweb: using quotemeta
From: Jakub Narebski @ 2006-10-02 20:50 UTC (permalink / raw)
  To: Luben Tuikov, Junio Hamano; +Cc: git
In-Reply-To: <20061002201256.89409.qmail@web31809.mail.mud.yahoo.com>

Luben Tuikov wrote:
> --- Jakub Narebski <jnareb@gmail.com> wrote:
>> Luben Tuikov wrote:
>> 
>>> --- Junio C Hamano <junkio@cox.net> wrote:
>>>> 
>>>> Ouch, that was a sloppy planning and coding, and sloppier
>>>> reviewing.  Sorry.
>>>> 
>>>> What is the right quoting there?  Just quoting double-quotes?
>>> 
>>> I'm not sure.  What undesired character could we have in $filename
>>> of a snapshot?  The commit ab41dfbfd4f message gives this
>>> justification: "Just in case filename contains end of line character."
>>> 
>>> It looks like $filename is constructed by well defined strings:
>>> basename($project), $hash and $suffix all of which should be ok.
>>> 
>>> I'd say we don't need quotemeta for $filename of snapshot.
>> 
>> But we do need quoting for blob_plain and perhaps blobdiff_plain
>> views, although not quotemeta, but perhaps the reverse of unescape,
>> i.e. quote '"', EOLN (end of line) and perhaps also TAB.
> 
> Escaping for the purposes of HTML _view_ and URL generation is ok,
> but it is not ok when _saving_ the file with a file name.
> 
> A file name is just a string of chars, and I want to _save_ the file
> name as its name is. No changes or interpretations please.  I don't
> care what the string is, what chars it is made of, etc.

We need to _escape_ HTML output (using esc_html subroutine) because
HTML treats some characters specially, and we need to escape them
to turn off this interpretation. Examples include SPC (for example
file name which has two consecutive spaces) and ampersand '&' which
might be treated as entity reference.

Sometimes we want to quote filename for view, for example to fit it
in one line, and to distinguish between tab and spaces.


But you forget that in HTTP headers, to be more exact in
	Content-Disposition: inline; filename="<filename>"
header, the quote '"' and end-of-line '\n' characters in <filename>
are treated specially. So you need to quote somehow at least those
two characters.


<checks the RFC>

RFC 2616 "Hypertext Transfer Protocol -- HTTP/1.1":
   RFC 1806 [35], from which the often implemented Content-Disposition
   (see section 19.5.1) header in HTTP is derived, has a number of very
   serious security considerations. Content-Disposition is not part of
   the HTTP standard, but since it is widely implemented, we are
   documenting its use and risks for implementors. See RFC 2183 [49]
   (which updates RFC 1806) for details.

RFC 2183 "Communicating Presentation Information in Internet Messages:
          The Content-Disposition Header Field":
2.3  The Filename Parameter

   The sender may want to suggest a filename to be used if the entity is
   detached and stored in a separate file. If the receiving MUA writes
   the entity to a file, the suggested filename should be used as a
   basis for the actual filename, where possible.

   It is important that the receiving MUA not blindly use the suggested
   filename.  The suggested filename SHOULD be checked (and possibly
   changed) to see that it conforms to local filesystem conventions,
   does not overwrite an existing file, and does not present a security
   problem (see Security Considerations below).

   The receiving MUA SHOULD NOT respect any directory path information
   that may seem to be present in the filename parameter.  The filename
   should be treated as a terminal component only.  Portable
   specification of directory paths might possibly be done in the future
   via a separate Content-Disposition parameter, but no provision is
   made for it in this draft.

   Current [RFC 2045] grammar restricts parameter values (and hence
   Content-Disposition filenames) to US-ASCII.  We recognize the great
   desirability of allowing arbitrary character sets in filenames, but
   it is beyond the scope of this document to define the necessary
   mechanisms.  We expect that the basic [RFC 1521] `value'
   specification will someday be amended to allow use of non-US-ASCII
   characters, at which time the same mechanism should be used in the
   Content-Disposition filename parameter.

> Please don't interpret file names and their characters when the files
> are _saved_ by the user's browser.
>
> The file name in my filesystem should be the exact same file name
> as it appears on any other filesystem hosting the same git repo.

As you can see from the above RFC, there is no standard way to pass
suggested filename to be exactly the same as in git repository, if
it contains non US-ASCII characters. At least not for _HTTP_ 
Content-Disposition: header; for email we can use quoted-printable
from RFC 2047 to encode non US-ASCII characters (including " and EOLN)
with good probability of this working correctly.
 
> I don't want this translation:
> Server FS: linux-2.6.git-5c2d97cb31fb77981797fec46230ca005b865799.tar.gz
> Quotemeta: linux\-2\.6\.git\-5c2d97cb31fb77981797fec46230ca005b865799\.tar\.gz
> User FS: linux\-2\.6\.git\-5c2d97cb31fb77981797fec46230ca005b865799\.tar\.gz
> 
> When you comitted ab41dfbfd4f3f9fedac71550027e9813b11abe3d, it extended
> quotemeta to where it shouldn't have been applied.

I only followed (I admit blindly, without checking the details, and
I agree wrongly) the commit a2f3db2f5de2a3667b0e038aa65e3e097e642e7d
   "gitweb: Consolidate escaping/validation of query string"
by Petr "Pasky" Baudis, which among others included the following patch
(which part of commit is not documented enough in commit message):

@@ -3126,7 +3116,7 @@ sub git_blobdiff {
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
-			-content_disposition => qq(inline; filename="${file_name}.patch"));
+			-content_disposition => qq(inline; filename=") . quotemeta($file_name) . qq(.patch"));
 
 		print "X-Git-Url: " . $cgi->self_url() . "\n\n";
 

And I agree that your
   "gitweb: Don't use quotemeta on internally generated strings"
is (partially) correct.

>     Luben
> P.S. When replying please don't redact the CC field.

When I reply by email, I have no problem (I have to add git
mailing list if I forget to use reply to all). When I reply
via GMane NNTP interface, KNode by default replies only to
newsgroup (coupled with git mailing list).
-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] gitweb: Escape ESCAPE (\e) character
From: Luben Tuikov @ 2006-10-02 21:54 UTC (permalink / raw)
  To: git

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

Take a look at commit 20a3847d8a5032ce41f90dcc68abfb36e6fee9b1
using gitweb before this patch.  This patch fixes this.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

[-- Attachment #2: 497324604-pe.txt --]
[-- Type: text/plain, Size: 475 bytes --]

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 10e803a..a9e95c9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -485,6 +485,7 @@ sub esc_html {
 	$str = decode("utf8", $str, Encode::FB_DEFAULT);
 	$str = escapeHTML($str);
 	$str =~ s/\014/^L/g; # escape FORM FEED (FF) character (e.g. in COPYING file)
+	$str =~ s/\033/^[/g; # "escape" ESCAPE (\e) character (e.g. commit 20a3847d8a5032ce41f90dcc68abfb36e6fee9b1)
 	return $str;
 }
 
-- 
1.4.2.3.gc269

^ permalink raw reply related

* Re: git and time
From: Jakub Narebski @ 2006-10-02 23:50 UTC (permalink / raw)
  To: git
In-Reply-To: <7vd59d7y8v.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Linus Torvalds <torvalds@osdl.org> writes:
> 
>> On Sat, 30 Sep 2006, Junio C Hamano wrote:
>>
>>> Shawn Pearce <spearce@spearce.org> writes:
>>> 
>>> > Agreed.  I've been missing such a command and have wanted to add
>>> > one but it wasn't important enough to me to actually code it.  :)
>>> 
>>> Everything you said in your message sounds sane and makes sense
>>> to me.  Now we have to find a sucker^Wvolunteer to implement it
>>> ;-).
>>
>> Ehh. As far as I can see it's 
>>  - a damn hard thing to do efficiently
>>  - essentially exactly the same problem you already solved with "git 
>>    describe"
>>
>> In other words, I think you could make git describe do it, by simply 
>> making it parse not just all tags, but also walking the branch log.
> 
> As a user interface, I think it makes a lot of sense to have
> "git describe" do it without introducing a new command.
> 
> However, I think the traditional "find the closest ancestor"
> behaviour and ref-log behaviour are mutually incompatible, while
> they both return information to help address similar issues to
> the end user when viewed at a very high level.
> 
> Especially, "find the closest ancestor" behaviour means when you
> get "tag-gXXXX" as an answer, the tag proper does _not_ contain
> the given commit (e.g. commit v1.4.2-g4839bd8 is not part of
> v1.4.2).  To answer "when did the fix deadbeef go into master
> branch", reporting "master@{yesterday}-gdeadbeef" with the same
> logic and format is misleading; "master@{yesterday}" may be the
> closest ancestor of commit deadbeef, but that means it does
> _not_ contain the fix.  When walking ref-log, we want it the
> other way around: "find the earliest descendant among the
> entries in ref-log for a particular branch".
> 
> The internal logic for doing that may be somewhat different and
> I suspect you may not be able to share much code with the
> existing logic..

Isn't git-name-rev doing kind of earliest descendant among refs?
Well, I'm not sure if name-rev does not use shortest description
instead of earliest (closest) descendant... but we could extend it.
I'd like also to limit refs used to given pattern, and perhaps
also to only tag objects.

This is command to extend it using it together with ref-log to know
when given fix appeared in repository.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ 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