threads / patch / 5627

patchgitweb: Fix showing of path in tree view

Subject: [PATCH] gitweb: Fix showing of path in tree view

## tl;dr

10 messages between Sep 20, 2006 and Sep 21, 2006. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Petr Baudis· Sep 20, 2006, 23:12 UTC · lore

This patch fixes two things - links to all path elements except the last one were broken since gitweb does not like the trailing slash in them, and the root tree was not reachable from the subdirectory view.

To compensate for the one more slash in the front, the trailing slash is not there anymore. ;-) I don't care if it stays there though.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 gitweb/gitweb.perl |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

Show changes to gitweb/gitweb.perl +4 −4

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 834a773..70f409e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1503,12 +1503,13 @@ sub git_print_page_path {
 		my $fullname = '';
 
 		print "<div class=\"page_path\">";
+		print $cgi->a({-href => href(action=>"tree", hash_base=>$hb),
+			      -title => '/'}, '/');
 		foreach my $dir (@dirname) {
-			$fullname .= $dir . '/';
+			$fullname .= ($fullname ? '/' : '') . $dir;
 			print $cgi->a({-href => href(action=>"tree", file_name=>$fullname,
 			                             hash_base=>$hb),
-			              -title => $fullname}, esc_html($dir));
-			print "/";
+			              -title => $fullname}, esc_html($dir . '/'));
 		}
 		if (defined $type && $type eq 'blob') {
 			print $cgi->a({-href => href(action=>"blob_plain", file_name=>$file_name,
@@ -1518,7 +1519,6 @@ sub git_print_page_path {
 			print $cgi->a({-href => href(action=>"tree", file_name=>$file_name,
 			                             hash_base=>$hb),
 			              -title => $name}, esc_html($basename));
-			print "/";
 		} else {
 			print esc_html($basename);
 		}

Junio C Hamano· Sep 20, 2006, 23:32 UTC · re: Petr Baudis · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Petr Baudis <pasky@suse.cz> writes:

Show 6 quoted lines

> This patch fixes two things - links to all path elements except the last
> one were broken since gitweb does not like the trailing slash in them, and
> the root tree was not reachable from the subdirectory view.
>
> To compensate for the one more slash in the front, the trailing slash is
> not there anymore. ;-) I don't care if it stays there though.

Hmph.  I see the breakage and behaviour-wise your patch is fine.

But I think the older one was visually nicer and more intuitive in one very small detail that I think matters. Slashes between path components were not part of the anchor elements, so it was clear that there were two links in "gitweb / gitweb.perl" page title, not just one link. Now it is not obvious that clicking different parts of the path string in "/stgit/commands/mail.py" would lead to different places. And as you are aware, losing the leading "/" would be nicer as well ;-).

Petr Baudis· Sep 20, 2006, 23:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Dear diary, on Thu, Sep 21, 2006 at 01:32:42AM CEST, I got a letter where Junio C Hamano <junkio@cox.net> said that...

Show 19 quoted lines

> Petr Baudis <pasky@suse.cz> writes:
> 
> > This patch fixes two things - links to all path elements except the last
> > one were broken since gitweb does not like the trailing slash in them, and
> > the root tree was not reachable from the subdirectory view.
> >
> > To compensate for the one more slash in the front, the trailing slash is
> > not there anymore. ;-) I don't care if it stays there though.
> 
> Hmph.  I see the breakage and behaviour-wise your patch is fine.
> 
> But I think the older one was visually nicer and more intuitive
> in one very small detail that I think matters.  Slashes between
> path components were not part of the anchor elements, so it was
> clear that there were two links in "gitweb / gitweb.perl" page
> title, not just one link.  Now it is not obvious that clicking
> different parts of the path string in "/stgit/commands/mail.py"
> would lead to different places.  And as you are aware, losing
> the leading "/" would be nicer as well ;-).

  Well, this was the best I could come up without introducing "[root] /"
or something in front of the path, which would be IMHO even uglier.
Alternative ideas welcomed. :-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Junio C Hamano· Sep 20, 2006, 23:51 UTC · re: Petr Baudis · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Petr Baudis <pasky@suse.cz> writes:

Show 14 quoted lines

>> Hmph.  I see the breakage and behaviour-wise your patch is fine.
>> 
>> But I think the older one was visually nicer and more intuitive
>> in one very small detail that I think matters.  Slashes between
>> path components were not part of the anchor elements, so it was
>> clear that there were two links in "gitweb / gitweb.perl" page
>> title, not just one link.  Now it is not obvious that clicking
>> different parts of the path string in "/stgit/commands/mail.py"
>> would lead to different places.  And as you are aware, losing
>> the leading "/" would be nicer as well ;-).
>
>   Well, this was the best I could come up without introducing "[root] /"
> or something in front of the path, which would be IMHO even uglier.
> Alternative ideas welcomed. :-)

Ok, when I commented on it I did not realize that the leading slash was clickable.

How about rendering it like this?

	/ stgit/ commands/ mail.py

Jakub Narebski· Sep 21, 2006, 08:50 UTC · re: Petr Baudis · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Petr Baudis wrote:

