threads / patch / 35887

patch, 3 partscommit --cleanup=<scissors marker>

Subject: [PATCH 0/3] commit --cleanup=<scissors marker>

## tl;dr

14 messages between Feb 16, 2014 and Feb 25, 2014. Diffs are folded; open one to read it.

replies: 13people: 4as markdown or json

Nguyễn Thái Ngọc Duy· Feb 16, 2014, 03:37 UTC · lore

I've been bitten by --cleanup=default stripping my "#" lines a few times. Cutting "git status" with a long scissors marker line reduces chances of that happening. Most of the code is already there in 1a72cfd. This is just to add the UI.

Nguyễn Thái Ngọc Duy (3):
  wt-status.c: make cut_lines[] const to shrink .data section a bit
  wt-status.c: move cut-line print code out to wt_status_add_cut_line
  commit: add --cleanup=scissors
 Documentation/git-commit.txt |  7 ++++++-
 builtin/commit.c             |  9 +++++++--
 t/t7502-commit.sh            | 16 ++++++++++++++++
 wt-status.c                  | 21 +++++++++++++--------
 wt-status.h                  |  1 +
 5 files changed, 43 insertions(+), 11 deletions(-)
-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 16, 2014, 03:37 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 1/3] wt-status.c: make cut_lines[] const to shrink .data section a bit

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to wt-status.c +1 −1
diff --git a/wt-status.c b/wt-status.c
index 4e55810..65e35c3 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -17,7 +17,7 @@
 #include "strbuf.h"
 #include "utf8.h"
 
-static char cut_line[] =
+static const char cut_line[] =
 "------------------------ >8 ------------------------\n";
 
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
-- 
1.8.5.2.240.g8478abd
brian m. carlson· Feb 16, 2014, 16:21 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 1/3] wt-status.c: make cut_lines[] const to shrink .data section a bit

On Sun, Feb 16, 2014 at 10:37:18AM +0700, Nguyễn Thái Ngọc Duy wrote:
> -static char cut_line[] =
> +static const char cut_line[] =
Your subject says cut_lines[], but the variable is cut_line[] (no "s").
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Nguyễn Thái Ngọc Duy· Feb 16, 2014, 03:37 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 2/3] wt-status.c: move cut-line print code out to wt_status_add_cut_line

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 19 ++++++++++++-------
 wt-status.h |  1 +
 2 files changed, 13 insertions(+), 7 deletions(-)
Show changes to 2 files +13 −7

wt-status.c, wt-status.h

diff --git a/wt-status.c b/wt-status.c
index 65e35c3..ed31b6a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -808,6 +808,17 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
 	strbuf_release(&pattern);
 }
 
+void wt_status_add_cut_line(FILE *fp)
+{
+	const char *explanation = _("Do not touch the line above.\nEverything below will be removed.");
+	struct strbuf buf = STRBUF_INIT;
+
+	fprintf(fp, "%c %s", comment_line_char, cut_line);
+	strbuf_add_commented_lines(&buf, explanation, strlen(explanation));
+	fputs(buf.buf, fp);
+	strbuf_release(&buf);
+}
+
 static void wt_status_print_verbose(struct wt_status *s)
 {
 	struct rev_info rev;
@@ -833,14 +844,8 @@ static void wt_status_print_verbose(struct wt_status *s)
 	 * diff before committing.
 	 */
 	if (s->fp != stdout) {
-		const char *explanation = _("Do not touch the line above.\nEverything below will be removed.");
-		struct strbuf buf = STRBUF_INIT;
-
 		rev.diffopt.use_color = 0;
-		fprintf(s->fp, "%c %s", comment_line_char, cut_line);
-		strbuf_add_commented_lines(&buf, explanation, strlen(explanation));
-		fputs(buf.buf, s->fp);
-		strbuf_release(&buf);
+		wt_status_add_cut_line(s->fp);
 	}
 	run_diff_index(&rev, 1);
 }
