On Mon, Oct 17, 2022 at 8:38 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > "Jerry Zhang via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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 > >
