{"thread":{"id":"61003","subject":"[PATCH] commit: Avoid redundant scissor line with --cleanup=scissors -v","startedAt":"2024-02-26T04:23:18Z","lastAt":"2024-02-29T16:39:06Z","messageCount":11,"participants":["Josh Triplett","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"489346","messageId":"9c09cea2679e14258720ee63e932e3b9459dbd8c.1708921369.git.josh@joshtriplett.org","threadId":"61003","inReplyTo":null,"subject":"[PATCH] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-02-26T04:23:16Z","receivedAt":"2024-02-26T04:23:18Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"`git commit --cleanup=scissors -v` currently prints two scissors lines:\none at the start of the comment lines, and the other right before the\ndiff. This is redundant, and pushes the diff further down in the user's\neditor than it needs to be.\n\nPass the cleanup mode into wt_status, so that wt_status_print can avoid\nprinting the extra scissors if already printed.\n\nThis moves the enum commit_msg_cleanup_mode from sequencer.h to\nwt-status.h to allow wt_status to use the type. sequencer.h already\nincludes wt-status.h, so this doesn't affect anything else.\n\nSigned-off-by: Josh Triplett <josh@joshtriplett.org>\n---\n builtin/commit.c | 2 ++\n sequencer.h      | 7 -------\n wt-status.c      | 6 ++++--\n wt-status.h      | 8 ++++++++\n 4 files changed, 14 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6d1fa71676..6b2b412932 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -888,6 +888,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t */\n \ts->hints = 0;\n \n+\ts->cleanup_mode = cleanup_mode;\n+\n \tif (clean_message_contents)\n \t\tstrbuf_stripspace(&sb, '\\0');\n \ndiff --git a/sequencer.h b/sequencer.h\nindex dcef7bb99c..9f818e96f0 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -22,13 +22,6 @@ enum replay_action {\n \tREPLAY_INTERACTIVE_REBASE\n };\n \n-enum commit_msg_cleanup_mode {\n-\tCOMMIT_MSG_CLEANUP_SPACE,\n-\tCOMMIT_MSG_CLEANUP_NONE,\n-\tCOMMIT_MSG_CLEANUP_SCISSORS,\n-\tCOMMIT_MSG_CLEANUP_ALL\n-};\n-\n struct replay_opts {\n \tenum replay_action action;\n \ndiff --git a/wt-status.c b/wt-status.c\nindex b5a29083df..459d399baa 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1143,11 +1143,13 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \t * file (and even the \"auto\" setting won't work, since it\n \t * will have checked isatty on stdout). But we then do want\n \t * to insert the scissor line here to reliably remove the\n-\t * diff before committing.\n+\t * diff before committing, if we didn't already include one\n+\t * before.\n \t */\n \tif (s->fp != stdout) {\n \t\trev.diffopt.use_color = 0;\n-\t\twt_status_add_cut_line(s->fp);\n+\t\tif (s->cleanup_mode != COMMIT_MSG_CLEANUP_SCISSORS)\n+\t\t\twt_status_add_cut_line(s->fp);\n \t}\n \tif (s->verbose > 1 && s->committable) {\n \t\t/* print_updated() printed a header, so do we */\ndiff --git a/wt-status.h b/wt-status.h\nindex 819dcad723..5ede705e93 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -22,6 +22,13 @@ enum color_wt_status {\n \tWT_STATUS_MAXSLOT\n };\n \n+enum commit_msg_cleanup_mode {\n+\tCOMMIT_MSG_CLEANUP_SPACE,\n+\tCOMMIT_MSG_CLEANUP_NONE,\n+\tCOMMIT_MSG_CLEANUP_SCISSORS,\n+\tCOMMIT_MSG_CLEANUP_ALL\n+};\n+\n enum untracked_status_type {\n \tSHOW_NO_UNTRACKED_FILES,\n \tSHOW_NORMAL_UNTRACKED_FILES,\n@@ -130,6 +137,7 @@ struct wt_status {\n \tint rename_score;\n \tint rename_limit;\n \tenum wt_status_format status_format;\n+\tenum commit_msg_cleanup_mode cleanup_mode;\n \tstruct wt_status_state state;\n \tstruct object_id oid_commit; /* when not Initial */\n \n-- \n2.43.0\n\n"},{"id":"489399","messageId":"xmqqbk83nlw5.fsf@gitster.g","threadId":"61003","inReplyTo":"9c09cea2679e14258720ee63e932e3b9459dbd8c.1708921369.git.josh@joshtriplett.org","subject":"Re: [PATCH] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-26T18:03:22Z","receivedAt":"2024-02-26T18:03:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett <josh@joshtriplett.org> writes:\n\n> `git commit --cleanup=scissors -v` currently prints two scissors lines:\n> one at the start of the comment lines, and the other right before the\n> diff. This is redundant, and pushes the diff further down in the user's\n> editor than it needs to be.\n\nInteresting discovery.\n\n> diff --git a/wt-status.c b/wt-status.c\n> index b5a29083df..459d399baa 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1143,11 +1143,13 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n>  \t * file (and even the \"auto\" setting won't work, since it\n>  \t * will have checked isatty on stdout). But we then do want\n>  \t * to insert the scissor line here to reliably remove the\n> -\t * diff before committing.\n> +\t * diff before committing, if we didn't already include one\n> +\t * before.\n>  \t */\n>  \tif (s->fp != stdout) {\n>  \t\trev.diffopt.use_color = 0;\n> -\t\twt_status_add_cut_line(s->fp);\n> +\t\tif (s->cleanup_mode != COMMIT_MSG_CLEANUP_SCISSORS)\n> +\t\t\twt_status_add_cut_line(s->fp);\n>  \t}\n\nThe machinery to populate the log message buffer should ideally be\ntaught to remember if it already has added a scissors-line and to\nrefrain from adding redundant ones.  That way, we do not have to\nrely on the order of places that make wt_status_add_cut_line() calls\nor what condition they use to decide to make these calls.\n\nThis hunk for example knows not just this one produces cut-line\nafter the other one potentially added one, but also the logic used\nby the other one to decide to add one, which is even worse.  I find\nthe solution presented here a bit unsatisfactory, for this reason,\nbut for now it may be OK, as we probably are not adding any more\nplaces and conditions to emit a scissors line.\n\n>  builtin/commit.c | 2 ++\n>  sequencer.h      | 7 -------\n>  wt-status.c      | 6 ++++--\n>  wt-status.h      | 8 ++++++++\n>  4 files changed, 14 insertions(+), 9 deletions(-)\n\nIf this change did not break any existing tests that checked the\ncombination of options and output when they are used together, it\nmeans we have a gap in the test coverage.  We needs a test or two\nto protect this fix from future breakages.\n\nThanks.\n"},{"id":"489473","messageId":"Zd2eLxPelxvP8FDk@localhost","threadId":"61003","inReplyTo":"xmqqbk83nlw5.fsf@gitster.g","subject":"Re: [PATCH] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-02-27T08:32:47Z","receivedAt":"2024-02-27T08:32:50Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Mon, Feb 26, 2024 at 10:03:22AM -0800, Junio C Hamano wrote:\n> Josh Triplett <josh@joshtriplett.org> writes:\n> \n> > `git commit --cleanup=scissors -v` currently prints two scissors lines:\n> > one at the start of the comment lines, and the other right before the\n> > diff. This is redundant, and pushes the diff further down in the user's\n> > editor than it needs to be.\n> \n> Interesting discovery.\n> \n> > diff --git a/wt-status.c b/wt-status.c\n> > index b5a29083df..459d399baa 100644\n> > --- a/wt-status.c\n> > +++ b/wt-status.c\n> > @@ -1143,11 +1143,13 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n> >  \t * file (and even the \"auto\" setting won't work, since it\n> >  \t * will have checked isatty on stdout). But we then do want\n> >  \t * to insert the scissor line here to reliably remove the\n> > -\t * diff before committing.\n> > +\t * diff before committing, if we didn't already include one\n> > +\t * before.\n> >  \t */\n> >  \tif (s->fp != stdout) {\n> >  \t\trev.diffopt.use_color = 0;\n> > -\t\twt_status_add_cut_line(s->fp);\n> > +\t\tif (s->cleanup_mode != COMMIT_MSG_CLEANUP_SCISSORS)\n> > +\t\t\twt_status_add_cut_line(s->fp);\n> >  \t}\n> \n> The machinery to populate the log message buffer should ideally be\n> taught to remember if it already has added a scissors-line and to\n> refrain from adding redundant ones.  That way, we do not have to\n> rely on the order of places that make wt_status_add_cut_line() calls\n> or what condition they use to decide to make these calls.\n> \n> This hunk for example knows not just this one produces cut-line\n> after the other one potentially added one, but also the logic used\n> by the other one to decide to add one, which is even worse.  I find\n> the solution presented here a bit unsatisfactory, for this reason,\n> but for now it may be OK, as we probably are not adding any more\n> places and conditions to emit a scissors line.\n\nI could add statefulness to wt_status_add_cut_line instead, on the\nassumption that it's the only thing that should be adding a cut line,\nand having it not add the line if previously added. For instance, it\ncould accept a pointer to the full wt_status rather than just the fp,\nand keep a boolean state there.\n\n> >  builtin/commit.c | 2 ++\n> >  sequencer.h      | 7 -------\n> >  wt-status.c      | 6 ++++--\n> >  wt-status.h      | 8 ++++++++\n> >  4 files changed, 14 insertions(+), 9 deletions(-)\n> \n> If this change did not break any existing tests that checked the\n> combination of options and output when they are used together, it\n> means we have a gap in the test coverage.  We needs a test or two\n> to protect this fix from future breakages.\n\nI did run the testsuite, and it passed. I can add a simple test easily\nenough.\n"},{"id":"489477","messageId":"4f97933f173220544a5be2bf05c2bee2b044d2b1.1709024540.git.josh@joshtriplett.org","threadId":"61003","inReplyTo":"Zd2eLxPelxvP8FDk@localhost","subject":"[PATCH v2 1/2] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-02-27T09:16:09Z","receivedAt":"2024-02-27T09:16:12Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"`git commit --cleanup=scissors -v` currently prints two scissors lines:\none at the start of the comment lines, and the other right before the\ndiff. This is redundant, and pushes the diff further down in the user's\neditor than it needs to be.\n\nMake wt_status_add_cut_line remember if it has added a cut line before,\nand avoid adding a redundant one.\n\nAdd a test for this.\n\nSigned-off-by: Josh Triplett <josh@joshtriplett.org>\n---\nv2: Make wt_status_add_cut_line remember if it has added a cut line,\nrather than making later callers try to figure out if it has been called\nbefore. Add a test.\n\nNote that other parts of the code do already try to figure out if the\n*merge* logic has added scissors already, which is where\nmerge_contains_scissors comes from. Patch 2/2 unifies that machinery.\n\n builtin/commit.c            |  4 ++--\n t/t7502-commit-porcelain.sh |  5 +++++\n wt-status.c                 | 12 ++++++++----\n wt-status.h                 |  3 ++-\n 4 files changed, 17 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6d1fa71676..e0a6d43179 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -926,7 +926,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tif (whence != FROM_COMMIT) {\n \t\t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&\n \t\t\t\t!merge_contains_scissors)\n-\t\t\t\twt_status_add_cut_line(s->fp);\n+\t\t\t\twt_status_add_cut_line(s);\n \t\t\tstatus_printf_ln(\n \t\t\t\ts, GIT_COLOR_NORMAL,\n \t\t\t\twhence == FROM_MERGE ?\n@@ -947,7 +947,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tstatus_printf(s, GIT_COLOR_NORMAL, hint_cleanup_all, comment_line_char);\n \t\telse if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n \t\t\tif (whence == FROM_COMMIT && !merge_contains_scissors)\n-\t\t\t\twt_status_add_cut_line(s->fp);\n+\t\t\t\twt_status_add_cut_line(s);\n \t\t} else /* COMMIT_MSG_CLEANUP_SPACE, that is. */\n \t\t\tstatus_printf(s, GIT_COLOR_NORMAL, hint_cleanup_space, comment_line_char);\n \ndiff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\nindex a87c211d0b..b37e2018a7 100755\n--- a/t/t7502-commit-porcelain.sh\n+++ b/t/t7502-commit-porcelain.sh\n@@ -736,6 +736,11 @@ test_expect_success 'message shows date when it is explicitly set' '\n \t  .git/COMMIT_EDITMSG\n '\n \n+test_expect_success 'message does not have multiple scissors lines' '\n+\tgit commit --cleanup=scissors -v --allow-empty -e -m foo &&\n+\ttest $(grep -c -e \"--- >8 ---\" .git/COMMIT_EDITMSG) -eq 1\n+'\n+\n test_expect_success AUTOIDENT 'message shows committer when it is automatic' '\n \n \techo >>negative &&\ndiff --git a/wt-status.c b/wt-status.c\nindex ea13f5d8db..2d576f7a44 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1108,12 +1108,15 @@ void wt_status_append_cut_line(struct strbuf *buf)\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+void wt_status_add_cut_line(struct wt_status *s)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \n+\tif (s->added_cut_line)\n+\t\treturn;\n+\ts->added_cut_line = 1;\n \twt_status_append_cut_line(&buf);\n-\tfputs(buf.buf, fp);\n+\tfputs(buf.buf, s->fp);\n \tstrbuf_release(&buf);\n }\n \n@@ -1144,11 +1147,12 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \t * file (and even the \"auto\" setting won't work, since it\n \t * will have checked isatty on stdout). But we then do want\n \t * to insert the scissor line here to reliably remove the\n-\t * diff before committing.\n+\t * diff before committing, if we didn't already include one\n+\t * before.\n \t */\n \tif (s->fp != stdout) {\n \t\trev.diffopt.use_color = 0;\n-\t\twt_status_add_cut_line(s->fp);\n+\t\twt_status_add_cut_line(s);\n \t}\n \tif (s->verbose > 1 && s->committable) {\n \t\t/* print_updated() printed a header, so do we */\ndiff --git a/wt-status.h b/wt-status.h\nindex 819dcad723..5e99ba4707 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -130,6 +130,7 @@ struct wt_status {\n \tint rename_score;\n \tint rename_limit;\n \tenum wt_status_format status_format;\n+\tunsigned char added_cut_line; /* boolean */\n \tstruct wt_status_state state;\n \tstruct object_id oid_commit; /* when not Initial */\n \n@@ -147,7 +148,7 @@ struct wt_status {\n \n size_t wt_status_locate_end(const char *s, size_t len);\n void wt_status_append_cut_line(struct strbuf *buf);\n-void wt_status_add_cut_line(FILE *fp);\n+void wt_status_add_cut_line(struct wt_status *s);\n void wt_status_prepare(struct repository *r, struct wt_status *s);\n void wt_status_print(struct wt_status *s);\n void wt_status_collect(struct wt_status *s);\n-- \n2.43.0\n"},{"id":"489478","messageId":"553c8692e9f0f0159bfba5b15e0abb3190a6f5f3.1709025354.git.josh@joshtriplett.org","threadId":"61003","inReplyTo":"4f97933f173220544a5be2bf05c2bee2b044d2b1.1709024540.git.josh@joshtriplett.org","subject":"[PATCH v2 2/2] commit: Unify logic to avoid multiple scissors lines when merging","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-02-27T09:17:36Z","receivedAt":"2024-02-27T09:17:38Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"prepare_to_commit has some logic to figure out whether merge already\nadded a scissors line, and therefore it shouldn't add another. Now that\nwt_status_add_cut_line has built-in state for whether it has\nalready added a previous line, just set that state instead, and then\nremove that condition from subsequent calls to wt_status_add_cut_line.\n\nSigned-off-by: Josh Triplett <josh@joshtriplett.org>\n---\nv2: New patch.\n\n builtin/commit.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e0a6d43179..142f54ea7c 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -737,7 +737,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tconst char *hook_arg2 = NULL;\n \tint clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);\n \tint old_display_comment_prefix;\n-\tint merge_contains_scissors = 0;\n \tint invoked_hook;\n \n \t/* This checks and barfs if author is badly specified */\n@@ -841,7 +840,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t    wt_status_locate_end(sb.buf + merge_msg_start,\n \t\t\t\t\t sb.len - merge_msg_start) <\n \t\t\t\tsb.len - merge_msg_start)\n-\t\t\tmerge_contains_scissors = 1;\n+\t\t\ts->added_cut_line = 1;\n \t} else if (!stat(git_path_squash_msg(the_repository), &statbuf)) {\n \t\tif (strbuf_read_file(&sb, git_path_squash_msg(the_repository), 0) < 0)\n \t\t\tdie_errno(_(\"could not read SQUASH_MSG\"));\n@@ -924,8 +923,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t  \" yourself if you want to.\\n\"\n \t\t\t  \"An empty message aborts the commit.\\n\");\n \t\tif (whence != FROM_COMMIT) {\n-\t\t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&\n-\t\t\t\t!merge_contains_scissors)\n+\t\t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)\n \t\t\t\twt_status_add_cut_line(s);\n \t\t\tstatus_printf_ln(\n \t\t\t\ts, GIT_COLOR_NORMAL,\n@@ -946,7 +944,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_ALL)\n \t\t\tstatus_printf(s, GIT_COLOR_NORMAL, hint_cleanup_all, comment_line_char);\n \t\telse if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {\n-\t\t\tif (whence == FROM_COMMIT && !merge_contains_scissors)\n+\t\t\tif (whence == FROM_COMMIT)\n \t\t\t\twt_status_add_cut_line(s);\n \t\t} else /* COMMIT_MSG_CLEANUP_SPACE, that is. */\n \t\t\tstatus_printf(s, GIT_COLOR_NORMAL, hint_cleanup_space, comment_line_char);\n-- \n2.43.0\n"},{"id":"489550","messageId":"xmqqedcxvnn8.fsf@gitster.g","threadId":"61003","inReplyTo":"Zd2eLxPelxvP8FDk@localhost","subject":"Re: [PATCH] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-27T17:10:35Z","receivedAt":"2024-02-27T17:10:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett <josh@joshtriplett.org> writes:\n\n> I could add statefulness to wt_status_add_cut_line instead, on the\n> assumption that it's the only thing that should be adding a cut line,\n> and having it not add the line if previously added. For instance, it\n> could accept a pointer to the full wt_status rather than just the fp,\n> and keep a boolean state there.\n\nYeah, that approach also has to assume that wt_status structure is\nused only once to create a single message buffer without being\nreused, but I think that is a safe assumption, too.  The function\nbeing the only thing that adds the scissors line should also be a\nsafe assumption in code hygiene standpoint---if somebody else tries\nto manually write such a line, we'll shoot such a patch down and\ntell them to call this function anyway ;-).\n\n> I did run the testsuite, and it passed. I can add a simple test easily\n> enough.\n\nIt would be prudent to do so.\n\nThanks.\n\n"},{"id":"489552","messageId":"xmqqjzmpu7k6.fsf@gitster.g","threadId":"61003","inReplyTo":"4f97933f173220544a5be2bf05c2bee2b044d2b1.1709024540.git.josh@joshtriplett.org","subject":"Re: [PATCH v2 1/2] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-27T17:43:21Z","receivedAt":"2024-02-27T17:43:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett <josh@joshtriplett.org> writes:\n\n> diff --git a/wt-status.c b/wt-status.c\n> index ea13f5d8db..2d576f7a44 100644\n\nI do not seem to have the preimage ea13f5d8db; as a bugfix patch, it\nwould be preferrable to make the patches not to depend on anything\nin flight.  If feasible, it may even be nicer to base them on one of\nthe maintenance tracks.\n\nI managed to wiggle the patch in (somehow a context line was\nmisindented), so there hopefully is no need to resend.\n\nThanks.\n"},{"id":"489647","messageId":"ZeAFutaddf4M2wjM@localhost","threadId":"61003","inReplyTo":"xmqqjzmpu7k6.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-02-29T04:19:06Z","receivedAt":"2024-02-29T04:19:09Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Tue, Feb 27, 2024 at 09:43:21AM -0800, Junio C Hamano wrote:\n> Josh Triplett <josh@joshtriplett.org> writes:\n> \n> > diff --git a/wt-status.c b/wt-status.c\n> > index ea13f5d8db..2d576f7a44 100644\n> \n> I do not seem to have the preimage ea13f5d8db; as a bugfix patch, it\n> would be preferrable to make the patches not to depend on anything\n> in flight.  If feasible, it may even be nicer to base them on one of\n> the maintenance tracks.\n> \n> I managed to wiggle the patch in (somehow a context line was\n> misindented), so there hopefully is no need to resend.\n\nSorry about that. I had these two patches in the same branch as my other recent patch\n`advice: Add advice.scissors to suppress \"do not modify or remove this line\"`\nbut the two ended up independent so I didn't send them as a series. That\npatch and this two-patch series should be applicable independently.\n\nIf you do end up needing a resend of any of them, I'm happy to do so.\n"},{"id":"489650","messageId":"xmqqttlrj08t.fsf@gitster.g","threadId":"61003","inReplyTo":"ZeAFutaddf4M2wjM@localhost","subject":"Re: [PATCH v2 1/2] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-29T05:41:22Z","receivedAt":"2024-02-29T05:41:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett <josh@joshtriplett.org> writes:\n\n> If you do end up needing a resend of any of them, I'm happy to do so.\n\nI do not think there is need for resending, but I think you promised\nto add some tests earlier, so an updated patch may be in order ;-)\n\nThanks.\n"},{"id":"489652","messageId":"ZeAftoPPRsltswbS@localhost","threadId":"61003","inReplyTo":"xmqqttlrj08t.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2024-02-29T06:09:58Z","receivedAt":"2024-02-29T06:10:01Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Wed, Feb 28, 2024 at 09:41:22PM -0800, Junio C Hamano wrote:\n> Josh Triplett <josh@joshtriplett.org> writes:\n> > If you do end up needing a resend of any of them, I'm happy to do so.\n>\n> I do not think there is need for resending, but I think you promised\n> to add some tests earlier, so an updated patch may be in order ;-)\n\nI did add a test; v2 that you replied to has this:\n> Add a test for this.\n[...]\n>  t/t7502-commit-porcelain.sh |  5 +++++\n[...]\n> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\n> index a87c211d0b..b37e2018a7 100755\n> --- a/t/t7502-commit-porcelain.sh\n> +++ b/t/t7502-commit-porcelain.sh\n> @@ -736,6 +736,11 @@ test_expect_success 'message shows date when it is explicitly set' '\n>         .git/COMMIT_EDITMSG\n>  '\n>  \n> +test_expect_success 'message does not have multiple scissors lines' '\n> +     git commit --cleanup=scissors -v --allow-empty -e -m foo &&\n> +     test $(grep -c -e \"--- >8 ---\" .git/COMMIT_EDITMSG) -eq 1\n> +'\n> +\n>  test_expect_success AUTOIDENT 'message shows committer when it is automatic' '\n>  \n>       echo >>negative &&\n\nhttps://lore.kernel.org/git/xmqqedcxvnn8.fsf@gitster.g/T/#Z2e.:..:4f97933f173220544a5be2bf05c2bee2b044d2b1.1709024540.git.josh::40joshtriplett.org:1t:t7502-commit-porcelain.sh\n"},{"id":"489668","messageId":"xmqqil27i5ss.fsf@gitster.g","threadId":"61003","inReplyTo":"ZeAftoPPRsltswbS@localhost","subject":"Re: [PATCH v2 1/2] commit: Avoid redundant scissor line with --cleanup=scissors -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-29T16:38:59Z","receivedAt":"2024-02-29T16:39:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett <josh@joshtriplett.org> writes:\n\n> I did add a test; v2 that you replied to has this:\n>> Add a test for this.\n\nThanks.  Let's mark it for 'next' then.\n"}]}