Re: [PATCHv5 06/12] gitweb: allow extra text after action in page header
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Sep 27, 2010, 07:42 UTC
- Message-ID
- <201009270942.14731.jnareb@gmail.com>
- In-Reply-To
- <AANLkTinpZ5gusCG4SoKbi54URyn7T4THNp2NeVM6uFL2@mail.gmail.com>
On Mon, 27 Sep 2010, Giuseppe Bilotta wrote:
> 2010/9/26 Jakub Narebski <jnareb@gmail.com>: >> On Fri, 24 Sep 2010, Giuseppe Bilotta wrote:
Show 11 quoted lines
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index e70897e..76cf806 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -3514,7 +3514,15 @@ EOF
>>> if (defined $project) {
>>> print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
>>> if (defined $action) {
>>> - print " / $action";
>>> + my $action_print = $action ;
>>> + if (defined $opts{'header_extra'}) {[...]
Show 9 quoted lines
>> I also think that we can think of better name for this option than >> 'header_extra', although what this name could be eludes me. > > I will add the dash to the option. Naming it header_extra keeps the > meaning of this extra text generic, but considering that the intended > use is mostly for the single-remote view (or similar, if/when they are > added) we could call it something related (I can only think of > 'main_argument' right now but I think this would suck more than > header_extra).
Perhaps name this argument '-subaction' or '-action_argument', or something like that; the meaning of this argument (as shown by the fact that action name is hyperlinked) is to specify subaction of given action, i.e. (possibly) more detailed view of some part of generic (argument-less) action output.
Show 12 quoted lines
>>> + }
>>> + print " / $action_print";
>>> + }
>>> + if (defined $opts{'header_extra'}) {
>>> + print " / $opts{'header_extra'}";
>>
>> Hmmm...
>
> You don't sound very convinced. I had some doubts myself about whether
> the slash should be inserted autmatically or whether it should be up
> to the caller to include it in header_extra, but I'm not sure this is
> what you are perplexed about.Ah, I'm sorry, I have misunderstood the control flow here. I see now that it is about adding subaction specifier after action, so that 'remotes' view for single remote 'origin' (<URL>/remotes/origin path_info URL, see comments for patch introducing single-remote view) has
_projects_ / _repo.git_ / _remotes_ / origin
in the "breadcrumbs" navigation in page header.
This should be better described in the commit message.
-- Jakub Narebski Poland