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

[Jeff King]

On Sat, Jan 21, 2017 at 07:48:50PM +0700, Duy Nguyen wrote:

> OK. Next question, how do we deal with the reflog count (i..e the
> argument of --decorate-remote-reflog). Should it be shared for all ref
> type, or can be specified differently for remote, local and tags? I'm
> leaning towards the former. But I'll wait a bit for ideas before
> rewriting the patch.

I doubt that anybody really cares about different reflog depths for
different refs. But I would say that the natural syntax ends up as:

  git log --decorate-reflog=10 --remotes \
  --decorate-reflog=10 --tags

anyway, so you get the ability to do it anyway "for free" (at the cost
of having to repeat yourself).

I guess the other option is:

  git log --decorate-reflog-depth=10 \
  --decorate-reflog --remotes
  --decorate-reflog --tags

That's actually _more_ typing, and besides being less flexible just
muddles the "is this option for the next ref-selector or not" question.

(The whole thing is obviously a lot of typing; I wonder if people would
want a config option to do this all the time).

-Peff

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

[Duy Nguyen]

On Sat, Jan 21, 2017 at 5:00 AM, Jacob Keller  wrote:
> On Fri, Jan 20, 2017 at 6:30 AM, Jeff King  wrote:
>>> 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.
>>
>>> 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.

OK. Next question, how do we deal with the reflog count (i..e the
argument of --decorate-remote-reflog). Should it be shared for all ref
type, or can be specified differently for remote, local and tags? I'm
leaning towards the former. But I'll wait a bit for ideas before
rewriting the patch.
--
Duy

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

[Jacob Keller]

On Fri, Jan 20, 2017 at 6:30 AM, Jeff King  wrote:
>> 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.
>
>> 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

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

[Jeff King]

On Fri, Jan 20, 2017 at 05:55:21PM +0700, Duy Nguyen wrote:

> > We already have very flexible ref-selectors like --remotes, --branches,
> > etc. The generalization of this would perhaps be something like:
> >
> >   git log --decorate-reflog --remotes --branches
> >
> > where "--decorate-reflog" applies to the next ref selector and then is
> > reset, the same way --exclude is. And it includes those refs _only_ for
> > decoration, not for traversal. So you could do:
> >
> >   git log --decorate-reflog --remotes --remotes
> >
> > if you wanted to see use those as traversal roots, too (if this is
> > common, it might even merit another option for "decorate and show").
> >
> > That's just off the top of my head, so maybe there are issues. I was
> > just surprised to see the "-remote" part in your option name.
> 
> 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.

> 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

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

[Duy Nguyen]

On Fri, Jan 20, 2017 at 12:23 AM, Jeff King  wrote:
> I think it's a neat idea, but the actual option:
>
>> +--decorate-remote-reflog[=]::
>> + Decorate `` most recent reflog entries on remote refs, up
>> + to the specified number of entries. By default, only the most
>> + recent reflog entry is decorated.
>
> seems weirdly limited and non-orthogonal. What happens when somebody
> wants to decorate other reflogs besides refs/remotes?
>
> We already have very flexible ref-selectors like --remotes, --branches,
> etc. The generalization of this would perhaps be something like:
>
>   git log --decorate-reflog --remotes --branches
>
> where "--decorate-reflog" applies to the next ref selector and then is
> reset, the same way --exclude is. And it includes those refs _only_ for
> decoration, not for traversal. So you could do:
>
>   git log --decorate-reflog --remotes --remotes
>
> if you wanted to see use those as traversal roots, too (if this is
> common, it might even merit another option for "decorate and show").
>
> That's just off the top of my head, so maybe there are issues. I was
> just surprised to see the "-remote" part in your option name.

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.

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.
-- 
Duy

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

[Jeff King]

On Thu, Jan 19, 2017 at 07:26:30PM +0700, Nguyễn Thái Ngọc Duy wrote:

> This is most useful when you fork your branches off a remote ref and
> rely on ref decoration to show your fork points in `git log`. Then you
> do a "git fetch" and suddenly the remote decoration is gone because
> remote refs are moved forward. With this, we can still see something
> like "origin/foo@{1}"
> 
> This is for remote refs only because based on my experience, docorating
> local reflog is just too noisy. You will most likely see HEAD@{1},
> HEAD@{2} and so on. We can add that as a separate option in future if we
> see a need for it.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  I've been using this for many weeks and it has proven its usefulness
>  (to me). Looks like good material to send upstream.

I think it's a neat idea, but the actual option:

> +--decorate-remote-reflog[=]::
> + Decorate `` most recent reflog entries on remote refs, up
> + to the specified number of entries. By default, only the most
> + recent reflog entry is decorated.

seems weirdly limited and non-orthogonal. What happens when somebody
wants to decorate other reflogs besides refs/remotes?

We already have very flexible ref-selectors like --remotes, --branches,
etc. The generalization of this would perhaps be something like:

  git log --decorate-reflog --remotes --branches

where "--decorate-reflog" applies to the next ref selector and then is
reset, the same way --exclude is. And it includes those refs _only_ for
decoration, not for traversal. So you could do:

  git log --decorate-reflog --remotes --remotes

if you wanted to see use those as traversal roots, too (if this is
common, it might even merit another option for "decorate and show").

That's just off the top of my head, so maybe there are issues. I was
just surprised to see the "-remote" part in your option name.

-Peff