Re: [PATCH v3 5/7] builtin: patch-id: add --include-whitespace as a command mode

[Jerry Zhang <jerry@skydio.com>]

On Mon, Oct 17, 2022 at 8:38 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Jerry Zhang via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> >> +--include-whitespace::
> >> +    Use the "stable" algorithm described below and also don't strip whitespace
> >> +    from lines when calculating the patch-id.
> >> +
> >> +    This is the default if patchid.includeWhitespace is true and implies
> >> +    patchid.stable.
> >
> > This seems very much orthogonal to "--stable/--unstable.
> >
> > Because the "--stable" variant is more expensive than "--unstable",
>
> Sorry, I misspoke.  The way we make the result stable is *not* by
> enforcing a fixed order to the input of the hash (which would have
> been more expensive), but by hashing each file separately and
> summing up the hashes, and it shouldn't be noticeably more expensive
> than the unstable variant.
>
> So, I do not think I mind if we introduced --include-whitespace as a
> third option in addition to --stable and --unstable, instead of allowing
> it to be combined with both --stable and --unstable.
>
> But I wonder:
>
>  * (minor) Would "--verbatim" work as a better option name?  The
>    name "--include-whitespace" can apply even to an implementation
>    that squashes multiple consecutive spaces and tabs into a single
>    space, i.e. we keep words on a single line as separate words,
>    instead of squishing them together, when hashing.
>
>  * Do users even care about the internal reliance on the "stable"
>    algorithm?  Wouldn't it be better to leave such an implementation
>    detail unsaid?  After all, "--verbatim --unstable" would not work
>    as they expect.
>
> So I would suggest dropping "and implies patchid.stable" from the
> above description.
Sure I can spin a new series with these changes
>
>