diff --git a/wt-status.h b/wt-status.h
index 30a4812..b56ce3f 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -92,6 +92,7 @@ struct wt_status_state {
 };
 
 void wt_status_truncate_message_at_cut_line(struct strbuf *);
+void wt_status_add_cut_line(FILE *fp);
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 16, 2014, 03:37 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 3/3] commit: add --cleanup=scissors

Since 1a72cfd (commit -v: strip diffs and submodule shortlogs from the commit message - 2013-12-05) we have a less fragile way to cut out "git status" at the end of a commit message but it's only enabled for stripping submodule shortlogs. Add new cleanup option that reuses the same mechanism for the entire "git status" without accidentally remove lines starting with '#'

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-commit.txt |  7 ++++++-
 builtin/commit.c             |  9 +++++++--
 t/t7502-commit.sh            | 16 ++++++++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)
Show changes to 3 files +29 −3

Documentation/git-commit.txt, builtin/commit.c, t/t7502-commit.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1a7616c..98f976a 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -176,7 +176,7 @@ OPTIONS
 --cleanup=<mode>::
 	This option determines how the supplied commit message should be
 	cleaned up before committing.  The '<mode>' can be `strip`,
-	`whitespace`, `verbatim`, or `default`.
+	`whitespace`, `verbatim`, `scissors` or `default`.
 +
 --
 strip::
@@ -186,6 +186,11 @@ whitespace::
 	Same as `strip` except #commentary is not removed.
 verbatim::
 	Do not change the message at all.
+scissors::
+	Same as `whitespace`, except that everything from the line
+	"`# ------------------------ >8 ------------------------`"
+	is truncated if the message is to be edited. "`#`" could be
+	customized with core.commentChar.
 default::
 	Same as `strip` if the message is to be edited.
 	Otherwise `whitespace`.
diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478..ea2912f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -113,6 +113,7 @@ static char *sign_commit;
 static enum {
 	CLEANUP_SPACE,
 	CLEANUP_NONE,
+	CLEANUP_SCISSORS,
 	CLEANUP_ALL
 } cleanup_mode;
 static const char *cleanup_arg;
@@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 				_("Please enter the commit message for your changes."
 				  " Lines starting\nwith '%c' will be ignored, and an empty"
 				  " message aborts the commit.\n"), comment_line_char);
+		else if (cleanup_mode == CLEANUP_SCISSORS)
+			wt_status_add_cut_line(s->fp);
 		else /* CLEANUP_SPACE, that is. */
 			status_printf(s, GIT_COLOR_NORMAL,
 				_("Please enter the commit message for your changes."
@@ -1132,6 +1135,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		cleanup_mode = CLEANUP_SPACE;
 	else if (!strcmp(cleanup_arg, "strip"))
 		cleanup_mode = CLEANUP_ALL;
+	else if (!strcmp(cleanup_arg, "scissors"))
+		cleanup_mode = use_editor ? CLEANUP_SCISSORS : CLEANUP_SPACE;
 	else
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
@@ -1600,8 +1605,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		die(_("could not read commit message: %s"), strerror(saved_errno));
 	}
 
-	/* Truncate the message just before the diff, if any. */
-	if (verbose)
+	if (verbose || /* Truncate the message just before the diff, if any. */
+	    cleanup_mode == CLEANUP_SCISSORS)
 		wt_status_truncate_message_at_cut_line(&sb);
 
 	if (cleanup_mode != CLEANUP_NONE)
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 6313da2..9a3f3a1 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -223,6 +223,22 @@ test_expect_success 'cleanup commit messages (whitespace option,-F)' '
 
 '
 
+test_expect_success 'cleanup commit messages (scissors option,-F,-e)' '
+
+	echo >>negative &&
+	cat >text <<EOF &&
+
+# to be kept
+# ------------------------ >8 ------------------------
+to be removed
+EOF
+	echo "# to be kept" >expect &&
+	git commit --cleanup=scissors -e -F text -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'cleanup commit messages (strip option,-F)' '
 
 	echo >>negative &&
-- 
1.8.5.2.240.g8478abd
Eric Sunshine· Feb 16, 2014, 03:44 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 3/3] commit: add --cleanup=scissors

