Re: [RFC PATCH 1/2] commit: don't add scissors line if one exists
- Date: Wed, 14 Nov 2018 13:06:08 -0500
- From: Denton Liu <liu.denton@xxxxxxxxx>
- Subject: Re: [RFC PATCH 1/2] commit: don't add scissors line if one exists
On Wed, Nov 14, 2018 at 05:06:32PM +0900, Junio C Hamano wrote:
> Denton Liu <liu.denton@xxxxxxxxx> writes:
> > If commit.cleanup = scissors is specified, don't produce a scissors line
> > if one already exists in the commit message.
> It is good that you won't have two such lines in the end result, but
> is this (1) hiding real problem under the rug? (2) losing information?
> If the current invocation of "git commit" added a scissors line in
> the buffer to be edited already, and we are adding another one in
> this function, is it possible that the real problem that somebody
> else has called wt_status_add_cut_line() before this function is
> called, in which case that other caller is what we need to fix,
> instead of this one?
In patch 2/2, I intentionally inserted a scissors line into MERGE_MSG so
this patch ensures that we don't get duplicate scissors.
> If the existing line in the buffer came from the end user (perhaps
> it was given from "-F <file>", etc., with "-e" option) or --amend,
> how can we be sure if it is OK to lose everything after that
> scissors looking line? In other words, the scissors looking line
> may just be part of the log message, in which case we may want to
> quote/escape it, so that the true scissors we append at a later
> place in the buffer would be noticed without losing the text before
> and after that scissors looking line we already had when this
> function was called?
With the existing behaviour, any messages that contain a scissors
looking line will get cut at the earliest scissors anyway, so I believe
that this patch would not change the behaviour. If the users were
dealing with commit messages with a scissors looking line, the current
behaviour already requires users to be extra careful to ensure that the
scissors don't get accidentally removed so in the interest of preserving
the existing behaviour, I don't think that any extra information would
be lost from this patch.