Josh Triplett <josh@xxxxxxxxxxxxxxxx> writes:
> `git commit --cleanup=scissors -v` currently prints two scissors lines:
> one at the start of the comment lines, and the other right before the
> diff. This is redundant, and pushes the diff further down in the user's
> editor than it needs to be.
Interesting discovery.
> diff --git a/wt-status.c b/wt-status.c
> index b5a29083df..459d399baa 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1143,11 +1143,13 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
> * file (and even the "auto" setting won't work, since it
> * will have checked isatty on stdout). But we then do want
> * to insert the scissor line here to reliably remove the
> - * diff before committing.
> + * diff before committing, if we didn't already include one
> + * before.
> */
> if (s->fp != stdout) {
> rev.diffopt.use_color = 0;
> - wt_status_add_cut_line(s->fp);
> + if (s->cleanup_mode != COMMIT_MSG_CLEANUP_SCISSORS)
> + wt_status_add_cut_line(s->fp);
> }
The machinery to populate the log message buffer should ideally be
taught to remember if it already has added a scissors-line and to
refrain from adding redundant ones. That way, we do not have to
rely on the order of places that make wt_status_add_cut_line() calls
or what condition they use to decide to make these calls.
This hunk for example knows not just this one produces cut-line
after the other one potentially added one, but also the logic used
by the other one to decide to add one, which is even worse. I find
the solution presented here a bit unsatisfactory, for this reason,
but for now it may be OK, as we probably are not adding any more
places and conditions to emit a scissors line.
> builtin/commit.c | 2 ++
> sequencer.h | 7 -------
> wt-status.c | 6 ++++--
> wt-status.h | 8 ++++++++
> 4 files changed, 14 insertions(+), 9 deletions(-)
If this change did not break any existing tests that checked the
combination of options and output when they are used together, it
means we have a gap in the test coverage. We needs a test or two
to protect this fix from future breakages.
Thanks.