On Sat, Feb 15, 2014 at 10:37 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

Show 5 quoted lines
> Since 1a72cfd (commit -v: strip diffs and submodule shortlogs from the
> commit message - 2013-12-05) we have a less fragile way to cut out
> "git status" at the end of a commit message but it's only enabled for
> stripping submodule shortlogs. Add new cleanup option that reuses the
> same mechanism for the entire "git status" without accidentally remove
s/remove/removing/
Show 24 quoted lines
> lines starting with '#'
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index 1a7616c..98f976a 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -176,7 +176,7 @@ OPTIONS
>  --cleanup=<mode>::
>         This option determines how the supplied commit message should be
>         cleaned up before committing.  The '<mode>' can be `strip`,
> -       `whitespace`, `verbatim`, or `default`.
> +       `whitespace`, `verbatim`, `scissors` or `default`.
>  +
>  --
>  strip::
> @@ -186,6 +186,11 @@ whitespace::
>         Same as `strip` except #commentary is not removed.
>  verbatim::
>         Do not change the message at all.
> +scissors::
> +       Same as `whitespace`, except that everything from the line
> +       "`# ------------------------ >8 ------------------------`"

Would it make sense to be more explicit and say that the 'cut' line is also removed?

    Same as `whitespace`, except that the line
    `# ------------------------ >8 ------------------------`
    and all lines following it are removed ...
> +       is truncated if the message is to be edited. "`#`" could be
s/could/can/
> +       customized with core.commentChar.
>  default::
>         Same as `strip` if the message is to be edited.
>         Otherwise `whitespace`.
Nguyễn Thái Ngọc Duy· Feb 17, 2014, 12:15 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 0/3] commit --cleanup=<scissors marker>

Slight changes in git-commit.txt and some commit messages after Brian and Eric's comments.

Nguyễn Thái Ngọc Duy (3):
  wt-status.c: make cut_line[] const to shrink .data section a bit
  wt-status.c: move cut-line print code out to wt_status_add_cut_line
  commit: add --cleanup=scissors
 Documentation/git-commit.txt |  8 +++++++-
 builtin/commit.c             |  9 +++++++--
 t/t7502-commit.sh            | 16 ++++++++++++++++
 wt-status.c                  | 21 +++++++++++++--------
 wt-status.h                  |  1 +
 5 files changed, 44 insertions(+), 11 deletions(-)
-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 17, 2014, 12:15 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 1/3] wt-status.c: make cut_line[] const to shrink .data section a bit

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to wt-status.c +1 −1
diff --git a/wt-status.c b/wt-status.c
index 4e55810..65e35c3 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -17,7 +17,7 @@
 #include "strbuf.h"
 #include "utf8.h"
 
-static char cut_line[] =
+static const char cut_line[] =
 "------------------------ >8 ------------------------\n";
 
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 17, 2014, 12:15 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 2/3] wt-status.c: move cut-line print code out to wt_status_add_cut_line

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 wt-status.c | 19 ++++++++++++-------
 wt-status.h |  1 +
 2 files changed, 13 insertions(+), 7 deletions(-)
Show changes to 2 files +13 −7

wt-status.c, wt-status.h

diff --git a/wt-status.c b/wt-status.c
index 65e35c3..ed31b6a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -808,6 +808,17 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
 	strbuf_release(&pattern);
 }
 
