Re: [PATCH] branch: optionally setup branch.*.merge from upstream local branches

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2008, 00:55 UTC
Message-ID: <7vzltwe9a4.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <7v4pc4fo6y.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 28 quoted lines

> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> Well, it could also mean that there is no rules yet, and you can
>> do the next sane thing of your choice.
>>
>>> enum color_branch {
>>> 	COLOR_BRANCH_RESET = 0,
>>> 	COLOR_BRANCH_PLAIN = 1,
>>> 	COLOR_BRANCH_REMOTE = 2,
>>> 	COLOR_BRANCH_LOCAL = 3,
>>> 	COLOR_BRANCH_CURRENT = 4,
>>> };
>
> This enum is used as an index into branch_colors[] array.  ...
> ...  But we would want to leave a clue for people who would
> want to touch this later that individual values have some
> meaning, more than just that they have to be distinct.
> ...
>>> enum CAPABILITY {
>>> 	NOLOGIN = 0,
>>> 	UIDPLUS,
>>> 	LITERALPLUS,
>>> 	NAMESPACE,
>>> };
>
> This seems to be meant to match the order in the corresponding
> cap_list[] array, so this cannot be reshuffled (iow, it is
> similar to color_branch).

Side note. My preference for enum that indexes an array is to use the latter, not spelling everything out like "color_branch" does. The first one being 0 and everybody else increments by 1 is a good enough clue that you cannot arbitrarily reshuffle them (as opposed to everbody left to the default). After being warned with the clue, somebody who is adding new elements to the array and defining a symbolic index to the element will know to append to the list, not just insert into an arbitrary place. Spelling out all the rest explicitly like "color_branch" one does is wasteful (new entries would need to carry "= (n+1)"), pointless (you cannot write anything but "= (n+1)" because it is an array index), and misleading (it makes you wonder if the specific values have meaning other than being used for an array index).