[PATCH] Add commit.infodisplay option to give message editor empty file

This patch creates commit.infodisplay which causes git commit to
display the status information on the standard output rather
than in the temporary file for the commit message.  By doing
this, it becomes feasible to set core.editor for commit messages
to be a line editor, e.g. ex or ed.

Signed-off-by: James P. Howard, II <redacted>
---
 Documentation/config.txt |    5 +++++
 builtin-commit.c         |   31 ++++++++++++++++++-------------
 cache.h                  |    1 +
 environment.c            |    3 +++
 4 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a1e36d7..56b3238 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -705,6 +705,11 @@ color.ui::
 	terminal. When more specific variables of color.* are set, they always
 	take precedence over this setting. Defaults to false.
 
+commit.infodisplay::
+	When true and a commit message is not specified on the command line,
+	the status information is not placed in the message template but is
+	printed to the standard output.
+
 commit.template::
 	Specify a file to use as the template for new commit messages.
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the

diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..e4db374 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -463,7 +463,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	int commitable, saved_color_setting;
 	struct strbuf sb = STRBUF_INIT;
 	char *buffer;
-	FILE *fp;
+	FILE *fp, *infofp;
 	const char *hook_arg1 = NULL;
 	const char *hook_arg2 = NULL;
 	int ident_shown = 0;

@@ -540,7 +540,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
 		die_errno("could not write commit template");
-
+	if (info_display)
+		infofp = stdout;
+	else {
+		infofp = fp;
+		fprintf(infofp, "\n");
+	}
 	strbuf_release(&sb);
 
 	determine_author_info();

@@ -552,7 +557,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		const char *committer_ident;
 
 		if (in_merge)
