Re: [PATCH] Change "refs/" references to symbolic constants

[Andy Parkins <andyparkins@gmail.com>]

On Tuesday 2007 October 02, Jeff King wrote:

> Perhaps a better quest would be to eliminate all of those counts
> entirely with code that is obviously correct. I think it is much more
> readable to replace:

I've got a patch replacing every appropriate memcmp() with prefixcmp(), but it 
goes on top of this one, so wanted to get this through review to save 
constantly spamming the list with the same patch slightly modified because of 
changes in a different patch.

>   url = xmalloc(strlen(repo->base) + 64);
>   sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
>
> with something like:
>
>   strbuf_init(&url);
>   strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);

I've not been following the strbuf() changes, so have missed the appearance of 
these handy new functions.  They would appear to be an improvement for cases 
just like this.

> > constants in CAPITAL_LETTERS_WITH_UNDERSCORE shout too loudly to
>
> Part of the problem is also that they're long. Perhaps REFS_HEADS, while
> being less unique in the C namespace, would look better?

I completely agree with the length and loudness concerns, but my worry was 
polluting the namespace while maintaining some sort of rationality between 
PATH_REFS_HEADS and STRLEN_PATH_REFS_HEADS.  My reasoning was that

 "refs/heads" -> PATH_REFS_HEADS

is only three extra characters, and

 strlen("refs/heads/") -> STRLEN_PATH_REFS_HEADS

is only one extra character.

However I have no strong feelings about changing them.



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com