threads / discuss / 24331

gitweb.perl suggestion

Subject: gitweb.perl suggestion

## tl;dr

5 messages between Jul 8, 2010 and Jul 8, 2010.

replies: 4people: 2as markdown or json

Eli Barzilay· Jul 8, 2010, 02:38 UTC · lore
Currently, if I go to
  http://server/gitweb/project/<commit-sha1>

I get to the shortlog page, which is not as useful as the commit page. But changing this to have a default `commit' action isn't right either since the shortlog is more appropriate with

  http://server/gitweb/project/<commit-sha1>..<other-commit-sha1>
So how about changing this:
  $input_params{'action'} ||= "shortlog";
to this:
  $input_params{'action'} ||= ((defined $parentrefname) ? "shortlog" : "commit");

which will make the first case show the commit, and the second show the shortlog?

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!
Jakub Narebski· Jul 8, 2010, 15:30 UTC · re: Eli Barzilay · lore

Re: gitweb.perl suggestion

Eli Barzilay <eli@barzilay.org> writes:
Show 20 quoted lines
> Currently, if I go to
> 
>   http://server/gitweb/project/<commit-sha1>
> 
> I get to the shortlog page, which is not as useful as the commit
> page.  But changing this to have a default `commit' action isn't right
> either since the shortlog is more appropriate with
> 
>   http://server/gitweb/project/<commit-sha1>..<other-commit-sha1>
> 
> So how about changing this:
> 
>   $input_params{'action'} ||= "shortlog";
> 
> to this:
> 
>   $input_params{'action'} ||= ((defined $parentrefname) ? "shortlog" : "commit");
> 
> which will make the first case show the commit, and the second show
> the shortlog?
Thanks for noticing and informing about this issue.

Unfortunately for having a fast fixup, you have hit upon larger issue. Namely how gitweb guesses action if there isn't provided one.

Currently there is one set of rules for evaluate_path_info (used with path_info URL like e.g. http://server/gitweb/project/<sha1>), and another in dispath() after evaluating path info and query params (query params version would be http://server/gitweb?p=project;h=<sha1>).

  path_info                    | action
  -----------------------------+--------------------------
  object:dirname/              | tree
  object:filename              | blob_plain
  objectA..objectB:filename    | blobdiff_plain
  object                       | shortlog
  objectA..objectB             | shortlog
  query params                 | action
  -----------------------------+---------------------------
  h=object                     | git_get_type(object)
  hb=object;f=filename [*]     | git_get_type(object:filename)

[*] There is no optimization that if it ends in '/' it is 'tree' object, and if it is not, then it is 'blob' (file) object.

Finally if project is defined, default action is 'summary', and if it isn't then default action is 'project_list'.

So your proposed solution is good enough, but perhaps better would be to leave 'action' unset if there is no parent info? Then dispatch would guess action, instead of doing it in less sophisticated way in evaluate_path_info().

-- 
Jakub Narebski
Poland
ShadeHawk on #git
Eli Barzilay· Jul 8, 2010, 17:34 UTC · re: Jakub Narebski · lore

Re: gitweb.perl suggestion

On Jul  8, Jakub Narebski wrote:
Show 5 quoted lines
> 
> So your proposed solution is good enough, but perhaps better would
> be to leave 'action' unset if there is no parent info?  Then
> dispatch would guess action, instead of doing it in less
> sophisticated way in evaluate_path_info().

Ah, looking at the dispatch point, I see what you're talking about. But that sounds like a larger change to the code -- since it would probably lead to more default actions that get determined in dispatch instead of earlier.

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!
Jakub Narebski· Jul 8, 2010, 19:01 UTC · re: Eli Barzilay · lore

Re: gitweb.perl suggestion

Eli Barzilay wrote:
Show 11 quoted lines
> On Jul  8, Jakub Narebski wrote:
> > 
> > So your proposed solution is good enough, but perhaps better would
> > be to leave 'action' unset if there is no parent info?  Then
> > dispatch would guess action, instead of doing it in less
> > sophisticated way in evaluate_path_info().
> 
> Ah, looking at the dispatch point, I see what you're talking about.
> But that sounds like a larger change to the code -- since it would
> probably lead to more default actions that get determined in dispatch
> instead of earlier.

I don't remember if it was the real reson why http://gitweb/project/<object> defaults to 'shortlog' action, but <object> can be commit, but can be also tag. The 'shortlog' action makes sense for both... assuming that tag points to a commit object, that is.

P.S. As usual, patches welcome...
-- 
Jakub Narebski
Poland
Eli Barzilay· Jul 8, 2010, 19:05 UTC · re: Jakub Narebski · lore

Re: gitweb.perl suggestion

On Jul  8, Jakub Narebski wrote:
Show 18 quoted lines
> Eli Barzilay wrote:
> > On Jul  8, Jakub Narebski wrote:
> > > 
> > > So your proposed solution is good enough, but perhaps better would
> > > be to leave 'action' unset if there is no parent info?  Then
> > > dispatch would guess action, instead of doing it in less
> > > sophisticated way in evaluate_path_info().
> > 
> > Ah, looking at the dispatch point, I see what you're talking about.
> > But that sounds like a larger change to the code -- since it would
> > probably lead to more default actions that get determined in dispatch
> > instead of earlier.
> 
> I don't remember if it was the real reson why
> http://gitweb/project/<object> defaults to 'shortlog' action, but
> <object> can be commit, but can be also tag.  The 'shortlog' action
> makes sense for both... assuming that tag points to a commit object,
> that is.
Ah, so it has to check the type of the object anyway...
> P.S. As usual, patches welcome...

(I can turn that one-line change into a patch, but for anything more substantial I'll actually need to deal with perl...)

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

← back to recent threads