{"thread":{"id":"61002","subject":"[PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","startedAt":"2024-02-26T04:21:54Z","lastAt":"2024-04-17T04:00:14Z","messageCount":8,"participants":["Josh Triplett","Rubén Justo","Junio C Hamano","rsbecker@nexbridge.com","Dragan Simic"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"489345","messageId":"0a7b9172add0a0107e0765a59a798b92161788dd.1708921148.git.josh@joshtriplett.org","threadId":"61002","inReplyTo":null,"subject":"[PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-02-26T04:21:51Z","receivedAt":"2024-02-26T04:21:54Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"The scissors line before the diff in a verbose commit, or above all the\ncomments when using --cleanup=scissors, has the following two lines of\nexplanation after it:\n\nDo not modify or remove the line above.\nEverything below it will be ignored.\n\nThis is useful advice for new users, but potentially redundant for\nexperienced users, who might instead appreciate seeing two more lines of\ninformation in their editor.\n\nAdd advice.scissors to suppress that explanation.\n\nSigned-off-by: Josh Triplett \n---\n Documentation/config/advice.txt | 5 +++++\n advice.c | 1 +\n advice.h | 1 +\n wt-status.c | 3 ++-\n 4 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex c7ea70f2e2..33ab688b6c 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -104,6 +104,11 @@ advice.*::\n \trmHints::\n \t\tIn case of failure in the output of linkgit:git-rm[1],\n \t\tshow directions on how to proceed from the current state.\n+\tscissors::\n+\t\tAdvice shown by linkgit:git-commit[1] in the commit message\n+\t\topened in an editor, after a scissors line (containing >8),\n+\t\tsaying not to remove the line and that everything after the line\n+\t\twill be ignored.\n \tsequencerInUse::\n \t\tAdvice shown when a sequencer command is already in progress.\n \tskippedCherryPicks::\ndiff --git a/advice.c b/advice.c\nindex 6e9098ff08..0588012562 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -71,6 +71,7 @@ static struct {\n \t[ADVICE_RESET_NO_REFRESH_WARNING]\t\t= { \"resetNoRefresh\" },\n \t[ADVICE_RESOLVE_CONFLICT]\t\t\t= { \"resolveConflict\" },\n \t[ADVICE_RM_HINTS]\t\t\t\t= { \"rmHints\" },\n+\t[ADVICE_SCISSORS]\t\t\t\t= { \"scissors\" },\n \t[ADVICE_SEQUENCER_IN_USE]\t\t\t= { \"sequencerInUse\" },\n \t[ADVICE_SET_UPSTREAM_FAILURE]\t\t\t= { \"setUpstreamFailure\" },\n \t[ADVICE_SKIPPED_CHERRY_PICKS]\t\t\t= { \"skippedCherryPicks\" },\ndiff --git a/advice.h b/advice.h\nindex 9d4f49ae38..9725aa4199 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -39,6 +39,7 @@ enum advice_type {\n \tADVICE_RESET_NO_REFRESH_WARNING,\n \tADVICE_RESOLVE_CONFLICT,\n \tADVICE_RM_HINTS,\n+\tADVICE_SCISSORS,\n \tADVICE_SEQUENCER_IN_USE,\n \tADVICE_SET_UPSTREAM_FAILURE,\n \tADVICE_SKIPPED_CHERRY_PICKS,\ndiff --git a/wt-status.c b/wt-status.c\nindex 459d399baa..19d4986351 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1104,7 +1104,8 @@ void wt_status_append_cut_line(struct strbuf *buf)\n \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n \n \tstrbuf_commented_addf(buf, comment_line_char, \"%s\", cut_line);\n-\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n+\tif (advice_enabled(ADVICE_SCISSORS))\n+\t\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n }\n \n void wt_status_add_cut_line(FILE *fp)\n-- \n2.43.0\n\n"},{"id":"493000","messageId":"Zh7NZhB1n2o1k7jI@localhost","threadId":"61002","inReplyTo":"0a7b9172add0a0107e0765a59a798b92161788dd.1708921148.git.josh@joshtriplett.org","subject":"Re: [PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-04-16T19:11:34Z","receivedAt":"2024-04-16T19:11:37Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Sun, Feb 25, 2024 at 08:21:54PM -0800, Josh Triplett wrote:\n> The scissors line before the diff in a verbose commit, or above all the\n> comments when using --cleanup=scissors, has the following two lines of\n> explanation after it:\n> \n> Do not modify or remove the line above.\n> Everything below it will be ignored.\n> \n> This is useful advice for new users, but potentially redundant for\n> experienced users, who might instead appreciate seeing two more lines of\n> information in their editor.\n> \n> Add advice.scissors to suppress that explanation.\n\nFollowing up on this patch. Happy to rework if needed.\n"},{"id":"493007","messageId":"6d90c73b-0026-4715-a00a-084da7ac2c75@gmail.com","threadId":"61002","inReplyTo":"0a7b9172add0a0107e0765a59a798b92161788dd.1708921148.git.josh@joshtriplett.org","subject":"Re: [PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-04-16T20:28:55Z","receivedAt":"2024-04-16T20:28:58Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Sun, Feb 25, 2024 at 08:21:51PM -0800, Josh Triplett wrote:\n> The scissors line before the diff in a verbose commit, or above all the\n> comments when using --cleanup=scissors, has the following two lines of\n> explanation after it:\n> \n> Do not modify or remove the line above.\n> Everything below it will be ignored.\n> \n> This is useful advice for new users, but potentially redundant for\n> experienced users, who might instead appreciate seeing two more lines of\n> information in their editor.\n\nSounds sensible.\n\n> \n> Add advice.scissors to suppress that explanation.\n\nPerhaps \"advice.scissorsHint\" is a better name? I'm very bad at\nchoosing names, but just \"scissors\" seems too generic to me.\n\n> \n> Signed-off-by: Josh Triplett \n> ---\n> Documentation/config/advice.txt | 5 +++++\n> advice.c | 1 +\n> advice.h | 1 +\n> wt-status.c | 3 ++-\n> 4 files changed, 9 insertions(+), 1 deletion(-)\n\nSome tests would be desirable, to ensure that this keeps working in the\nfuture.\n\n> \n> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\n> index c7ea70f2e2..33ab688b6c 100644\n> --- a/Documentation/config/advice.txt\n> +++ b/Documentation/config/advice.txt\n> @@ -104,6 +104,11 @@ advice.*::\n> \trmHints::\n> \t\tIn case of failure in the output of linkgit:git-rm[1],\n> \t\tshow directions on how to proceed from the current state.\n> +\tscissors::\n\nGood. After \"rmHints\" and before \"sequencerInUse\". Looks like the\nright position for the new name.\n\n> +\t\tAdvice shown by linkgit:git-commit[1] in the commit message\n> +\t\topened in an editor, after a scissors line (containing >8),\n> +\t\tsaying not to remove the line and that everything after the line\n> +\t\twill be ignored.\n> \tsequencerInUse::\n> \t\tAdvice shown when a sequencer command is already in progress.\n> \tskippedCherryPicks::\n> diff --git a/advice.c b/advice.c\n> index 6e9098ff08..0588012562 100644\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -71,6 +71,7 @@ static struct {\n> \t[ADVICE_RESET_NO_REFRESH_WARNING]\t\t= { \"resetNoRefresh\" },\n> \t[ADVICE_RESOLVE_CONFLICT]\t\t\t= { \"resolveConflict\" },\n> \t[ADVICE_RM_HINTS]\t\t\t\t= { \"rmHints\" },\n> +\t[ADVICE_SCISSORS]\t\t\t\t= { \"scissors\" },\n\nDitto.\n\n> \t[ADVICE_SEQUENCER_IN_USE]\t\t\t= { \"sequencerInUse\" },\n> \t[ADVICE_SET_UPSTREAM_FAILURE]\t\t\t= { \"setUpstreamFailure\" },\n> \t[ADVICE_SKIPPED_CHERRY_PICKS]\t\t\t= { \"skippedCherryPicks\" },\n> diff --git a/advice.h b/advice.h\n> index 9d4f49ae38..9725aa4199 100644\n> --- a/advice.h\n> +++ b/advice.h\n> @@ -39,6 +39,7 @@ enum advice_type {\n> \tADVICE_RESET_NO_REFRESH_WARNING,\n> \tADVICE_RESOLVE_CONFLICT,\n> \tADVICE_RM_HINTS,\n> +\tADVICE_SCISSORS,\n\nDitto.\n\n> \tADVICE_SEQUENCER_IN_USE,\n> \tADVICE_SET_UPSTREAM_FAILURE,\n> \tADVICE_SKIPPED_CHERRY_PICKS,\n> diff --git a/wt-status.c b/wt-status.c\n> index 459d399baa..19d4986351 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1104,7 +1104,8 @@ void wt_status_append_cut_line(struct strbuf *buf)\n> \tconst char *explanation = _(\"Do not modify or remove the line above.\\nEverything below it will be ignored.\");\n> \n> \tstrbuf_commented_addf(buf, comment_line_char, \"%s\", cut_line);\n> -\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n> +\tif (advice_enabled(ADVICE_SCISSORS))\n> +\t\tstrbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_char);\n\nI wonder if advise_if_enabled() might have a chance here. I'm just\nthinking out loud. \"if(advice_enabled(..\" is fine.\n\n> }\n> \n> void wt_status_add_cut_line(FILE *fp)\n> -- \n> 2.43.0\n> \n\nThanks.\n"},{"id":"493008","messageId":"xmqq7cgxdp0i.fsf@gitster.g","threadId":"61002","inReplyTo":"Zh7NZhB1n2o1k7jI@localhost","subject":"Re: [PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-16T20:35:41Z","receivedAt":"2024-04-16T20:35:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett writes:\n\n> On Sun, Feb 25, 2024 at 08:21:54PM -0800, Josh Triplett wrote:\n>> The scissors line before the diff in a verbose commit, or above all the\n>> comments when using --cleanup=scissors, has the following two lines of\n>> explanation after it:\n>> \n>> Do not modify or remove the line above.\n>> Everything below it will be ignored.\n>> \n>> This is useful advice for new users, but potentially redundant for\n>> experienced users, who might instead appreciate seeing two more lines of\n>> information in their editor.\n>> \n>> Add advice.scissors to suppress that explanation.\n>\n> Following up on this patch. Happy to rework if needed.\n\nI am not personally interested in the feature myself, and I doubt it\nwould help the end-user experience very much. You'd need to find\nsomebody else to cheer for the topic ;-)\n\nThanks.\n\n"},{"id":"493009","messageId":"046c01da903e$d6d97ac0$848c7040$@nexbridge.com","threadId":"61002","inReplyTo":"xmqq7cgxdp0i.fsf@gitster.g","subject":"RE: [PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-04-16T20:44:06Z","receivedAt":"2024-04-16T20:44:26Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Tuesday, April 16, 2024 4:36 PM, Junio C Hamano wrote:\n>Josh Triplett writes:\n>\n>> On Sun, Feb 25, 2024 at 08:21:54PM -0800, Josh Triplett wrote:\n>>> The scissors line before the diff in a verbose commit, or above all\n>>> the comments when using --cleanup=scissors, has the following two\n>>> lines of explanation after it:\n>>>\n>>> Do not modify or remove the line above.\n>>> Everything below it will be ignored.\n>>>\n>>> This is useful advice for new users, but potentially redundant for\n>>> experienced users, who might instead appreciate seeing two more lines\n>>> of information in their editor.\n>>>\n>>> Add advice.scissors to suppress that explanation.\n>>\n>> Following up on this patch. Happy to rework if needed.\n>\n>I am not personally interested in the feature myself, and I doubt it would\nhelp the\n>end-user experience very much. You'd need to find somebody else to cheer\nfor the\n>topic ;-)\n\nI am having a bit of trouble understanding the use-case for this. Is it\nlimited to linkgit? Under what circumstances would I need to use such\ncapabilities?\nThanks,\nRandall\n\n"},{"id":"493010","messageId":"xmqqv84hc53e.fsf@gitster.g","threadId":"61002","inReplyTo":"046c01da903e$d6d97ac0$848c7040$@nexbridge.com","subject":"Re: [PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-16T22:31:17Z","receivedAt":"2024-04-16T22:31:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":" writes:\n\n>>>> This is useful advice for new users, but potentially redundant for\n>>>> experienced users, who might instead appreciate seeing two more lines\n>>>> of information in their editor.\n>>>>\n>>>> Add advice.scissors to suppress that explanation.\n>>>\n>>> Following up on this patch. Happy to rework if needed.\n>>\n>>I am not personally interested in the feature myself, and I doubt it would\n> help the\n>>end-user experience very much. You'd need to find somebody else to cheer\n> for the\n>>topic ;-)\n>\n> I am having a bit of trouble understanding the use-case for\n> this. Is it limited to linkgit? Under what circumstances would I\n> need to use such capabilities?\n\nWhen you run \"git commit\" from the command line without specifying\nany message, you'd get an editor spawned for you with something like\nthis in the file (\"--- >8 ---\" and \"--- 8< ---\" are for illustration\npurposes in this message).\n\n --- >8 ---\n\n # Please enter the commit message for your changes. Lines starting\n # with '#' will be ignored, and an empty message aborts the commit.\n #\n # On branch next\n # Untracked files:\n #\t+runme.sh\n #\tP\n #\n --- 8< ---\n\nBut when you run \"git commit --cleanup=scissors\" (or use the\nequivalent configuration variables), you'd get this instead.\n\n --- >8 ---\n\n # ------------------------ >8 ------------------------\n # Do not modify or remove the line above.\n # Everything below it will be ignored.\n #\n # On branch next\n # Untracked files:\n #\t+runme.sh\n #\tP\n #\n --- 8< ---\n\nThe new advice configuration is to suppress the two lines from that\nmessage template.\n\nI agree that a bit more background information should be given in\nthe proposed log message to help readers. It should mention when\nthis new setting is relevant (e.g., when the \"--cleanup=scissors\"\noption is in effect), at least.\n\nI suspect that the same configuration variable wants to also control\nthe \"helpful\" comment in the \"normal\" case, in which case the name\nof the configuration variable would need to be rethought.\n\n\n\n\n"},{"id":"493013","messageId":"047301da9050$bae58b80$30b0a280$@nexbridge.com","threadId":"61002","inReplyTo":"xmqqv84hc53e.fsf@gitster.g","subject":"RE: [PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2024-04-16T22:52:11Z","receivedAt":"2024-04-16T22:52:20Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Tuesday, April 16, 2024 6:31 PM, Junio C Hamano wrote:\n> writes:\n>\n>>>>> This is useful advice for new users, but potentially redundant for\n>>>>> experienced users, who might instead appreciate seeing two more\n>>>>> lines of information in their editor.\n>>>>>\n>>>>> Add advice.scissors to suppress that explanation.\n>>>>\n>>>> Following up on this patch. Happy to rework if needed.\n>>>\n>>>I am not personally interested in the feature myself, and I doubt it\n>>>would\n>> help the\n>>>end-user experience very much. You'd need to find somebody else to\n>>>cheer\n>> for the\n>>>topic ;-)\n>>\n>> I am having a bit of trouble understanding the use-case for this. Is\n>> it limited to linkgit? Under what circumstances would I need to use\n>> such capabilities?\n>\n>When you run \"git commit\" from the command line without specifying any\n>message, you'd get an editor spawned for you with something like this in\nthe file (\"--\n>- >8 ---\" and \"--- 8< ---\" are for illustration purposes in this message).\n>\n> --- >8 ---\n>\n> # Please enter the commit message for your changes. Lines starting\n> # with '#' will be ignored, and an empty message aborts the commit.\n> #\n> # On branch next\n> # Untracked files:\n> #\t+runme.sh\n> #\tP\n> #\n> --- 8< ---\n>\n>But when you run \"git commit --cleanup=scissors\" (or use the equivalent\n>configuration variables), you'd get this instead.\n>\n> --- >8 ---\n>\n> # ------------------------ >8 ------------------------\n> # Do not modify or remove the line above.\n> # Everything below it will be ignored.\n> #\n> # On branch next\n> # Untracked files:\n> #\t+runme.sh\n> #\tP\n> #\n> --- 8< ---\n>\n>The new advice configuration is to suppress the two lines from that message\n>template.\n>\n>I agree that a bit more background information should be given in the\nproposed log\n>message to help readers. It should mention when this new setting is\nrelevant (e.g.,\n>when the \"--cleanup=scissors\"\n>option is in effect), at least.\n>\n>I suspect that the same configuration variable wants to also control the\n\"helpful\"\n>comment in the \"normal\" case, in which case the name of the configuration\nvariable\n>would need to be rethought.\n\nThanks. I hope that the default \"helpful\" comment in the \"normal\" case would\nnot change anything. I personally have course material that references\nexamples with the current commit message structure that would be impacted -\nNo problem changing it, but I am not sure what the new result would be.\n\nAs a comment on this, the scissors option might need some NLS consideration\nin organizations where there are more than one languages/encodings in use. I\ncould see some global settings conflicting with local, but that would need\ndocumentation.\n\n"},{"id":"493050","messageId":"9eede976d6be7a93199f9d53e916431a@manjaro.org","threadId":"61002","inReplyTo":"xmqqv84hc53e.fsf@gitster.g","subject":"Re: [PATCH] advice: Add advice.scissors to suppress \"do not modify or remove this line\"","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-17T04:00:12Z","receivedAt":"2024-04-17T04:00:14Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello all,\n\nPlease see my comments below.\n\nOn 2024-04-17 00:31, Junio C Hamano wrote:\n> writes:\n>> I am having a bit of trouble understanding the use-case for\n>> this. Is it limited to linkgit? Under what circumstances would I\n>> need to use such capabilities?\n> \n> When you run \"git commit\" from the command line without specifying\n> any message, you'd get an editor spawned for you with something like\n> this in the file (\"--- >8 ---\" and \"--- 8< ---\" are for illustration\n> purposes in this message).\n> \n> --- >8 ---\n> \n> # Please enter the commit message for your changes. Lines starting\n> # with '#' will be ignored, and an empty message aborts the commit.\n> #\n> # On branch next\n> # Untracked files:\n> #\t+runme.sh\n> #\tP\n> #\n> --- 8< ---\n\nFrankly, I'd much rather see a new configuration option that would\nsuppress the two leading lines in the example above. Of course, they'd\nstill be displayed by default.\n\n> But when you run \"git commit --cleanup=scissors\" (or use the\n> equivalent configuration variables), you'd get this instead.\n> \n> --- >8 ---\n> \n> # ------------------------ >8 ------------------------\n> # Do not modify or remove the line above.\n> # Everything below it will be ignored.\n> #\n> # On branch next\n> # Untracked files:\n> #\t+runme.sh\n> #\tP\n> #\n> --- 8< ---\n> \n> The new advice configuration is to suppress the two lines from that\n> message template.\n\nI think this new configuration option might be useful to some users,\nbut I'd suggest that it gets extended to also suppress the two opening\nlines mentioned in my comment on the first example above.\n\n> I agree that a bit more background information should be given in\n> the proposed log message to help readers. It should mention when\n> this new setting is relevant (e.g., when the \"--cleanup=scissors\"\n> option is in effect), at least.\n> \n> I suspect that the same configuration variable wants to also control\n> the \"helpful\" comment in the \"normal\" case, in which case the name\n> of the configuration variable would need to be rethought.\n\nAgreed, a better name for the new option would be helpful.\n"}]}