+void wt_status_add_cut_line(FILE *fp)
+{
+	const char *explanation = _("Do not touch the line above.\nEverything below will be removed.");
+	struct strbuf buf = STRBUF_INIT;
+
+	fprintf(fp, "%c %s", comment_line_char, cut_line);
+	strbuf_add_commented_lines(&buf, explanation, strlen(explanation));
+	fputs(buf.buf, fp);
+	strbuf_release(&buf);
+}
+
 static void wt_status_print_verbose(struct wt_status *s)
 {
 	struct rev_info rev;
@@ -833,14 +844,8 @@ static void wt_status_print_verbose(struct wt_status *s)
 	 * diff before committing.
 	 */
 	if (s->fp != stdout) {
-		const char *explanation = _("Do not touch the line above.\nEverything below will be removed.");
-		struct strbuf buf = STRBUF_INIT;
-
 		rev.diffopt.use_color = 0;
-		fprintf(s->fp, "%c %s", comment_line_char, cut_line);
-		strbuf_add_commented_lines(&buf, explanation, strlen(explanation));
-		fputs(buf.buf, s->fp);
-		strbuf_release(&buf);
+		wt_status_add_cut_line(s->fp);
 	}
 	run_diff_index(&rev, 1);
 }
diff --git a/wt-status.h b/wt-status.h
index 30a4812..b56ce3f 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -92,6 +92,7 @@ struct wt_status_state {
 };
 
 void wt_status_truncate_message_at_cut_line(struct strbuf *);
+void wt_status_add_cut_line(FILE *fp);
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 17, 2014, 12:15 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 3/3] commit: add --cleanup=scissors

Since 1a72cfd (commit -v: strip diffs and submodule shortlogs from the commit message - 2013-12-05) we have a less fragile way to cut out "git status" at the end of a commit message but it's only enabled for stripping submodule shortlogs. Add new cleanup option that reuses the same mechanism for the entire "git status" without accidentally removing lines starting with '#'

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-commit.txt |  8 +++++++-
 builtin/commit.c             |  9 +++++++--
 t/t7502-commit.sh            | 16 ++++++++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)
Show changes to 3 files +30 −3

Documentation/git-commit.txt, builtin/commit.c, t/t7502-commit.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1a7616c..7948233 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -176,7 +176,7 @@ OPTIONS
 --cleanup=<mode>::
 	This option determines how the supplied commit message should be
 	cleaned up before committing.  The '<mode>' can be `strip`,
-	`whitespace`, `verbatim`, or `default`.
+	`whitespace`, `verbatim`, `scissors` or `default`.
 +
 --
 strip::
@@ -186,6 +186,12 @@ whitespace::
 	Same as `strip` except #commentary is not removed.
 verbatim::
 	Do not change the message at all.
+scissors::
+	Same as `whitespace`, except that everything from (and
+	including) the line
+	"`# ------------------------ >8 ------------------------`"
+	is truncated if the message is to be edited. "`#`" can be
+	customized with core.commentChar.
 default::
 	Same as `strip` if the message is to be edited.
 	Otherwise `whitespace`.
diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478..ea2912f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -113,6 +113,7 @@ static char *sign_commit;
 static enum {
 	CLEANUP_SPACE,
 	CLEANUP_NONE,
+	CLEANUP_SCISSORS,
 	CLEANUP_ALL
 } cleanup_mode;
 static const char *cleanup_arg;
@@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 				_("Please enter the commit message for your changes."
 				  " Lines starting\nwith '%c' will be ignored, and an empty"
 				  " message aborts the commit.\n"), comment_line_char);
