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

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

[Luben Tuikov]

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

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

[-- Attachment #2: 1908141687-p2.txt --]
[-- Type: text/plain, Size: 1075 bytes --]

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

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

[Jakub Narebski]

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

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

[Luben Tuikov]

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

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

[Junio C Hamano]

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?

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

[Luben Tuikov]

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

   Luben

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

[Luben Tuikov]

--- Luben Tuikov <ltuikov@yahoo.com> wrote:
> --- 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

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

[Junio C Hamano]

Luben Tuikov <ltuikov@yahoo.com> writes:

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

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

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

[Luben Tuikov]

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

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

[Luben Tuikov]

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

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 |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

[-- Attachment #2: 1207600725-p1.txt --]
[-- Type: text/plain, Size: 1129 bytes --]

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index be336f6..44991b1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1672,9 +1672,9 @@ sub git_print_tree_entry {
 			              "history");
 		}
 		print " | " .
-		      $cgi->a({-href => href(action=>"blob_plain",
-		                             hash=>$t->{'hash'}, file_name=>"$basedir$t->{'name'}")},
-		              "raw");
+			$cgi->a({-href => href(action=>"blob_plain", hash_base=>$hash_base,
+					       file_name=>"$basedir$t->{'name'}")},
+				"raw");
 		print "</td>\n";
 
 	} elsif ($t->{'type'} eq "tree") {
@@ -2745,14 +2745,14 @@ sub git_blob {
 sub git_tree {
 	my $have_snapshot = gitweb_have_snapshot();
 
+	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";

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

[Jakub Narebski]

Luben Tuikov wrote:
> --- 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

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

[Luben Tuikov]

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


> 
> I was AFK for few days.
> -- 
> Jakub Narebski
> Poland
> 

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

[Jakub Narebski]

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

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

[Luben Tuikov]

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