Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> On Wed, Nov 27, 2013 at 3:34 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak
>> <krzesimir@xxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html