+		else if (cleanup_mode == CLEANUP_SCISSORS)
+			wt_status_add_cut_line(s->fp);
 		else /* CLEANUP_SPACE, that is. */
 			status_printf(s, GIT_COLOR_NORMAL,
 				_("Please enter the commit message for your changes."
@@ -1132,6 +1135,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		cleanup_mode = CLEANUP_SPACE;
 	else if (!strcmp(cleanup_arg, "strip"))
 		cleanup_mode = CLEANUP_ALL;
+	else if (!strcmp(cleanup_arg, "scissors"))
+		cleanup_mode = use_editor ? CLEANUP_SCISSORS : CLEANUP_SPACE;
 	else
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
@@ -1600,8 +1605,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		die(_("could not read commit message: %s"), strerror(saved_errno));
 	}
 
-	/* Truncate the message just before the diff, if any. */
-	if (verbose)
+	if (verbose || /* Truncate the message just before the diff, if any. */
+	    cleanup_mode == CLEANUP_SCISSORS)
 		wt_status_truncate_message_at_cut_line(&sb);
 
 	if (cleanup_mode != CLEANUP_NONE)
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 6313da2..9a3f3a1 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -223,6 +223,22 @@ test_expect_success 'cleanup commit messages (whitespace option,-F)' '
 
 '
 
+test_expect_success 'cleanup commit messages (scissors option,-F,-e)' '
+
+	echo >>negative &&
+	cat >text <<EOF &&
+
+# to be kept
+# ------------------------ >8 ------------------------
+to be removed
+EOF
+	echo "# to be kept" >expect &&
+	git commit --cleanup=scissors -e -F text -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
+	test_cmp expect actual
+
+'
+
 test_expect_success 'cleanup commit messages (strip option,-F)' '
 
 	echo >>negative &&
-- 
1.8.5.2.240.g8478abd
Duy Nguyen· Feb 22, 2014, 00:58 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v2 3/3] commit: add --cleanup=scissors

