Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Karthik Nayak writes: > Introduce match_atom_name() which helps in checking if a particular > atom is the atom we're looking for and if it has a value attached to > it or not. > > Use it instead of starts_with() for checking the value of %(color:...) > atom. Write a test

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

On Thu, Sep 10, 2015 at 10:26 PM, Junio C Hamano wrote: > Karthik Nayak writes: > >> Introduce match_atom_name() which helps in checking if a particular >> atom is the atom we're looking for and if it has a value attached to >> it or not. >> >> Use it

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Matthieu Moy writes: > OTOH, you are now accepting %(atom:) as a synonym to %(atom), and it's > not clear whether this is a deliberate decition. I would say so. When the caller wants to reject %(atom:), the caller can tell it by checking val[0] == '\0' and reject

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Junio C Hamano writes: > Karthik Nayak writes: > >> -} else if (starts_with(name, "color:")) { >> +} else if (match_atom_name(name, "color", )) { > > Why use the helper only for this one? Aren't existing calls to > starts_with()

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Karthik Nayak writes: > Introduce match_atom_name() which helps in checking if a particular > atom is the atom we're looking for and if it has a value attached to > it or not. > > Use it instead of starts_with() for checking the value of %(color:...) > atom. Write a test

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Karthik Nayak writes: > The check was for checking if there is anything after the colon, Why do you even care? If %(color) expects more specific customization by adding colon followed by specific data after it, i.e. %(color:something), %(color:) should clearly be that

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

On Thu, Sep 10, 2015 at 10:58 PM, Junio C Hamano wrote: > Karthik Nayak writes: > >> The check was for checking if there is anything after the colon, > > Why do you even care? If %(color) expects more specific > customization by adding colon followed by

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Karthik Nayak writes: > It is one thing that the user can actually do the check themselves, > but doesn't it make more sense that when we're using colon we expect a > value after it, and something like %(color:) makes no sense when color > specifically needs a value after

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

On Thu, Sep 10, 2015 at 11:15 PM, Junio C Hamano wrote: > Karthik Nayak writes: > >> It is one thing that the user can actually do the check themselves, >> but doesn't it make more sense that when we're using colon we expect a >> value after it, and

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Matthieu Moy writes: > Junio C Hamano writes: > >> Karthik Nayak writes: >> >>> - } else if (starts_with(name, "color:")) { >>> + } else if (match_atom_name(name, "color", )) { >> >> Why use the helper

Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

Junio C Hamano writes: > Matthieu Moy writes: > >> OTOH, you are now accepting %(atom:) as a synonym to %(atom), and it's >> not clear whether this is a deliberate decition. > > I would say so. When the caller wants to reject %(atom:), the >