> Dear diary, on Thu, Sep 21, 2006 at 01:32:42AM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...

Show 12 quoted lines

>> But I think the older one was visually nicer and more intuitive
>> in one very small detail that I think matters.  Slashes between
>> path components were not part of the anchor elements, so it was
>> clear that there were two links in "gitweb / gitweb.perl" page
>> title, not just one link.  Now it is not obvious that clicking
>> different parts of the path string in "/stgit/commands/mail.py"
>> would lead to different places.  And as you are aware, losing
>> the leading "/" would be nicer as well ;-).
> 
>   Well, this was the best I could come up without introducing "[root] /"
> or something in front of the path, which would be IMHO even uglier.
> Alternative ideas welcomed. :-)

"[$project] /", or "~ /" instead of "[root] /".

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Petr Baudis· Sep 21, 2006, 11:25 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Dear diary, on Thu, Sep 21, 2006 at 10:50:46AM CEST, I got a letter where Jakub Narebski <jnareb@gmail.com> said that...

Show 19 quoted lines

> Petr Baudis wrote:
> 
> > Dear diary, on Thu, Sep 21, 2006 at 01:32:42AM CEST, I got a letter
> > where Junio C Hamano <junkio@cox.net> said that...
> 
> >> But I think the older one was visually nicer and more intuitive
> >> in one very small detail that I think matters.  Slashes between
> >> path components were not part of the anchor elements, so it was
> >> clear that there were two links in "gitweb / gitweb.perl" page
> >> title, not just one link.  Now it is not obvious that clicking
> >> different parts of the path string in "/stgit/commands/mail.py"
> >> would lead to different places.  And as you are aware, losing
> >> the leading "/" would be nicer as well ;-).
> > 
> >   Well, this was the best I could come up without introducing "[root] /"
> > or something in front of the path, which would be IMHO even uglier.
> > Alternative ideas welcomed. :-)
> 
> "[$project] /", or "~ /" instead of "[root] /".

Yes, the former is what repo.or.cz uses now and in a patch I've sent privately to Junio (since my StGIT configuration at repo.or.cz is messed up and I had no time to fix it).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Jakub Narebski· Sep 20, 2006, 23:35 UTC · re: Petr Baudis · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Petr Baudis wrote:

Show 6 quoted lines

> This patch fixes two things - links to all path elements except the last
> one were broken since gitweb does not like the trailing slash in them, and
> the root tree was not reachable from the subdirectory view.
> 
> To compensate for the one more slash in the front, the trailing slash is
> not there anymore. ;-) I don't care if it stays there though.

Originally '/' was used as separator between directories making the path. I'd rather use ' / ' to separate parts of pathname more, and not incorporate it in the link.

Trailing (final) slash is present (and I think should be present) only for path in "tree" view. From the path alone you can see if it is "tree" or "blob" view.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Jakub Narebski· Sep 20, 2006, 23:38 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Jakub Narebski wrote:

Show 16 quoted lines

> Petr Baudis wrote:
> 
>> This patch fixes two things - links to all path elements except the last
>> one were broken since gitweb does not like the trailing slash in them, and
>> the root tree was not reachable from the subdirectory view.
>> 
>> To compensate for the one more slash in the front, the trailing slash is
>> not there anymore. ;-) I don't care if it stays there though.
> 
> Originally '/' was used as separator between directories making the path.
> I'd rather use ' / ' to separate parts of pathname more, and not incorporate
> it in the link.
> 
> Trailing (final) slash is present (and I think should be present) only
> for path in "tree" view. From the path alone you can see if it is "tree"
> or "blob" view.

About link to root tree: we could change the commit title to link to root tree instead of linking to commit view.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Petr Baudis· Sep 20, 2006, 23:43 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Dear diary, on Thu, Sep 21, 2006 at 01:38:49AM CEST, I got a letter where Jakub Narebski <jnareb@gmail.com> said that...

> Jakub Narebski wrote:
> > Trailing (final) slash is present (and I think should be present) only
> > for path in "tree" view. From the path alone you can see if it is "tree"
> > or "blob" view.

Which sounds like a nice property but the rest of the page looks totally different anyway, so I don't know how practical that is.

> About link to root tree: we could change the commit title to link to root
> tree instead of linking to commit view.

I don't think that makes any sense. It's a _commit_ title! :-) And if I want to get to root tree it wouldn't occur to me to click _there_ anyway.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Jakub Narebski· Sep 21, 2006, 10:19 UTC · re: Petr Baudis · lore

Re: [PATCH] gitweb: Fix showing of path in tree view

Petr Baudis wrote:

> This patch fixes two things - links to all path elements except the last
> one were broken since gitweb does not like the trailing slash in them, and
> the root tree was not reachable from the subdirectory view.

Wouldn't it be better to fix git_get_hash_by_path to have it strip trailing slash? Gitweb don't like trailing slash in pathname (directory name) because while

        git ls-tree $base -- dirname

shows dirname entry

        git ls-tree $base -- dirname/

shows directory _contents_.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git