Re: [PATCH] log: new option decorate reflog of remote refs

On Fri, Jan 20, 2017 at 6:30 AM, Jeff King [off-list ref] wrote:

quoted

Imposing order between options could cause confusion, I think, if you
remove --decorate-reflog leaving --remotes on by accident, now you get
--remotes with a new meaning. We could go with something like
--decodate-reflog=remote, but that clashes with the number of reflog
entries and we may need a separator, like --decorate-reflog=remote,3.
Or we could add something to --decorate= in addition to
short|full|auto|no. Something like --decorate=full,reflog or
--decorate=full,reflog=remote,entries=3 if I want 3 reflog entries.

I agree that making option-order important is potentially confusing. But
it does already exist with --exclude. It's necessary to specify some
sets of refs (e.g., all of A, except for those that match B, and then
all of C, including those that match B).

Having --decorate-reflog=remote would be similarly constrained. You
couldn't do "decorate all remotes except for these ones". For that
matter, I'm not sure how you would do "decorate just the refs from
origin".

I'll grant that those are going to be a lot less common than just "all
the remotes" (or all the tags, or whatever). I'd just hate to see us
revisiting this in a year to generalize it, and being stuck with
historical baggage.

quoted

My hesitant to go that far is because I suspect decorating reflog
won't be helpful for non-remotes. But I'm willing to make more changes
if it opens door to master.

Forgetting reflogs for a moment, I'd actually find it useful to just
decorate tags and local branches, but not remotes. But right now there
isn't any way to select which refs are worthy of decoration (reflog or
not).

That's why I'm thinking so much about a general ref-selection system. I
agree the "--exclude=... --remotes" thing is complicated, but it's also
the ref-selection system we _already_ have, which to me is a slight
point in its favor.

-Peff

I agree that the interaction between --exclude and --remotes/etc is
confusing, but I think it's reasonable enough because we already
support it, so it makes sense to extend it with this. I also think its
better to extend here than it is to hard-code it. We could provide a
single short-option that does the longer variant if it's that common.

Thanks,
Jake