Karthik Nayak <karthik.188@xxxxxxxxx> writes: > On Thu, Oct 8, 2015 at 5:57 PM, Matthieu Moy > <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: >>> + unsigned int nobracket = 0; >>> + >>> + if (!strcmp(valp, ",nobracket")) >>> + nobracket = 1; >> >> The code to parse comma-separated values is different here and >> elsewhere. I'd rather have the same code (possibly factored into a >> helper function), both to get consistent behavior (you're not allowing >> %(upstream:nobracket,track) for example, right?) and consistent code. >> > > Speaking of comma-separated values, the only other place we use that is > in the align atom. Also I find this very specific to get a function out of. > Somehow I think this is the simplest way to go about this. Well, most pieces of code start with one instance, then two, then more ;-). When the second instance starts being different from the first, it doesn't give a good example for the future third instance. This particular piece of code is so important and I won't fight for a better factored one, but in general "there are only two instances" is a dubious argument to avoid refactoring. Still, I find it weird to force the nobracket to be always at the same position. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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