2018-01-16 0:42 GMT+03:00 Jeff King <peff@xxxxxxxx>: > On Wed, Jan 10, 2018 at 09:36:41AM +0000, Olga Telezhnaya wrote: > >> Make valid_atom as a function parameter, >> there could be another variable further. >> Need that for further reusing of formatting logic in cat-file.c. >> >> We do not need to allow users to pass their own valid_atom variable in >> global functions like verify_ref_format because in the end we want to >> have same set of valid atoms for all commands. But, as a first step >> of migrating, I create further another version of valid_atom >> for cat-file. > > I agree in the end we'd want a single valid_atom list. It doesn't look > like we hit that end state in this series, though. > > I guess I'm not quite clear on why we're not adding these new atoms to > ref-filter (and for-each-ref) right away, though. We already have the > first three (name, type, and size), and we'd just need to support > %(rest) and %(deltabase). > > I think %(rest) doesn't really make sense for for-each-ref (we're not > reading any input), but it could expand to the empty string by default > (or even throw an error if the caller asks us not to support it). > > IOW, the progression I'd expect in a series like this is: > > 1. Teach ref-filter.c to support everything that cat-file can do. > > 2. Convert cat-file to use ref-filter.c. > > -Peff I agree, I even made this and it's working fine: https://github.com/git/git/pull/450/commits/1b74f1047f07434dccb207534d1ad45a143e3f2b But I decided not to add that to patch because I expand the functionality of several commands (not only cat-file and for-each-ref), and I need to support all new functionality in a proper way, make these error messages, test everything and - the hardest one - support many new commands for cat-file. As I understand, it is not possible unless we finally move to ref-filter and print results also there. Oh, and I also need to rewrite docs in that case. And I decided to apply this in another patch. But, please, say your opinion, maybe we could do that here in some way. Olga
