From: Junio C Hamano <gitster@pobox•com>
To: Eric Sunshine <sunshine@sunshineco•com>
Cc: Krzesimir Nowak <krzesimir@endocode•com>, Git List <git@vger•kernel.org>
Subject: Re: [PATCH v2] gitweb: Add an option for adding more branch refs
Date: Wed, 27 Nov 2013 12:55:59 -0800 [thread overview]
Message-ID: <xmqqob55sgww.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAPig+cR3hoWmDPJyWP6HbpuDvRwgdNj5VwX8A20DCfRBK5OuSQ@mail.gmail.com> (Eric Sunshine's message of "Wed, 27 Nov 2013 15:38:11 -0500")
Eric Sunshine <sunshine@sunshineco•com> writes:
> On Wed, Nov 27, 2013 at 3:34 PM, Eric Sunshine <sunshine@sunshineco•com> wrote:
>> On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak
>> <krzesimir@endocode•com> wrote:
>>> Overriding an @additional_branch_refs configuration variable with
>>> value ('wip') will make gitweb to show branches that appear in
>>> refs/heads and refs/wip (refs/heads is hardcoded).
"branches" are by definition what are in refs/heads/ hierarchy, so
Allow @additional_branch_refs configuration variable to tell
gitweb to show refs from additional hierarchies in addition to
branches in the list-of-branches view.
would be more appropriate and sufficient.
>> Mentioning $ref in the error message would help the user resolve the
>> problem more quickly.
>>
>>> + die_error(500, '"heads" specified in @additional_branch_refs') if ($ref eq 'heads');
>>
>> Rephrasing this as
>>
>> "heads" disallowed in @additional_branch_refs
>>
>> would better explain the problem to a user who has only made a cursory
>> read of the documentation.
>
> The program could easily filter out the redundant 'heads', so does
> this really deserve a diagnostic?
True.
I was primarily worried about metacharacters in the specified
strings getting in the way of regexp matches the new code allows on
them, but that has been resolved with the use of \Q..\E; if that
automatic deduping is done, I do not immediately see any remaining
issues in the latest round of the patch.
Thanks.
next prev parent reply other threads:[~2013-11-27 20:56 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-27 15:30 [PATCH v2] gitweb: Add an option for adding more branch refs Krzesimir Nowak
2013-11-27 20:34 ` Eric Sunshine
2013-11-27 20:38 ` Eric Sunshine
2013-11-27 20:55 ` Junio C Hamano [this message]
2013-11-28 11:49 ` Krzesimir Nowak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqob55sgww.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=krzesimir@endocode$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox