Remove help advice text from git editors for interactive rebase and reword

Remove help advice text from git editors for interactive rebase and reword

[Kirill Likhodedov]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

Hello,

is it possible to remove the helping text which appears at the bottom of the Git interactive rebase editor (the one with the list of instructions), and the one which appears at the bottom of the commit editor (which appears on rewording a commit or squashing commits)? 

The texts I'm talking about are:

# Rebase e025896..efc3d17 onto e025896¬
#¬
# Commands:¬
#  p, pick = use commit¬
...

and

# Please enter the commit message for your changes. Lines starting¬
# with '#' will be ignored, and an empty message aborts the commit.
# Not currently on any branch.¬
...


If there is no way to do it now, do you think it makes sense to provide a configuration variable for this, e.g. to introduce more advice.* config variables in addition to existing ones?

My motivation is the following: I'm improving the Git client inside of IntelliJ IDEA IDE and I would like to provide only the plain commit message text to the user (any hints can be shown separately, not inside the editor).

I know I can load the original commit message myself (but I prefer not to make extra calls when possible); and I can parse and strip out the help pages (but it is not very reliable since the text may change in future), so I'd appreciate any other solution to my problem, as well.

However I suppose that experienced command line users could also benefit from such configuration, since this helping text is intended only for newbies and is more like a noise for advanced users.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2857 bytes --]

Re: Remove help advice text from git editors for interactive rebase and reword

[Alexei Lozovsky]

On 23 July 2017 at 13:03, Kirill Likhodedov wrote:
> Hello,
>
> is it possible to remove the helping text which appears at the bottom
> of the Git interactive rebase editor (the one with the list of
> instructions)

I believe currently there is not way to do it. The interactive rebase
is implemented in git-rebase--interactive.sh which always makes a call
to append_todo_help to append the help text to the todo list of commits.

> and the one which appears at the bottom of the commit editor (which
> appears on rewording a commit or squashing commits)?

This one too seems to be hardcoded in builtin/commit.c.

> I can parse and strip out the help pages (but it is not very reliable
> since the text may change in future)

I doubt the syntax of the interactive rebase todo list will ever change,
so you can reliably remove all lines that are empty or start with the
$(git config --get core.commentchar) or '#' if that's empty or 'auto'.

However, it's harder with the commit messages during --amend as the
comment character is not really fixed and can be dynamically selected
to not conflict with the characters used in the commit message if the
core.commentchar is set to 'auto'.

> However I suppose that experienced command line users could also
> benefit from such configuration, since this helping text is intended
> only for newbies and is more like a noise for advanced users.

Well, the text is appended to the todo list of commits, so not that it
gets too much in the way of editing the list by humans.

Re: Remove help advice text from git editors for interactive rebase and reword

[Junio C Hamano]

Kirill Likhodedov <kirill.likhodedov@jetbrains.com> writes:

> My motivation is the following: I'm improving the Git client
> inside of IntelliJ IDEA IDE and I would like to provide only the
> plain commit message text to the user (any hints can be shown
> separately, not inside the editor).

Who is running "git commit --amend" and "git rebase -i" in the
workflow of a user of your tool?  Is it the end user who types these
commands to the shell command prompt, or does your tool formulate
the command line and does an equivalent of system(3) to run it?

I am assuming that the answer is the latter in my response.

> If there is no way to do it now, do you think it makes sense to
> provide a configuration variable for this, e.g. to introduce more
> advice.* config variables in addition to existing ones?

Not at all interested, as that would mean your tool will tell its
users to set such a configuration variable and their interactive use
of Git outside your tool will behave differently from other people
who use vanilla Git, and they will complain to us.

But I do not think adding a new command line option that only is
passed by a tool like yours when it runs "git rebase -i" via
system(3) equivalent would introduce such an issue, so that may be
workable.

But stepping back a bit, as you said in the parentheses, your tool
would need to grab these "hints" from Git, instead of having a
separate hardcoded hints that will go stale while the underlying Git
command improves, to be able to show them "separately".  Which means
to me that you would need to get the output Git would normally show
to the end user and do your own splitting and parsing anyway.  Which
in turn would mean that a configuration or a command line option to
squelch these, which would rob your tool the ability to read what
Git would have told to your users, would be a bad idea and not a
useful addition to the overall system.  So...

