On Mon, Jul 27, 2015 at 6:17 PM, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> Make the `color` atom a pseudo atom and ensure that it uses >> `ref_formatting_state`. > > Actually, I think this is an incorrect change. > > Previously, %(color:...) was taking effect immediately, and after this > patch, it takes effect starting from the next atom. > > Try this: > > git for-each-ref --format '%(color:red)red %(color:reset)normal' > > Before your patch, I get 'red' as red, and 'normal' as normal. After > your patch, I get no color at all, since %(color:...) just stores > information that is never used because I have no real atom. > >> --- a/ref-filter.h >> +++ b/ref-filter.h >> @@ -19,11 +19,13 @@ >> struct atom_value { >> const char *s; >> unsigned long ul; /* used for sorting when not FIELD_STR */ >> - unsigned int pseudo_atom : 1; /* atoms which aren't placeholders for ref attributes */ >> + unsigned int pseudo_atom : 1, /* atoms which aren't placeholders for ref attributes */ >> + color : 1; >> }; > > As a consequence of the remark above, I think the name and comment of > the field are misleading. There are 3 kinds of atoms: > > * Placeholders for ref attributes > > * Atoms that take action immediately, but that are not ref attributes > like %(color) > > * Atoms that only act as modifiers for the next atom. Perhaps they could > be called "prefix atoms" or "modifier atoms". > Thats right, Junio and I missed out the second part and overlapped it over with the third part. Thanks for seeing it through. -- Regards, Karthik Nayak -- 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