Re: [PATCH] advice: Add advice.scissors to suppress "do not modify or remove this line"

["Rubén Justo" <rjusto@gmail.com>]

On Sun, Feb 25, 2024 at 08:21:51PM -0800, Josh Triplett wrote:
> The scissors line before the diff in a verbose commit, or above all the
> comments when using --cleanup=scissors, has the following two lines of
> explanation after it:
> 
> Do not modify or remove the line above.
> Everything below it will be ignored.
> 
> This is useful advice for new users, but potentially redundant for
> experienced users, who might instead appreciate seeing two more lines of
> information in their editor.

Sounds sensible.

> 
> Add advice.scissors to suppress that explanation.

Perhaps "advice.scissorsHint" is a better name?  I'm very bad at
choosing names, but just "scissors" seems too generic to me.

> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  Documentation/config/advice.txt | 5 +++++
>  advice.c                        | 1 +
>  advice.h                        | 1 +
>  wt-status.c                     | 3 ++-
>  4 files changed, 9 insertions(+), 1 deletion(-)

Some tests would be desirable, to ensure that this keeps working in the
future.

> 
> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
> index c7ea70f2e2..33ab688b6c 100644
> --- a/Documentation/config/advice.txt
> +++ b/Documentation/config/advice.txt
> @@ -104,6 +104,11 @@ advice.*::
>  	rmHints::
>  		In case of failure in the output of linkgit:git-rm[1],
>  		show directions on how to proceed from the current state.
> +	scissors::

Good.  After "rmHints" and before "sequencerInUse".  Looks like the
right position for the new name.

> +		Advice shown by linkgit:git-commit[1] in the commit message
> +		opened in an editor, after a scissors line (containing >8),
> +		saying not to remove the line and that everything after the line
> +		will be ignored.
>  	sequencerInUse::
>  		Advice shown when a sequencer command is already in progress.
>  	skippedCherryPicks::
> diff --git a/advice.c b/advice.c
> index 6e9098ff08..0588012562 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -71,6 +71,7 @@ static struct {
>  	[ADVICE_RESET_NO_REFRESH_WARNING]		= { "resetNoRefresh" },
>  	[ADVICE_RESOLVE_CONFLICT]			= { "resolveConflict" },
>  	[ADVICE_RM_HINTS]				= { "rmHints" },
> +	[ADVICE_SCISSORS]				= { "scissors" },

Ditto.

>  	[ADVICE_SEQUENCER_IN_USE]			= { "sequencerInUse" },
>  	[ADVICE_SET_UPSTREAM_FAILURE]			= { "setUpstreamFailure" },
>  	[ADVICE_SKIPPED_CHERRY_PICKS]			= { "skippedCherryPicks" },
> diff --git a/advice.h b/advice.h
> index 9d4f49ae38..9725aa4199 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -39,6 +39,7 @@ enum advice_type {
>  	ADVICE_RESET_NO_REFRESH_WARNING,
>  	ADVICE_RESOLVE_CONFLICT,
>  	ADVICE_RM_HINTS,
> +	ADVICE_SCISSORS,

Ditto.

>  	ADVICE_SEQUENCER_IN_USE,
>  	ADVICE_SET_UPSTREAM_FAILURE,
>  	ADVICE_SKIPPED_CHERRY_PICKS,
> diff --git a/wt-status.c b/wt-status.c
> index 459d399baa..19d4986351 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1104,7 +1104,8 @@ void wt_status_append_cut_line(struct strbuf *buf)
>  	const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
>  
>  	strbuf_commented_addf(buf, comment_line_char, "%s", cut_line);
> -	strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);
> +	if (advice_enabled(ADVICE_SCISSORS))
> +		strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);

I wonder if advise_if_enabled() might have a chance here.  I'm just
thinking out loud.  "if(advice_enabled(.." is fine.

>  }
>  
>  void wt_status_add_cut_line(FILE *fp)
> -- 
> 2.43.0
> 

Thanks.