Re: Remove help advice text from git editors for interactive rebase and reword

[Kirill Likhodedov]

[-- Attachment #1: Type: text/plain, Size: 1849 bytes --]


> On 24 Jul 2017, at 01:09 , Junio C Hamano <gitster@pobox.com> wrote:
> 
> Who is running "git commit --amend" and "git rebase -i" in the
> workflow of a user of your tool?  Is it the end user who types these
> commands to the shell command prompt, or does your tool formulate
> the command line and does an equivalent of system(3) to run it?
> 
> I am assuming that the answer is the latter in my response.

Yes, it is the latter case: the tool formulates the command line and forks a process.

> Not at all interested, as that would mean your tool will tell its
> users to set such a configuration variable and their interactive use
> of Git outside your tool will behave differently from other people
> who use vanilla Git, and they will complain to us.

That's not true, since the tool can (and would) use the `git -c config.var=value rebase -i` syntax to set the configuration variable just for this particular command, without affecting the environment.

Btw, if my proposal is so uninteresting, why the existing advice.* variables were previously introduced? I don't know the motivation, but assume that it was about making Git less wordy for experienced users. So I don't see any difference here.

> But stepping back a bit, as you said in the parentheses, your tool
> would need to grab these "hints" from Git, instead of having a
> separate hardcoded hints that will go stale while the underlying Git
> command improves, to be able to show them "separately".  

There is no need to call Git to get these "hints". They are quite obvious, well-known and can be hardcoded. However, I don't plan to use these hints anyway, since they are a bit foreign to the GUI of the tool I develop. For instance, for reword I'd like to show an editor containing just the plain commit message that the user is about to change. 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2857 bytes --]

Re: Remove help advice text from git editors for interactive rebase and reword

[Jeff King]

On Mon, Jul 24, 2017 at 01:26:09AM +0300, Kirill Likhodedov wrote:

