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

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

[Jakub Narebski]

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

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

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

[Junio C Hamano]

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

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

[Jakub Narebski]

Junio C Hamano wrote:
> 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

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

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

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