-			fprintf(fp,
+			fprintf(infofp,
 				"#\n"
 				"# It looks like you may be committing a MERGE.\n"
 				"# If this is not correct, please remove the file\n"

@@ -561,28 +566,27 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 				"#\n",
 				git_path("MERGE_HEAD"));
 
-		fprintf(fp,
-			"\n"
+		fprintf(infofp,
 			"# Please enter the commit message for your changes.");
 		if (cleanup_mode == CLEANUP_ALL)
-			fprintf(fp,
+			fprintf(infofp,
 				" Lines starting\n"
 				"# with '#' will be ignored, and an empty"
 				" message aborts the commit.\n");
 		else /* CLEANUP_SPACE, that is. */
-			fprintf(fp,
+			fprintf(infofp,
 				" Lines starting\n"
 				"# with '#' will be kept; you may remove them"
 				" yourself if you want to.\n"
 				"# An empty message aborts the commit.\n");
 		if (only_include_assumed)
-			fprintf(fp, "# %s\n", only_include_assumed);
+			fprintf(infofp, "# %s\n", only_include_assumed);
 
 		author_ident = xstrdup(fmt_name(author_name, author_email));
 		committer_ident = fmt_name(getenv("GIT_COMMITTER_NAME"),
 					   getenv("GIT_COMMITTER_EMAIL"));
 		if (strcmp(author_ident, committer_ident))
-			fprintf(fp,
+			fprintf(infofp,
 				"%s"
 				"# Author:    %s\n",
 				ident_shown++ ? "" : "#\n",

@@ -590,18 +594,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		free(author_ident);
 
 		if (!user_ident_explicitly_given)
-			fprintf(fp,
+			fprintf(infofp,
 				"%s"
 				"# Committer: %s\n",
 				ident_shown++ ? "" : "#\n",
 				committer_ident);
 
 		if (ident_shown)
-			fprintf(fp, "#\n");
+			fprintf(infofp, "#\n");
 
 		saved_color_setting = s->use_color;
 		s->use_color = 0;
-		commitable = run_status(fp, index_file, prefix, 1, s);
+		commitable = run_status(infofp, index_file, prefix, 1, s);
 		s->use_color = saved_color_setting;
 	} else {
 		unsigned char sha1[20];

@@ -1006,7 +1010,8 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 
 	if (!strcmp(k, "commit.template"))
 		return git_config_pathname(&template_file, k, v);
-
+	if (!strcmp(k, "commit.infodisplay"))
+		info_display = git_config_bool(k, v);
 	return git_status_config(k, v, s);
 }
 

diff --git a/cache.h b/cache.h
index bf468e5..2b36fb3 100644
--- a/cache.h
+++ b/cache.h

@@ -529,6 +529,7 @@ extern int auto_crlf;
 extern int read_replace_refs;
 extern int fsync_object_files;
 extern int core_preload_index;
+extern int info_display;
 
 enum safe_crlf {
 	SAFE_CRLF_FALSE = 0,

diff --git a/environment.c b/environment.c
index 5171d9f..ac7cfed 100644
--- a/environment.c
+++ b/environment.c

@@ -55,6 +55,9 @@ int grafts_replace_parents = 1;
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
 
+/* Controls whether commit information is appended to message text or displayed */
+int info_display = 0;
+
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
 static char *work_tree;

-- 
1.6.5.3

"James P. Howard, II" [off-list ref] writes:

This patch creates commit.infodisplay which causes git commit to
display the status information on the standard output rather
than in the temporary file for the commit message.  By doing
this, it becomes feasible to set core.editor for commit messages
to be a line editor, e.g. ex or ed.

Two points and a half:

 - Why does info_display need to be visible to everybody else by being in
   cache.h and environment.c?  Shouldn't its scope be similar to that of
   template_file, that is static to builtin-commit.c?

 - A configuration variable that does not allow the users to override from
   the command line is a no-no.  We usually do command line option first
   and then (or at the same time) configuration variable as a typesaver.

 - While the amount of change necessary for this change doesn't look too
   bad, is it really worth it?  What is wrong with "1,$p" while using ed
   as your editor?

On Fri, Dec 04, 2009 at 11:30:56PM -0800, Junio C Hamano wrote:

Two points and a half:

 - Why does info_display need to be visible to everybody else by being in
   cache.h and environment.c?  Shouldn't its scope be similar to that of
   template_file, that is static to builtin-commit.c?

 - A configuration variable that does not allow the users to override from
   the command line is a no-no.  We usually do command line option first
   and then (or at the same time) configuration variable as a typesaver.

I can make these changes in the next few days.
 

 - While the amount of change necessary for this change doesn't look too
   bad, is it really worth it?  What is wrong with "1,$p" while using ed
   as your editor?

I tried a few variants of this, and shell script wrappers for ex (and
gate, which is a specizalized text editor that actually works kind of
well for commit message editing), and it worked fairly well.

The real motivation is that this feature ditches all assumptions about
the capabilities of the text editor.  I am not sure where that leads
yet, but I'd rather make it possible.

James

-- 
James P. Howard, II, MPA MBCS
jh@jameshoward.us

On Sat, Dec 05, 2009 at 10:47:53AM -0500, James P. Howard, II wrote:

quoted

 - While the amount of change necessary for this change doesn't look too
   bad, is it really worth it?  What is wrong with "1,$p" while using ed
   as your editor?

I tried a few variants of this, and shell script wrappers for ex (and
gate, which is a specizalized text editor that actually works kind of
well for commit message editing), and it worked fairly well.

The real motivation is that this feature ditches all assumptions about
the capabilities of the text editor.  I am not sure where that leads
yet, but I'd rather make it possible.

Is your problem that your editor doesn't show the template content and
you want to see it, or is it that your editor isn't pleasant to use when
the buffer is pre-filled with the template?

If the former, it seems like just dumping it to stdout isn't all that
satisfying, either. What happens when your editing causes the
information to scroll off the screen and you want to see it again?
Couldn't you get the same thing just by doing "git status; git commit"?

If the latter, I think we would be better served by an option to simply
turn off the template. Then that is also helpful for the case of people
using decent editors, but who don't want to waste the CPU time on
generating the template information (which can be substantial for things
like media repositories).

I suspect your answer will be that it is some of both, but this just
really feels like we are putting hacks into git because of one
featureless editor. Hacks like that would be better suited (IMHO) to a
wrapper script for the editor.

-Peff

On Sat, Dec 05, 2009 at 11:28:27AM -0500, Jeff King wrote:

If the latter, I think we would be better served by an option to simply
turn off the template. Then that is also helpful for the case of people
using decent editors, but who don't want to waste the CPU time on
generating the template information (which can be substantial for things
like media repositories).

Actually, I find this a reasonable solution for both cases and would be
willing to reimplment my change this way, as it meets my needs and would
be useful to others.  The only question I have is, what should variable/
command line option be called?

James

-- 
James P. Howard, II, MPA MBCS
jh@jameshoward.us

On Sat, Dec 05, 2009 at 06:09:03PM -0500, James P. Howard, II wrote:

On Sat, Dec 05, 2009 at 11:28:27AM -0500, Jeff King wrote:

quoted

If the latter, I think we would be better served by an option to simply
turn off the template. Then that is also helpful for the case of people
using decent editors, but who don't want to waste the CPU time on
generating the template information (which can be substantial for things
like media repositories).

Actually, I find this a reasonable solution for both cases and would be
willing to reimplment my change this way, as it meets my needs and would
be useful to others.  The only question I have is, what should variable/
command line option be called?

I would be tempted to call it "--no-template", but I think that is too
confusing. The "--template" option is not really about the git-generated
template, but about a user-defined template that goes on top of the
git-generated one (I would have expected --template=/dev/null to do what
you want, too, but it retains the git template).

Probably "--no-status" would be a good name, as the generated template
is the format generated by "git status".

-Peff

On Sat, Dec 5, 2009 at 23:22, Jeff King [off-list ref] wrote:

Probably "--no-status" would be a good name, as the generated template
is the format generated by "git status".

I have produced a patch against master that implements this in, what I
hope, is a better way.

James

-- 
James P. Howard, II, MPA MBCS
jh@jameshoward.us