threads / patch / 5708

patchgitweb: tree view: hash_base and hash are now context sensitive

Subject: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

## tl;dr

12 messages between Sep 26, 2006 and Oct 2, 2006. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Luben Tuikov· Sep 26, 2006, 19:47 UTC · lore

In tree view, by default, hash_base is HEAD and hash is the entry equivalent. Else the user had selected a hash_base or hash, say by clicking on a revision or commit, in which case those values are used.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.perl |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
Show changes to gitweb/gitweb.perl +7 −7
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8e9c827..56638f2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1625,7 +1625,7 @@ sub git_print_tree_entry {
 			              "history");
 		}
 		print " | " .
-			$cgi->a({-href => href(action=>"blob_plain", hash_base=>"HEAD",
+			$cgi->a({-href => href(action=>"blob_plain", hash_base=>$hash_base,
 					       file_name=>"$basedir$t->{'name'}")},
 				"raw");
 		print "</td>\n";
@@ -2713,14 +2713,14 @@ sub git_tree {
 	my ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');
 	my $have_snapshot = (defined $ctype && defined $suffix);
 
+	if (!defined $hash_base) {
+		$hash_base = "HEAD";
+	}
 	if (!defined $hash) {
-		$hash = git_get_head_hash($project);
 		if (defined $file_name) {
-			my $base = $hash_base || $hash;
-			$hash = git_get_hash_by_path($base, $file_name, "tree");
-		}
-		if (!defined $hash_base) {
-			$hash_base = $hash;
+			$hash = git_get_hash_by_path($hash_base, $file_name, "tree");
+		} else {
+			$hash = $hash_base;
 		}
 	}
 	$/ = "\0";

-- 
1.4.2.1.g893b0
Jakub Narebski· Sep 26, 2006, 20:34 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

Luben Tuikov wrote:
> In tree view, by default, hash_base is HEAD and hash is the
> entry equivalent.  Else the user had selected a hash_base or
> hash, say by clicking on a revision or commit, in which case
> those values are used.

I think that this need some thinking over. For blob we have two "base" objects: tree which have specified blob, and commit which have tree which have specified blob. We might want to specify that all hash*base are to the commit-ish.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Luben Tuikov· Sep 26, 2006, 21:17 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

--- Jakub Narebski <jnareb@gmail.com> wrote:
> I think that this need some thinking over. For blob we have two
> "base" objects: tree which have specified blob, and commit which
> have tree which have specified blob. We might want to specify
> that all hash*base are to the commit-ish.

Agreed, we should always refer to the commit-ish, for obvious reasons.

This patch doesn't make this decision though. It simply sets hash_base to HEAD if not defined.

Now, since "git-ls-tree" works on both commit-ish and tree-ish, we are ok.

    Luben
Junio C Hamano· Sep 27, 2006, 06:48 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

Luben Tuikov <ltuikov@yahoo.com> writes:
Show 14 quoted lines
> --- Jakub Narebski <jnareb@gmail.com> wrote:
>> I think that this need some thinking over. For blob we have two
>> "base" objects: tree which have specified blob, and commit which
>> have tree which have specified blob. We might want to specify
>> that all hash*base are to the commit-ish.
>
> Agreed, we should always refer to the commit-ish, for obvious
> reasons.
>
> This patch doesn't make this decision though.  It simply
> sets hash_base to HEAD if not defined.
>
> Now, since "git-ls-tree" works on both commit-ish and
> tree-ish, we are ok.
I think so, too.  Jakub?
Luben Tuikov· Sep 27, 2006, 22:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

--- Junio C Hamano <junkio@cox.net> wrote:
Show 18 quoted lines
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > --- Jakub Narebski <jnareb@gmail.com> wrote:
> >> I think that this need some thinking over. For blob we have two
> >> "base" objects: tree which have specified blob, and commit which
> >> have tree which have specified blob. We might want to specify
> >> that all hash*base are to the commit-ish.
> >
> > Agreed, we should always refer to the commit-ish, for obvious
> > reasons.
> >
> > This patch doesn't make this decision though.  It simply
> > sets hash_base to HEAD if not defined.
> >
> > Now, since "git-ls-tree" works on both commit-ish and
> > tree-ish, we are ok.
> 
> I think so, too.  Jakub?

Status? I see Jakub hasn't objected, and it is really important for us to be able to link to "latest" binary blob -- for example a PDF spec file.

   Luben
Luben Tuikov· Sep 28, 2006, 18:52 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

--- Luben Tuikov <ltuikov@yahoo.com> wrote:
Show 23 quoted lines
> --- Junio C Hamano <junkio@cox.net> wrote:
> > Luben Tuikov <ltuikov@yahoo.com> writes:
> > 
> > > --- Jakub Narebski <jnareb@gmail.com> wrote:
> > >> I think that this need some thinking over. For blob we have two
> > >> "base" objects: tree which have specified blob, and commit which
> > >> have tree which have specified blob. We might want to specify
> > >> that all hash*base are to the commit-ish.
> > >
> > > Agreed, we should always refer to the commit-ish, for obvious
> > > reasons.
> > >
> > > This patch doesn't make this decision though.  It simply
> > > sets hash_base to HEAD if not defined.
> > >
> > > Now, since "git-ls-tree" works on both commit-ish and
> > > tree-ish, we are ok.
> > 
> > I think so, too.  Jakub?
> 
> Status?  I see Jakub hasn't objected, and it is really important
> for us to be able to link to "latest" binary blob -- for example
> a PDF spec file.
Junio,
What are the objections to this patch?
Thanks,
   Luben
Junio C Hamano· Sep 29, 2006, 06:06 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

Luben Tuikov <ltuikov@yahoo.com> writes:
Show 18 quoted lines
>> > > Agreed, we should always refer to the commit-ish, for obvious
>> > > reasons.
>> > >
>> > > This patch doesn't make this decision though.  It simply
>> > > sets hash_base to HEAD if not defined.
>> > >
>> > > Now, since "git-ls-tree" works on both commit-ish and
>> > > tree-ish, we are ok.
>> > 
>> > I think so, too.  Jakub?
>> 
>> Status?  I see Jakub hasn't objected, and it is really important
>> for us to be able to link to "latest" binary blob -- for example
>> a PDF spec file.
>
> Junio,
>
> What are the objections to this patch?
No objections from me --- rather lack of objections from Jakub ;-)
But the fragments do not appear to apply anymore.
Show changes to diff +1 −1
@@ -1625,7 +1625,7 @@ sub git_print_tree_entry {
 			              "history");
 		}
 		print " | " .
-			$cgi->a({-href => href(action=>"blob_plain", hash_base=>"HEAD",
+			$cgi->a({-href => href(action=>"blob_plain", hash_base=>$hash_base,
 					       file_name=>"$basedir$t->{'name'}")},
 				"raw");
 		print "</td>\n";

There is no call to create blob_plain link with hash_base
hardcoded to "HEAD" in git-print-tree-entry.

Care to re-spin?
Luben Tuikov· Sep 29, 2006, 16:16 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

--- Junio C Hamano <junkio@cox.net> wrote:
> > What are the objections to this patch?
> 
> No objections from me --- rather lack of objections from Jakub ;-)

We haven't heard anything from him, and on our last correspondence in this thread it seemed we're in agreement.

> But the fragments do not appear to apply anymore.
[...]
> There is no call to create blob_plain link with hash_base
> hardcoded to "HEAD" in git-print-tree-entry.
> 
> Care to re-spin?
Doing it right now.
Thanks,
   Luben
Jakub Narebski· Sep 29, 2006, 20:35 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

Luben Tuikov wrote:
Show 7 quoted lines
> --- Junio C Hamano <junkio@cox.net> wrote:
> > > What are the objections to this patch?
> > 
> > No objections from me --- rather lack of objections from Jakub ;-)
> 
> We haven't heard anything from him, and on our last correspondence
> in this thread it seemed we're in agreement.

As a interim solution it is a good idea, moreover with using $hash_base defaulting to HEAD instead of using HEAD literaly.

The correct solution would be to make "html" page (i.e. "blob" not "blob_plain" view) also for binary files. <img> element for image/* mimetype, perhaps <embed> or <object>, or just plain link for other binary (not text/* or some application/*) types

I was AFK for few days.
-- 
Jakub Narebski
Poland
Luben Tuikov· Sep 29, 2006, 23:30 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

--- Jakub Narebski <jnareb@gmail.com> wrote:
Show 7 quoted lines
> As a interim solution it is a good idea, moreover with using $hash_base
> defaulting to HEAD instead of using HEAD literaly.
> 
> The correct solution would be to make "html" page (i.e. "blob" not 
> "blob_plain" view) also for binary files. <img> element for image/*
> mimetype, perhaps <embed> or <object>, or just plain link for other
> binary (not text/* or some application/*) types

We already do have a hash_base context. It is what allows us to "select" a revision from shortlog and ask "tree", "history", etc from there on, having set "h" and/or "hb".

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.

    Luben
Show 6 quoted lines
> 
> I was AFK for few days.
> -- 
> Jakub Narebski
> Poland
>
Jakub Narebski· Oct 1, 2006, 22:27 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

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.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Luben Tuikov· Oct 2, 2006, 19:51 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: tree view: hash_base and hash are now context sensitive

--- Jakub Narebski <jnareb@gmail.com> wrote:
Show 12 quoted lines
> 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.

← back to recent threads