Re: [PATCH 17/36] attr: expose validity check for attribute names
- From
Stefan Beller <sbeller@google.com>
- Date
- Oct 24, 2016, 21:07 UTC
- Message-ID
- <CAGZ79ka_zr_NXKoxC45swFrj168fP6S7_nQ1jjcfPOtTN4Jd1A@mail.gmail.com>
- In-Reply-To
- <0425fea3-3419-c265-b964-f5a309b867fa@ramsayjones.plus.com>
On Sun, Oct 23, 2016 at 8:07 AM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
Show 30 quoted lines
> > > On 23/10/16 00:32, Stefan Beller wrote: >> From: Junio C Hamano <gitster@pobox.com> >> >> Export attr_name_valid() function, and a helper function that >> returns the message to be given when a given <name, len> pair >> is not a good name for an attribute. >> >> We could later update the message to exactly spell out what the >> rules for a good attribute name are, etc. >> >> Signed-off-by: Junio C Hamano <gitster@pobox.com> >> Signed-off-by: Stefan Beller <sbeller@google.com> >> --- > > [snip] > >> +extern int attr_name_valid(const char *name, size_t namelen); >> +extern void invalid_attr_name_message(struct strbuf *, const char *, int); >> + > > The symbol 'attr_name_valid()' is not used outside of attr.c, even > by the end of this series. Do you expect this function to be used > in any future series? (The export is deliberate and it certainly > seems like it should be part of the public interface, but ...) > > In contrast, the 'invalid_attr_name_message()' function is called > from code in pathspec.c, which relies on 'git_attr_counted()' to > call 'attr_name_valid()' internally to check for validity. :-D
Yeah, I am taking over Junios patches and do not quite implement what Junio thought I would. ;) So I guess it is a communication mismatch.
git_attr_counted is a wrapper around attr_name_valid in the way that it either returns NULL when the attr name is invalid or it does extra work and returns a pointer to an attr.
So I think for API completeness we'd want to keep attr_name_valid around, as otherwise the API looks strange. But that doesn't seem like a compelling reason, so I'll drop it from the header file and make it static.
Thanks, Stefan