Re: [PATCH] blame: add the ability to ignore commits

From: Stefan Beller <sbeller@google.com>
Date: Jan 9, 2019, 21:21 UTC
Message-ID: <CAGZ79kYFC+ws00isp0F1kxmxVx0geg+R0VjjRf0=u2ocbkFUUQ@mail.gmail.com>
In-Reply-To: <xmqqimyz57l9.fsf@gitster-ct.c.googlers.com>

On Tue, Jan 8, 2019 at 10:26 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 25 quoted lines

>
> Barret Rhoden <brho@google.com> writes:
>
> >> A policy decision like the above two shouldn't be hardcoded in the
> >> feature like this, but should be done as a separate option.  By
> >> default, these shouldn't be marked with '*', as the same tools you
> >> said you are afraid of breaking would be expecting a word with only
> >> digits and no asterisk in the column where line numbers appear and
> >> will get broken by this change if done unconditionally.
> >
> > Since users are already opting-in to the blame-ignore, do you also want
> > them to opt-in to the annotation?
>
> Absolutely.
>
> After all, the users of a moral equivalent that is -S
> never needed such an extra annotation, which tells us two things.
> (1) the claim "It's useful to be alerted to the presence of an
> ignored commit" in the proposed log message is merely a personal
> preference and universal users' requirement; (2) if it is useful to
> mark a blame-source whose parents (used while blaming) do not match
> the actual parents, such an annotation would also be useful while
> running -S.  So probably it should be a separate option that can be
> given when any of the --skip-commit=<rev>, --skip-commits-file=<file>,
> r -S<file> option is given.

From a users point of view it may be desirable to express all this
in the grafts file, i.e. -S <file> where the syntax of that file is extended.
For example we could introduce
    !<hash>
to make it exclude that commit.
Of course this could lead to confusion, as this puts 2 very different
concepts into the same option/file.

Speaking of the implementation: This patch proposes an oid-set that is handled specially in blame itself. I wonder if this could be generalized.

Jonathan Tan (cc'd) refactored and extended revision walking for git-fetch and its negotiation leading to 7c85ee6c58 (Merge branch 'jt/fetch-negotiator-skipping', 2018-08-02), and 3390e42adb (fetch-pack: support negotiation tip whitelist, 2018-07-02) which implements another revision walking algorithm that can be used to fine-tune revisions walked when fetching.

I wonder if that work could be generalized more to have "generic" revision walking algorithms and then making use of them in either fetch or blame.

For git-fetch there is a new algorithm that increases step size between commits, which would be funny to try for blame here. It would give the wrong blamed commit, but would speed up blaming a lot.

Omitting some revisions seems to be applicable to more than just blame/fetch, too.

Stefan