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

2014-02-24 Thread Junio C Hamano
Duy Nguyen pclo...@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.

 -- 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 --
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-02-24 Thread Duy Nguyen
On Tue, Feb 25, 2014 at 12:20 AM, Junio C Hamano gits...@pobox.com wrote:
 Duy Nguyen pclo...@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).


 -- 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-02-24 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 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.



 -- 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 --
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-02-21 Thread Duy Nguyen
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.

-- 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 --
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-02-17 Thread Nguyễn Thái Ngọc Duy
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 pclo...@gmail.com
---
 Documentation/git-commit.txt |  8 +++-
 builtin/commit.c |  9 +++--
 t/t7502-commit.sh| 16 
 3 files changed, 30 insertions(+), 3 deletions(-)

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,/^\$/dactual 
+   test_cmp expect actual
+
+'
+
 test_expect_success 'cleanup commit messages (strip option,-F)' '
 
echo negative 
-- 
1.8.5.2.240.g8478abd

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html