> > Not at all interested, as that would mean your tool will tell its
> > users to set such a configuration variable and their interactive use
> > of Git outside your tool will behave differently from other people
> > who use vanilla Git, and they will complain to us.
> 
> That's not true, since the tool can (and would) use the `git -c
> config.var=value rebase -i` syntax to set the configuration variable
> just for this particular command, without affecting the environment.

Yes, but if you are adding a config variable that is only ever meant to
be used from the command line, it probably makes sense to just add a
command-line option.

> Btw, if my proposal is so uninteresting, why the existing advice.*
> variables were previously introduced? I don't know the motivation, but
> assume that it was about making Git less wordy for experienced users.
> So I don't see any difference here.

That is exactly what advice.* is for, but it is about the _user_
deciding that they don't care about seeing that text. Not a tool that is
calling Git deciding that in one particular context, it would like to
suppress the hint text.

So I actually would be OK with having an advice.* option to squelch
rebase and/or commit instructions. But only if users decide they would
never like to see that text. So yes, your tool could piggy-back on that
config option, but it would be a slight abuse of the intent.

> > But stepping back a bit, as you said in the parentheses, your tool
> > would need to grab these "hints" from Git, instead of having a
> > separate hardcoded hints that will go stale while the underlying Git
> > command improves, to be able to show them "separately".  
> 
> There is no need to call Git to get these "hints". They are quite
> obvious, well-known and can be hardcoded. However, I don't plan to use
> these hints anyway, since they are a bit foreign to the GUI of the
> tool I develop. For instance, for reword I'd like to show an editor
> containing just the plain commit message that the user is about to
> change.

If this is all scripted anyway, wouldn't it be an option to just process
the commit message in your program?  The format is well-known, with
hints and instructions on lines marked by core.commentChar ("#" by
default).

I'm not sure exactly of the flow in which the user sees the commit
message buffer (i.e., if you are invoking the editor yourself, or if you
are relying on git-commit to do so). But even in the latter case, you
can hook the editor invocation to do whatever you like. For example:

  GIT_EDITOR='f() { sed -i /^#/d "$1"; $EDITOR "$1"; }; f' git commit

That allows you not only to strip out the existing instructions, but to
insert whatever other instructions you choose.

-Peff

Re: Remove help advice text from git editors for interactive rebase and reword

[SZEDER Gábor]

> So I actually would be OK with having an advice.* option to squelch
> rebase and/or commit instructions. But only if users decide they would
> never like to see that text. So yes, your tool could piggy-back on that
> config option, but it would be a slight abuse of the intent.

I don't mind the advice in the interactive rebase TODO list.  It's at
the end of the file, nothing comes after that, so it's never in the
way.

However, I do care about the advices in the commit message template,
because they are _between_ the commit message I'm writing and the diff
(using commit.verbose=true) I'm writing about.  So I build git
for my own use with the patch below for a couple of years now, but
never submitted it.


On a related note, when committing a merge or cherry-pick the commit
message templates includes this:

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

This text traces back almost to the dawn of time, to commit 9c065315f
(Make "git commit" work correctly in the presense of a manual merge,
2005-06-08).  Now, I can well imagine that stray MERGE_HEAD files
caused troubles back then, especially with those "manual merges"...  
But is it really an issue with modern git?!  I think this is long
outdated and could be removed.


  -- >8 --

Subject: [PATCH] commit: allow suppression of commit message template advices

The commit message template includes a lot of advices:

  - The default commit message template asks the user nicely to write
    a commit message and tells about comments and how to abort.
  - It includes some outdated hints about merges and cherry-picks.
  - Finally, in case of 'git commit -v' it reminds about the role of
    the scissors line separating the commit message from the diff.

While these reminders are useful for new users, with time they learn
what the score is, and experienced users might find these advices are
just wasting a couple of lines' worth of screen real estate.

Make displaying these advices configurable via the 'advice.commitMsg'
config variable.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 Documentation/config.txt |  2 ++
 advice.c                 |  2 ++
 advice.h                 |  1 +
 builtin/commit.c         | 69 ++++++++++++++++++++++++++----------------------
 wt-status.c              | 14 +++++-----
 5 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5c9c4cab..29c8736b1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -331,6 +331,8 @@ advice.*::
 	commitBeforeMerge::
 		Advice shown when linkgit:git-merge[1] refuses to
 		merge to avoid overwriting local changes.
+	commitMsg::
+		Advices shown in the commit message template.
 	resolveConflict::
 		Advice shown by various commands when conflicts
 		prevent the operation from being performed.
diff --git a/advice.c b/advice.c
index d81e1cb74..7851bb20c 100644
--- a/advice.c
+++ b/advice.c
@@ -10,6 +10,7 @@ int advice_push_needs_force = 1;
 int advice_status_hints = 1;
 int advice_status_u_option = 1;
 int advice_commit_before_merge = 1;
+int advice_commit_msg = 1;
 int advice_resolve_conflict = 1;
 int advice_implicit_identity = 1;
 int advice_detached_head = 1;
@@ -31,6 +32,7 @@ static struct {
 	{ "statushints", &advice_status_hints },
 	{ "statusuoption", &advice_status_u_option },
 	{ "commitbeforemerge", &advice_commit_before_merge },
+	{ "commitmsg", &advice_commit_msg },
 	{ "resolveconflict", &advice_resolve_conflict },
 	{ "implicitidentity", &advice_implicit_identity },
 	{ "detachedhead", &advice_detached_head },
diff --git a/advice.h b/advice.h
index c84a44531..92c9937d6 100644
--- a/advice.h
+++ b/advice.h
@@ -12,6 +12,7 @@ extern int advice_push_needs_force;
 extern int advice_status_hints;
 extern int advice_status_u_option;
 extern int advice_commit_before_merge;
+extern int advice_commit_msg;
 extern int advice_resolve_conflict;
 extern int advice_implicit_identity;
 extern int advice_detached_head;
diff --git a/builtin/commit.c b/builtin/commit.c
index 8e9380251..ead7bf5ef 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -810,38 +810,42 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		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"
-					"It looks like you may be committing a merge.\n"
-					"If this is not correct, please remove the file\n"
-					"	%s\n"
-					"and try again.\n")
-				: _("\n"
-					"It looks like you may be committing a cherry-pick.\n"
-					"If this is not correct, please remove the file\n"
-					"	%s\n"
-					"and try again.\n"),
-				whence == FROM_MERGE ?
-					git_path_merge_head() :
-					git_path_cherry_pick_head());
+			if (advice_commit_msg)
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+				    whence == FROM_MERGE
+					? _("\n"
+						"It looks like you may be committing a merge.\n"
+						"If this is not correct, please remove the file\n"
+						"	%s\n"
+						"and try again.\n")
+					: _("\n"
+						"It looks like you may be committing a cherry-pick.\n"
+						"If this is not correct, please remove the file\n"
+						"	%s\n"
+						"and try again.\n"),
+					whence == FROM_MERGE ?
+						git_path_merge_head() :
+						git_path_cherry_pick_head());
 		}
 
 		fprintf(s->fp, "\n");
-		if (cleanup_mode == CLEANUP_ALL)
-			status_printf(s, GIT_COLOR_NORMAL,
-				_("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 && whence == FROM_COMMIT)
+		if (cleanup_mode == CLEANUP_ALL) {
+			if (advice_commit_msg)
+				status_printf(s, GIT_COLOR_NORMAL,
+					_("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 && whence == FROM_COMMIT)
 			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."
-				  " Lines starting\n"
-				  "with '%c' will be kept; you may remove them"
-				  " yourself if you want to.\n"
-				  "An empty message aborts the commit.\n"), comment_line_char);
+		else /* CLEANUP_SPACE, that is. */ {
+			if (advice_commit_msg)
+				status_printf(s, GIT_COLOR_NORMAL,
+					_("Please enter the commit message for your changes."
+					  " Lines starting\n"
+					  "with '%c' will be kept; you may remove them"
+					  " yourself if you want to.\n"
+					  "An empty message aborts the commit.\n"), comment_line_char);
+		}
 
 		/*
 		 * These should never fail because they come from our own
@@ -856,7 +860,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			status_printf_ln(s, GIT_COLOR_NORMAL,
 				_("%s"
 				"Author:    %.*s <%.*s>"),
-				ident_shown++ ? "" : "\n",
+				ident_shown++ || !advice_commit_msg ? "" : "\n",
 				(int)(ai.name_end - ai.name_begin), ai.name_begin,
 				(int)(ai.mail_end - ai.mail_begin), ai.mail_begin);
 
@@ -864,18 +868,19 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			status_printf_ln(s, GIT_COLOR_NORMAL,
 				_("%s"
 				"Date:      %s"),
-				ident_shown++ ? "" : "\n",
+				ident_shown++ || !advice_commit_msg ? "" : "\n",
 				show_ident_date(&ai, DATE_MODE(NORMAL)));
 
 		if (!committer_ident_sufficiently_given())
 			status_printf_ln(s, GIT_COLOR_NORMAL,
 				_("%s"
 				"Committer: %.*s <%.*s>"),
-				ident_shown++ ? "" : "\n",
+				ident_shown++ || !advice_commit_msg ? "" : "\n",
 				(int)(ci.name_end - ci.name_begin), ci.name_begin,
 				(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);
 
-		status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); /* Add new line for clarity */
+		if (ident_shown || advice_commit_msg)
+			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); /* Add new line for clarity */
 
 		saved_color_setting = s->use_color;
 		s->use_color = 0;
diff --git a/wt-status.c b/wt-status.c
index 77c27c511..09cb24be9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -934,13 +934,15 @@ size_t wt_status_locate_end(const char *s, size_t len)
 
 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);
+	if (advice_commit_msg) {
+		const char *explanation = _("Do not touch the line above.\nEverything below will be removed.");
+		struct strbuf buf = STRBUF_INIT;
+
+		strbuf_add_commented_lines(&buf, explanation, strlen(explanation));
+		fputs(buf.buf, fp);
+		strbuf_release(&buf);
+	}
 }
 
 static void wt_longstatus_print_verbose(struct wt_status *s)
-- 
2.14.0.rc0.88.ge338f4246

Re: Remove help advice text from git editors for interactive rebase and reword

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> While these reminders are useful for new users, with time they learn
> what the score is, and experienced users might find these advices are
> just wasting a couple of lines' worth of screen real estate.
>
> Make displaying these advices configurable via the 'advice.commitMsg'
> config variable.

It may not be a bad idea, but the code after the patch does look
ugly with too deep indentation levels.  Can some refactoring help, I
wonder?

Is that advice.commitMsg?  It looks more like commitEditor advice to
me but it may be just me.