threads / rfc / 43599

patchRe: [PATCH/RFC] gitweb: Default to $hash_base or HEAD for $hash in "commit" and "commitdiff" views

Subject: Re: [PATCH/RFC] gitweb: Default to $hash_base or HEAD for $hash in "commit" and "commitdiff" views

## tl;dr

4 messages between Nov 8, 2006 and Nov 9, 2006. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Jakub Narebski· Nov 8, 2006, 22:11 UTC · lore

[PATCH/RFC] gitweb: Default to $hash_base or HEAD for $hash in "commit" and "commitdiff" views

Set $hash parameter to $hash_base || "HEAD" if it is not set (if it is not true to be more exact). This allows [hand-edited] URLs with 'action' "commit" or "commitdiff" but without 'hash' parameter.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
RFC because I want to ask if we should default to HEAD if hash is not
provided for commit and commitdiff views, or should we error out with
more reasonable error message.

 gitweb/gitweb.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Show changes to gitweb/gitweb.perl +2 −0

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3118cb0..8313517 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3391,6 +3391,7 @@ sub git_log {
 }
 
 sub git_commit {
+	$hash ||= $hash_base || "HEAD";
 	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
@@ -3668,6 +3669,7 @@ sub git_blobdiff_plain {
 
 sub git_commitdiff {
 	my $format = shift || 'html';
+	$hash ||= $hash_base || "HEAD";
 	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");

-- 
1.4.3.4

Junio C Hamano· Nov 8, 2006, 22:21 UTC · re: Jakub Narebski · lore

Jakub Narebski <jnareb@gmail.com> writes:

Show 9 quoted lines

> Set $hash parameter to $hash_base || "HEAD" if it is not set (if it is
> not true to be more exact). This allows [hand-edited] URLs with 'action'
> "commit" or "commitdiff" but without 'hash' parameter.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> RFC because I want to ask if we should default to HEAD if hash is not
> provided for commit and commitdiff views, or should we error out with
> more reasonable error message.

Probably the latter; the user might be hand-crafting a URL (maybe learned a commit object name from mailing list and cutting and pasting) and mispasted the long hexadecimal string. Silently giving HEAD may leave the user confused than "oops, we do not see that commit object".

Jakub Narebski· Nov 8, 2006, 22:35 UTC · re: Junio C Hamano · lore

Junio C Hamano wrote:

Show 17 quoted lines

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Set $hash parameter to $hash_base || "HEAD" if it is not set (if it is
>> not true to be more exact). This allows [hand-edited] URLs with 'action'
>> "commit" or "commitdiff" but without 'hash' parameter.
>>
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>> ---
>> RFC because I want to ask if we should default to HEAD if hash is not
>> provided for commit and commitdiff views, or should we error out with
>> more reasonable error message.
> 
> Probably the latter; the user might be hand-crafting a URL
> (maybe learned a commit object name from mailing list and
> cutting and pasting) and mispasted the long hexadecimal string.
> Silently giving HEAD may leave the user confused than "oops, we
> do not see that commit object".

 
No, if there is 'h' (hash) parameter provided, then gitweb tries
to use this. HEAD is used _only_ if nether hash, nor hash_base
are provided, i.e. for URL like below

  URL?p=project.git;a=commit

i.e. without neither 'h' nor 'hb'.

But if it is (it being defaulting to HEAD, _like git_) wrong solution, still it would be better to show 'commit not provided' error instead of current 'bad commit' error.

-- 
Jakub Narebski

Junio C Hamano· Nov 9, 2006, 00:09 UTC · re: Jakub Narebski · lore

Jakub Narebski <jnareb@gmail.com> writes:

Show 13 quoted lines

>> Probably the latter; the user might be hand-crafting a URL
>> (maybe learned a commit object name from mailing list and
>> cutting and pasting) and mispasted the long hexadecimal string.
>> Silently giving HEAD may leave the user confused than "oops, we
>> do not see that commit object".
>  
> No, if there is 'h' (hash) parameter provided, then gitweb tries
> to use this. HEAD is used _only_ if nether hash, nor hash_base
> are provided, i.e. for URL like below
>
>   URL?p=project.git;a=commit
>
> i.e. without neither 'h' nor 'hb'.

Ah, that one I agree it makes sense to use HEAD. Also I am all for giving a more readable error message.