On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote:
Show 9 quoted lines
> @@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  				_("Please enter the commit message for your changes."
>  				  " Lines starting\nwith '%c' will be ignored, and an empty"
>  				  " message aborts the commit.\n"), comment_line_char);
> +		else if (cleanup_mode == CLEANUP_SCISSORS)
> +			wt_status_add_cut_line(s->fp);
>  		else /* CLEANUP_SPACE, that is. */
>  			status_printf(s, GIT_COLOR_NORMAL,
>  				_("Please enter the commit message for your changes."

This cut line does not cover the merge conflict message before it. The following patch should be squashed in to move the cut line up in that case.

-- 8< --
Show changes to builtin/commit.c +5 −3
diff --git a/builtin/commit.c b/builtin/commit.c
index ea2912f..1033c50 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
 	if (use_editor && include_status) {
 		char *ai_tmp, *ci_tmp;
-		if (whence != FROM_COMMIT)
+		if (whence != FROM_COMMIT) {
+			if (cleanup_mode == CLEANUP_SCISSORS)
+				wt_status_add_cut_line(s->fp);
 			status_printf_ln(s, GIT_COLOR_NORMAL,
 			    whence == FROM_MERGE
 				? _("\n"
@@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 				git_path(whence == FROM_MERGE
 					 ? "MERGE_HEAD"
 					 : "CHERRY_PICK_HEAD"));
+		}
 
 		fprintf(s->fp, "\n");
 		if (cleanup_mode == CLEANUP_ALL)
@@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 				_("Please enter the commit message for your changes."
 				  " Lines starting\nwith '%c' will be ignored, and an empty"
 				  " message aborts the commit.\n"), comment_line_char);
-		else if (cleanup_mode == CLEANUP_SCISSORS)
+		else if (cleanup_mode == CLEANUP_SCISSORS && whence == FROM_COMMIT)
 			wt_status_add_cut_line(s->fp);
 		else /* CLEANUP_SPACE, that is. */
 			status_printf(s, GIT_COLOR_NORMAL,
-- 8< --
Junio C Hamano· Feb 24, 2014, 17:20 UTC · re: Duy Nguyen · lore

Re: [PATCH v2 3/3] commit: add --cleanup=scissors

Duy Nguyen <pclouds@gmail.com> writes:
Show 14 quoted lines
> On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote:
>> @@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>  				_("Please enter the commit message for your changes."
>>  				  " Lines starting\nwith '%c' will be ignored, and an empty"
>>  				  " message aborts the commit.\n"), comment_line_char);
>> +		else if (cleanup_mode == CLEANUP_SCISSORS)
>> +			wt_status_add_cut_line(s->fp);
>>  		else /* CLEANUP_SPACE, that is. */
>>  			status_printf(s, GIT_COLOR_NORMAL,
>>  				_("Please enter the commit message for your changes."
>
> This cut line does not cover the merge conflict message before it. The
> following patch should be squashed in to move the cut line up in that
> case.

I somehow thought that it was a policy decision we made in very early days that these conflict messages are meant to be edited with explanation of how they were resolved, not to be simply edited away?

The other stuff (commented out instructions and patch text) are meant to aid humans while editing the log message, and stripping away automatically after they are done editing like your patch does is perfectly fine, but I find this change questionable.

Show 34 quoted lines
> -- 8< --
> diff --git a/builtin/commit.c b/builtin/commit.c
> index ea2912f..1033c50 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  	strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
>  	if (use_editor && include_status) {
>  		char *ai_tmp, *ci_tmp;
> -		if (whence != FROM_COMMIT)
> +		if (whence != FROM_COMMIT) {
> +			if (cleanup_mode == CLEANUP_SCISSORS)
> +				wt_status_add_cut_line(s->fp);
>  			status_printf_ln(s, GIT_COLOR_NORMAL,
>  			    whence == FROM_MERGE
>  				? _("\n"
> @@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  				git_path(whence == FROM_MERGE
>  					 ? "MERGE_HEAD"
>  					 : "CHERRY_PICK_HEAD"));
> +		}
>  
>  		fprintf(s->fp, "\n");
>  		if (cleanup_mode == CLEANUP_ALL)
> @@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  				_("Please enter the commit message for your changes."
>  				  " Lines starting\nwith '%c' will be ignored, and an empty"
>  				  " message aborts the commit.\n"), comment_line_char);
> -		else if (cleanup_mode == CLEANUP_SCISSORS)
> +		else if (cleanup_mode == CLEANUP_SCISSORS && whence == FROM_COMMIT)
>  			wt_status_add_cut_line(s->fp);
>  		else /* CLEANUP_SPACE, that is. */
>  			status_printf(s, GIT_COLOR_NORMAL,
> -- 8< --
Duy Nguyen· Feb 24, 2014, 23:28 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 3/3] commit: add --cleanup=scissors

On Tue, Feb 25, 2014 at 12:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 25 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Mon, Feb 17, 2014 at 07:15:32PM +0700, Nguyễn Thái Ngọc Duy wrote:
>>> @@ -777,6 +778,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>>                              _("Please enter the commit message for your changes."
>>>                                " Lines starting\nwith '%c' will be ignored, and an empty"
>>>                                " message aborts the commit.\n"), comment_line_char);
>>> +            else if (cleanup_mode == CLEANUP_SCISSORS)
>>> +                    wt_status_add_cut_line(s->fp);
>>>              else /* CLEANUP_SPACE, that is. */
>>>                      status_printf(s, GIT_COLOR_NORMAL,
>>>                              _("Please enter the commit message for your changes."
>>
>> This cut line does not cover the merge conflict message before it. The
>> following patch should be squashed in to move the cut line up in that
>> case.
>
> I somehow thought that it was a policy decision we made in very
> early days that these conflict messages are meant to be edited with
> explanation of how they were resolved, not to be simply edited away?
>
> The other stuff (commented out instructions and patch text) are
> meant to aid humans while editing the log message, and stripping
> away automatically after they are done editing like your patch does
> is perfectly fine, but I find this change questionable.

I think I described it incorrectly as "conflict message" while it's not. This is the part to be cut out by the patch

# It looks like you may be committing a merge. # If this is not correct, please remove the file. # MERGE_HEAD # and try again.

(similar message for CHERRY_PICK_HEAD).
Show 35 quoted lines
>
>> -- 8< --
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index ea2912f..1033c50 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>       strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
>>       if (use_editor && include_status) {
>>               char *ai_tmp, *ci_tmp;
>> -             if (whence != FROM_COMMIT)
>> +             if (whence != FROM_COMMIT) {
>> +                     if (cleanup_mode == CLEANUP_SCISSORS)
>> +                             wt_status_add_cut_line(s->fp);
>>                       status_printf_ln(s, GIT_COLOR_NORMAL,
>>                           whence == FROM_MERGE
>>                               ? _("\n"
>> @@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>                               git_path(whence == FROM_MERGE
>>                                        ? "MERGE_HEAD"
>>                                        : "CHERRY_PICK_HEAD"));
>> +             }
>>
>>               fprintf(s->fp, "\n");
>>               if (cleanup_mode == CLEANUP_ALL)
>> @@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>                               _("Please enter the commit message for your changes."
>>                                 " Lines starting\nwith '%c' will be ignored, and an empty"
>>                                 " message aborts the commit.\n"), comment_line_char);
>> -             else if (cleanup_mode == CLEANUP_SCISSORS)
>> +             else if (cleanup_mode == CLEANUP_SCISSORS && whence == FROM_COMMIT)
>>                       wt_status_add_cut_line(s->fp);
>>               else /* CLEANUP_SPACE, that is. */
>>                       status_printf(s, GIT_COLOR_NORMAL,
>> -- 8< --
-- 
Duy
Junio C Hamano· Feb 25, 2014, 00:18 UTC · re: Duy Nguyen · lore

Re: [PATCH v2 3/3] commit: add --cleanup=scissors

Duy Nguyen <pclouds@gmail.com> writes:
Show 9 quoted lines
> I think I described it incorrectly as "conflict message" while it's
> not. This is the part to be cut out by the patch
>
> # It looks like you may be committing a merge.
> # If this is not correct, please remove the file.
> #     MERGE_HEAD
> # and try again.
>
> (similar message for CHERRY_PICK_HEAD).
Ahh, OK.  That should be removed, of course.
Will squash in (but not tonight).  Thanks.
Show 36 quoted lines
>
>>
>>> -- 8< --
>>> diff --git a/builtin/commit.c b/builtin/commit.c
>>> index ea2912f..1033c50 100644
>>> --- a/builtin/commit.c
>>> +++ b/builtin/commit.c
>>> @@ -755,7 +755,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>>       strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
>>>       if (use_editor && include_status) {
>>>               char *ai_tmp, *ci_tmp;
>>> -             if (whence != FROM_COMMIT)
>>> +             if (whence != FROM_COMMIT) {
>>> +                     if (cleanup_mode == CLEANUP_SCISSORS)
>>> +                             wt_status_add_cut_line(s->fp);
>>>                       status_printf_ln(s, GIT_COLOR_NORMAL,
>>>                           whence == FROM_MERGE
>>>                               ? _("\n"
>>> @@ -771,6 +773,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>>                               git_path(whence == FROM_MERGE
>>>                                        ? "MERGE_HEAD"
>>>                                        : "CHERRY_PICK_HEAD"));
>>> +             }
>>>
>>>               fprintf(s->fp, "\n");
>>>               if (cleanup_mode == CLEANUP_ALL)
>>> @@ -778,7 +781,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>>>                               _("Please enter the commit message for your changes."
>>>                                 " Lines starting\nwith '%c' will be ignored, and an empty"
>>>                                 " message aborts the commit.\n"), comment_line_char);
>>> -             else if (cleanup_mode == CLEANUP_SCISSORS)
>>> +             else if (cleanup_mode == CLEANUP_SCISSORS && whence == FROM_COMMIT)
>>>                       wt_status_add_cut_line(s->fp);
>>>               else /* CLEANUP_SPACE, that is. */
>>>                       status_printf(s, GIT_COLOR_NORMAL,
>>> -- 8< --

← back to recent threads