[RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case

[RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case

[Jonathan Nieder]

Hi,

When last seen[1], this series was a single patch in very rough form,
but there have been almost no functional changes since then.

The patches suppress most output when “git commit” is run without
stages changed.  So instead of

	$ git commit
	# On branch master
	# Changed but not updated:
	#   (use "git add <file>..." to update what will be committed)
	#   (use "git checkout -- <file>..." to discard changes in working directory)
	#
	#	modified:   dir1/modified
	#
	# Untracked files:
	#   (use "git add <file>..." to include in what will be committed)
	#
	#	actual
	#	dir1/untracked
	#	dir2/modified
	#	dir2/untracked
	#	expect
	#	output
	#	untracked
	no changes added to commit (use "git add" and/or "git commit -a")

which may cause a newcomer to panic, you get

	$ git commit
	no changes added to commit (use "git add" and/or "git commit -a")

which would just cause her to scratch her head or say “oh, right!”
instead.  Hopefully these patches will at least provide a reminder to
improve the various "no changes" advice messages.

Ideas for future work:

 - add some tests
 - give the full traditional output if -a or any paths were passed on
   the command line.

Most of the patches are code clarity improvements which is not
strictly related to this topic.

Patch 6 cleans up the most obvious script to add tests for this in,
though I have not added any tests to it.

Patch 8 changes commit --dry-run output in a more modest way, to
print the same advice Jeff added to commit proper last month.  I
suspect this is a good change, but input from people who script
around commit --dry-run would be welcome.

Patch 9 is the advertised patch.  It should be self-explanatory.

Thoughts?

Jonathan Nieder (9):
  wt-status: split wt_status_print into digestible pieces
  wt-status: give submodule summary printing its own function
  commit: split off a function to fetch the default log message
  commit: encapsulate commit -s handling in its own function
  commit: split off the piece that writes status
  t7508 (status): modernize style
  commit: give empty-commit avoidance code its own function
  commit --dry-run: give advice on empty amend
  commit: suppress status summary when no changes staged

 builtin/commit.c  |  328 ++++++++------
 t/t7508-status.sh | 1380 ++++++++++++++++++++++++++++-------------------------
 wt-status.c       |  103 +++--
 wt-status.h       |    1 +
 4 files changed, 989 insertions(+), 823 deletions(-)

[1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/698001/focus=145541

[PATCH 1/9] wt-status: split wt_status_print into digestible pieces

[Jonathan Nieder]

The result does not fit on a 24-line terminal yet, but it’s
getting close.  No functional change intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 wt-status.c |   85 ++++++++++++++++++++++++++++++++---------------------------
 1 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 2f9e33c..b0f17cf 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -604,6 +604,31 @@ static void wt_status_print_verbose(struct wt_status *s)
 	run_diff_index(&rev, 1);
 }
 
+static void wt_status_print_nochanges(struct wt_status *s)
+{
+	if (s->amend)
+		fprintf(s->fp, "# No changes\n");
+	else if (s->nowarn)
+		; /* nothing */
+	else if (s->workdir_dirty)
+		printf("no changes added to commit%s\n",
+			advice_status_hints
+			? " (use \"git add\" and/or \"git commit -a\")" : "");
+	else if (s->untracked.nr)
+		printf("nothing added to commit but untracked files present%s\n",
+			advice_status_hints
+			? " (use \"git add\" to track)" : "");
+	else if (s->is_initial)
+		printf("nothing to commit%s\n", advice_status_hints
+			? " (create/copy files and use \"git add\" to track)" : "");
+	else if (!s->show_untracked_files)
+		printf("nothing to commit%s\n", advice_status_hints
+			? " (use -u to show untracked files)" : "");
+	else
+		printf("nothing to commit%s\n", advice_status_hints
+			? " (working directory clean)" : "");
+}
+
 static void wt_status_print_tracking(struct wt_status *s)
 {
 	struct strbuf sb = STRBUF_INIT;
@@ -623,25 +648,28 @@ static void wt_status_print_tracking(struct wt_status *s)
 	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
 }
 
-void wt_status_print(struct wt_status *s)
+static void wt_status_print_onbranch(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_HEADER, s);
-
-	if (s->branch) {
-		const char *on_what = "On branch ";
-		const char *branch_name = s->branch;
-		if (!prefixcmp(branch_name, "refs/heads/"))
-			branch_name += 11;
-		else if (!strcmp(branch_name, "HEAD")) {
-			branch_name = "";
-			branch_color = color(WT_STATUS_NOBRANCH, s);
-			on_what = "Not currently on any branch.";
-		}
-		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "# ");
-		color_fprintf_ln(s->fp, branch_color, "%s%s", on_what, branch_name);
-		if (!s->is_initial)
-			wt_status_print_tracking(s);
+	const char *on_what = "On branch ";
+	const char *branch_name = s->branch;
+	if (!prefixcmp(branch_name, "refs/heads/"))
+		branch_name += 11;
+	else if (!strcmp(branch_name, "HEAD")) {
+		branch_name = "";
+		branch_color = color(WT_STATUS_NOBRANCH, s);
+		on_what = "Not currently on any branch.";
 	}
+	color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "# ");
+	color_fprintf_ln(s->fp, branch_color, "%s%s", on_what, branch_name);
+	if (!s->is_initial)
+		wt_status_print_tracking(s);
+}
+
+void wt_status_print(struct wt_status *s)
+{
+	if (s->branch)
+		wt_status_print_onbranch(s);
 
 	if (s->is_initial) {
 		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
@@ -669,29 +697,8 @@ void wt_status_print(struct wt_status *s)
 
 	if (s->verbose)
 		wt_status_print_verbose(s);
-	if (!s->commitable) {
-		if (s->amend)
-			fprintf(s->fp, "# No changes\n");
-		else if (s->nowarn)
-			; /* nothing */
-		else if (s->workdir_dirty)
-			printf("no changes added to commit%s\n",
-				advice_status_hints
-				? " (use \"git add\" and/or \"git commit -a\")" : "");
-		else if (s->untracked.nr)
-			printf("nothing added to commit but untracked files present%s\n",
-				advice_status_hints
-				? " (use \"git add\" to track)" : "");
-		else if (s->is_initial)
-			printf("nothing to commit%s\n", advice_status_hints
-				? " (create/copy files and use \"git add\" to track)" : "");
-		else if (!s->show_untracked_files)
-			printf("nothing to commit%s\n", advice_status_hints
-				? " (use -u to show untracked files)" : "");
-		else
-			printf("nothing to commit%s\n", advice_status_hints
-				? " (working directory clean)" : "");
-	}
+	if (!s->commitable)
+		wt_status_print_nochanges(s);
 }
 
 static void wt_shortstatus_unmerged(int null_termination, struct string_list_item *it,
-- 
1.7.2.9.ge3789.dirty

[PATCH 2/9] wt-status: split off a function for printing submodule summary

[Jonathan Nieder]

This only shaves a couple lines from wt_status_print().

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 wt-status.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index b0f17cf..90a0824 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -522,7 +522,7 @@ static void wt_status_print_changed(struct wt_status *s)
 	wt_status_print_trailer(s);
 }
 
-static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitted)
+static void do_submodule_summary(struct wt_status *s, int uncommitted)
 {
 	struct child_process sm_summary;
 	char summary_limit[64];
@@ -553,6 +553,14 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	run_command(&sm_summary);
 }
 
+static void wt_status_print_submodule_summary(struct wt_status *s)
+{
+	if (s->ignore_submodule_arg && !strcmp(s->ignore_submodule_arg, "all"))
+		return;
+	do_submodule_summary(s, 0);  /* staged */
+	do_submodule_summary(s, 1);  /* unstaged */
+}
+
 static void wt_status_print_other(struct wt_status *s,
 				  struct string_list *l,
 				  const char *what,
@@ -680,12 +688,8 @@ void wt_status_print(struct wt_status *s)
 	wt_status_print_updated(s);
 	wt_status_print_unmerged(s);
 	wt_status_print_changed(s);
-	if (s->submodule_summary &&
-	    (!s->ignore_submodule_arg ||
-	     strcmp(s->ignore_submodule_arg, "all"))) {
-		wt_status_print_submodule_summary(s, 0);  /* staged */
-		wt_status_print_submodule_summary(s, 1);  /* unstaged */
-	}
+	if (s->submodule_summary)
+		wt_status_print_submodule_summary(s);
 	if (s->show_untracked_files) {
 		wt_status_print_other(s, &s->untracked, "Untracked", "add");
 		if (s->show_ignored_files)
-- 
1.7.2.9.ge3789.dirty

[PATCH 3/9] commit: split off a function to fetch the default log message

[Jonathan Nieder]

The details of how the default message template is grabbed from
MERGE_MSG will be irrelevant to most people reading the commit
preparation code.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/commit.c |   94 +++++++++++++++++++++++++++++++----------------------
 1 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a78dbd8..6774180 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -549,62 +549,78 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 	return 1;
 }
 
-static int prepare_to_commit(const char *index_file, const char *prefix,
-			     struct wt_status *s)
+/*
+ * Return value is the "source" argument for hooks/prepare-commit-msg.
+ */
+static const char *get_template_message(struct strbuf *sb,
+					const char **hook_arg2)
 {
 	struct stat statbuf;
-	int commitable, saved_color_setting;
-	struct strbuf sb = STRBUF_INIT;
-	char *buffer;
-	FILE *fp;
-	const char *hook_arg1 = NULL;
-	const char *hook_arg2 = NULL;
-	int ident_shown = 0;
-
-	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
-		return 0;
-
 	if (message.len) {
-		strbuf_addbuf(&sb, &message);
-		hook_arg1 = "message";
-	} else if (logfile && !strcmp(logfile, "-")) {
+		strbuf_addbuf(sb, &message);
+		return "message";
+	}
+	if (logfile && !strcmp(logfile, "-")) {
 		if (isatty(0))
 			fprintf(stderr, "(reading log message from standard input)\n");
-		if (strbuf_read(&sb, 0, 0) < 0)
+		if (strbuf_read(sb, 0, 0) < 0)
 			die_errno("could not read log from standard input");
-		hook_arg1 = "message";
-	} else if (logfile) {
-		if (strbuf_read_file(&sb, logfile, 0) < 0)
-			die_errno("could not read log file '%s'",
-				  logfile);
-		hook_arg1 = "message";
-	} else if (use_message) {
-		buffer = strstr(use_message_buffer, "\n\n");
+		return "message";
+	}
+	if (logfile) {
+		if (strbuf_read_file(sb, logfile, 0) < 0)
+			die_errno("could not read log file '%s'", logfile);
+		return "message";
+	}
+	if (use_message) {
+		char *buffer = strstr(use_message_buffer, "\n\n");
 		if (!buffer || buffer[2] == '\0')
 			die("commit has empty message");
-		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
-		hook_arg1 = "commit";
-		hook_arg2 = use_message;
-	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
-		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
+		strbuf_add(sb, buffer + 2, strlen(buffer + 2));
+		*hook_arg2 = use_message;
+		return "commit";
+	}
+	if (!stat(git_path("MERGE_MSG"), &statbuf)) {
+		if (strbuf_read_file(sb, git_path("MERGE_MSG"), 0) < 0)
 			die_errno("could not read MERGE_MSG");
-		hook_arg1 = "merge";
-	} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
-		if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
+		return "merge";
+	}
+	if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
+		if (strbuf_read_file(sb, git_path("SQUASH_MSG"), 0) < 0)
 			die_errno("could not read SQUASH_MSG");
-		hook_arg1 = "squash";
-	} else if (template_file && !stat(template_file, &statbuf)) {
-		if (strbuf_read_file(&sb, template_file, 0) < 0)
+		return "squash";
+	}
+	if (template_file && !stat(template_file, &statbuf)) {
+		if (strbuf_read_file(sb, template_file, 0) < 0)
 			die_errno("could not read '%s'", template_file);
-		hook_arg1 = "template";
+		return "template";
 	}
 
 	/*
 	 * This final case does not modify the template message,
 	 * it just sets the argument to the prepare-commit-msg hook.
 	 */
-	else if (in_merge)
-		hook_arg1 = "merge";
+	if (in_merge)
+		return "merge";
+
+	return NULL;
+}
+
+
+static int prepare_to_commit(const char *index_file, const char *prefix,
+			     struct wt_status *s)
+{
+	int commitable, saved_color_setting;
+	struct strbuf sb = STRBUF_INIT;
+	FILE *fp;
+	const char *hook_arg1 = NULL;
+	const char *hook_arg2 = NULL;
+	int ident_shown = 0;
+
+	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
+		return 0;
+
+	hook_arg1 = get_template_message(&sb, &hook_arg2);
 
 	fp = fopen(git_path(commit_editmsg), "w");
 	if (fp == NULL)
-- 
1.7.2.9.ge3789.dirty

[PATCH 4/9] commit: split commit -s handling into its own function

[Jonathan Nieder]

prepare_to_commit is easier to read straight through with optional
steps moved out-of-line.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/commit.c |   37 ++++++++++++++++++++-----------------
 1 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 6774180..b599486 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -606,6 +606,24 @@ static const char *get_template_message(struct strbuf *sb,
 	return NULL;
 }
 
+static void add_committer_signoff(struct strbuf *sb)
+{
+	struct strbuf sob = STRBUF_INIT;
+	int i;
+
+	strbuf_addstr(&sob, sign_off_header);
+	strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
+				     getenv("GIT_COMMITTER_EMAIL")));
+	strbuf_addch(&sob, '\n');
+	for (i = sb->len - 1; i > 0 && sb->buf[i - 1] != '\n'; i--)
+		; /* do nothing */
+	if (prefixcmp(sb->buf + i, sob.buf)) {
+		if (!i || !ends_rfc2822_footer(sb))
+			strbuf_addch(sb, '\n');
+		strbuf_addbuf(sb, &sob);
+	}
+	strbuf_release(&sob);
+}
 
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct wt_status *s)
@@ -629,23 +647,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (cleanup_mode != CLEANUP_NONE)
 		stripspace(&sb, 0);
 
-	if (signoff) {
-		struct strbuf sob = STRBUF_INIT;
-		int i;
-
-		strbuf_addstr(&sob, sign_off_header);
-		strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
-					     getenv("GIT_COMMITTER_EMAIL")));
-		strbuf_addch(&sob, '\n');
-		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
-			; /* do nothing */
-		if (prefixcmp(sb.buf + i, sob.buf)) {
-			if (!i || !ends_rfc2822_footer(&sb))
-				strbuf_addch(&sb, '\n');
-			strbuf_addbuf(&sb, &sob);
-		}
-		strbuf_release(&sob);
-	}
+	if (signoff)
+		add_committer_signoff(&sb);
 
 	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
 		die_errno("could not write commit template");
-- 
1.7.2.9.ge3789.dirty

[PATCH 5/9] commit: split off the piece that writes status

[Jonathan Nieder]

The new write_status function takes care of writing status
information about the pending commit (e.g., author name and
whether a merge is pending) to COMMIT_EDITMSG.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/commit.c |  158 +++++++++++++++++++++++++++++-------------------------
 1 files changed, 85 insertions(+), 73 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index b599486..85e560e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -625,15 +625,94 @@ static void add_committer_signoff(struct strbuf *sb)
 	strbuf_release(&sob);
 }
 
+static int write_status(FILE *fp, const char *index_file,
+				const char *prefix, struct wt_status *s)
+{
+	int commitable, saved_color_setting;
+	char *author_ident;
+	const char *committer_ident;
+	int ident_shown = 0;
+
+	if (in_merge)
+		fprintf(fp,
+			"#\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",
+			git_path("MERGE_HEAD"));
+
+	fprintf(fp,
+		"\n"
+		"# Please enter the commit message for your changes.");
+	if (cleanup_mode == CLEANUP_ALL)
+		fprintf(fp,
+			" Lines starting\n"
+			"# with '#' will be ignored, and an empty"
+			" message aborts the commit.\n");
+	else /* CLEANUP_SPACE, that is. */
+		fprintf(fp,
+			" 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);
+
+	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,
+			"%s"
+			"# Author:    %s\n",
+			ident_shown++ ? "" : "#\n",
+			author_ident);
+	free(author_ident);
+
+	if (!user_ident_sufficiently_given())
+		fprintf(fp,
+			"%s"
+			"# Committer: %s\n",
+			ident_shown++ ? "" : "#\n",
+			committer_ident);
+
+	if (ident_shown)
+		fprintf(fp, "#\n");
+
+	saved_color_setting = s->use_color;
+	s->use_color = 0;
+	commitable = run_status(fp, index_file, prefix, 1, s);
+	s->use_color = saved_color_setting;
+	return commitable;
+}
+
+static int something_is_staged(void)
+{
+	unsigned char sha1[20];
+	const char *parent = "HEAD";
+
+	if (!active_nr && read_cache() < 0)
+		die("Cannot read index");
+
+	if (amend)
+		parent = "HEAD^1";
+
+	if (get_sha1(parent, sha1))
+		return !!active_nr;
+	else
+		return index_differs_from(parent, 0);
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct wt_status *s)
 {
-	int commitable, saved_color_setting;
+	int commitable;
 	struct strbuf sb = STRBUF_INIT;
 	FILE *fp;
 	const char *hook_arg1 = NULL;
 	const char *hook_arg2 = NULL;
-	int ident_shown = 0;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -659,77 +738,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	/* This checks if committer ident is explicitly given */
 	git_committer_info(0);
-	if (use_editor && include_status) {
-		char *author_ident;
-		const char *committer_ident;
-
-		if (in_merge)
-			fprintf(fp,
-				"#\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",
-				git_path("MERGE_HEAD"));
-
-		fprintf(fp,
-			"\n"
-			"# Please enter the commit message for your changes.");
-		if (cleanup_mode == CLEANUP_ALL)
-			fprintf(fp,
-				" Lines starting\n"
-				"# with '#' will be ignored, and an empty"
-				" message aborts the commit.\n");
-		else /* CLEANUP_SPACE, that is. */
-			fprintf(fp,
-				" 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);
-
-		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,
-				"%s"
-				"# Author:    %s\n",
-				ident_shown++ ? "" : "#\n",
-				author_ident);
-		free(author_ident);
-
-		if (!user_ident_sufficiently_given())
-			fprintf(fp,
-				"%s"
-				"# Committer: %s\n",
-				ident_shown++ ? "" : "#\n",
-				committer_ident);
-
-		if (ident_shown)
-			fprintf(fp, "#\n");
-
-		saved_color_setting = s->use_color;
-		s->use_color = 0;
-		commitable = run_status(fp, index_file, prefix, 1, s);
-		s->use_color = saved_color_setting;
-	} else {
-		unsigned char sha1[20];
-		const char *parent = "HEAD";
-
-		if (!active_nr && read_cache() < 0)
-			die("Cannot read index");
-
-		if (amend)
-			parent = "HEAD^1";
-
-		if (get_sha1(parent, sha1))
-			commitable = !!active_nr;
-		else
-			commitable = index_differs_from(parent, 0);
-	}
+	if (use_editor && include_status)
+		commitable = write_status(fp, index_file, prefix, s);
+	else
+		commitable = something_is_staged();
 
 	fclose(fp);
 
-- 
1.7.2.9.ge3789.dirty

[PATCH 6/9] t7508 (status): modernize style

[Jonathan Nieder]

Some cleanups to make this test script easier to read and use,
such as:

 - put setup code in test_expect_success scripts to make the
   test script easier to scan through;

 - avoid a pipe to test_decode_color that forgets the exit status of
   "git status", by redirecting to a temporary file (named "actual")
   for that function to consume instead;

 - remove precomputed object IDs;

 - reset configuration after each test, so there is less state
   for a reader to keep in mind.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7508-status.sh | 1373 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 734 insertions(+), 639 deletions(-)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index a72fe3a..882e5d7 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -8,23 +8,23 @@ test_description='git status'
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-	: >tracked &&
-	: >modified &&
+	>tracked &&
+	>modified &&
 	mkdir dir1 &&
-	: >dir1/tracked &&
-	: >dir1/modified &&
+	>dir1/tracked &&
+	>dir1/modified &&
 	mkdir dir2 &&
-	: >dir1/tracked &&
-	: >dir1/modified &&
+	>dir1/tracked &&
+	>dir1/modified &&
 	git add . &&
 
 	git status >output &&
 
 	test_tick &&
 	git commit -m initial &&
-	: >untracked &&
-	: >dir1/untracked &&
-	: >dir2/untracked &&
+	>untracked &&
+	>dir1/untracked &&
+	>dir2/untracked &&
 	echo 1 >dir1/modified &&
 	echo 2 >dir2/modified &&
 	echo 3 >dir2/added &&
@@ -32,564 +32,604 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'status (1)' '
-
 	grep "use \"git rm --cached <file>\.\.\.\" to unstage" output
-
 '
 
-cat >expect <<\EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   dir2/added
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-EOF
-
 test_expect_success 'status (2)' '
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	EOF
 
 	git status >output &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
-# On branch master
-# Changes to be committed:
-#	new file:   dir2/added
-#
-# Changed but not updated:
-#	modified:   dir1/modified
-#
-# Untracked files:
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-EOF
-
-git config advice.statusHints false
-
 test_expect_success 'status (advice.statusHints false)' '
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changes to be committed:
+	#	new file:   dir2/added
+	#
+	# Changed but not updated:
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	EOF
+	git config advice.statusHints false &&
+	test_when_finished "git config --unset advice.statusHints" &&
 
 	git status >output &&
 	test_cmp expect output
-
 '
 
-git config --unset advice.statusHints
-
-cat >expect <<\EOF
- M dir1/modified
-A  dir2/added
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
-
 test_expect_success 'status -s' '
+	cat >expect <<-\EOF &&
+	 M dir1/modified
+	A  dir2/added
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
 
 	git status -s >output &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
-## master
- M dir1/modified
-A  dir2/added
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
-
 test_expect_success 'status -s -b' '
+	cat >expect <<-\EOF &&
+	## master
+	 M dir1/modified
+	A  dir2/added
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
 
 	git status -s -b >output &&
 	test_cmp expect output
-
 '
 
-cat >expect <<EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   dir2/added
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files not listed (use -u option to show untracked files)
-EOF
-test_expect_success 'status -uno' '
+test_expect_success 'set up dir3 for untracked files tests' '
 	mkdir dir3 &&
-	: >dir3/untracked1 &&
-	: >dir3/untracked2 &&
+	>dir3/untracked1 &&
+	>dir3/untracked2 &&
+
+	cat >expect <<-\EOF
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files not listed (use -u option to show untracked files)
+	EOF
+'
+
+test_expect_success 'status -uno' '
 	git status -uno >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'status (status.showUntrackedFiles no)' '
-	git config status.showuntrackedfiles no
+	git config status.showuntrackedfiles no &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
 	git status >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
-# On branch master
-# Changes to be committed:
-#	new file:   dir2/added
-#
-# Changed but not updated:
-#	modified:   dir1/modified
-#
-# Untracked files not listed
-EOF
-git config advice.statusHints false
 test_expect_success 'status -uno (advice.statusHints false)' '
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changes to be committed:
+	#	new file:   dir2/added
+	#
+	# Changed but not updated:
+	#	modified:   dir1/modified
+	#
+	# Untracked files not listed
+	EOF
+	git config status.showuntrackedfiles no &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
+	git config advice.statusHints false &&
+	test_when_finished "git config --unset advice.statusHints" &&
 	git status -uno >output &&
 	test_cmp expect output
 '
-git config --unset advice.statusHints
 
-cat >expect << EOF
- M dir1/modified
-A  dir2/added
-EOF
+test_expect_success 'setup: status -s -uno expected output' '
+	cat >expect <<-\EOF
+	 M dir1/modified
+	A  dir2/added
+	EOF
+'
+
 test_expect_success 'status -s -uno' '
-	git config --unset status.showuntrackedfiles
+	test_might_fail git config --unset status.showuntrackedfiles &&
 	git status -s -uno >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'status -s (status.showUntrackedFiles no)' '
-	git config status.showuntrackedfiles no
+	git config status.showuntrackedfiles no &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
 	git status -s >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   dir2/added
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	dir3/
-#	expect
-#	output
-#	untracked
-EOF
+test_expect_success 'setup: status -unormal expected output' '
+	cat >expect <<-\EOF
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	dir3/
+	#	expect
+	#	output
+	#	untracked
+	EOF
+'
+
 test_expect_success 'status -unormal' '
+	git config status.showuntrackedfiles no &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
 	git status -unormal >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'status (status.showUntrackedFiles normal)' '
-	git config status.showuntrackedfiles normal
+	git config status.showuntrackedfiles normal &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
 	git status >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
- M dir1/modified
-A  dir2/added
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? dir3/
-?? expect
-?? output
-?? untracked
-EOF
+test_expect_success 'setup: status -s -unormal expected output' '
+	cat >expect <<-\EOF
+	 M dir1/modified
+	A  dir2/added
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? dir3/
+	?? expect
+	?? output
+	?? untracked
+	EOF
+'
+
 test_expect_success 'status -s -unormal' '
-	git config --unset status.showuntrackedfiles
+	test_might_fail git config --unset status.showuntrackedfiles &&
 	git status -s -unormal >output &&
 	test_cmp expect output
 '
 
 test_expect_success 'status -s (status.showUntrackedFiles normal)' '
-	git config status.showuntrackedfiles normal
+	git config status.showuntrackedfiles normal &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
 	git status -s >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   dir2/added
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	dir3/untracked1
-#	dir3/untracked2
-#	expect
-#	output
-#	untracked
-EOF
+test_expect_success 'setup: status -uall expected output' '
+	cat >expect <<-\EOF
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	dir3/untracked1
+	#	dir3/untracked2
+	#	expect
+	#	output
+	#	untracked
+	EOF
+'
+
 test_expect_success 'status -uall' '
 	git status -uall >output &&
 	test_cmp expect output
 '
 test_expect_success 'status (status.showUntrackedFiles all)' '
-	git config status.showuntrackedfiles all
+	git config status.showuntrackedfiles all &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
 	git status >output &&
 	rm -rf dir3 &&
-	git config --unset status.showuntrackedfiles &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
- M dir1/modified
-A  dir2/added
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
+test_expect_success 'setup: status -s -uall expected output' '
+	cat >expect <<-\EOF
+	 M dir1/modified
+	A  dir2/added
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
+'
+
 test_expect_success 'status -s -uall' '
-	git config --unset status.showuntrackedfiles
+	test_might_fail git config --unset status.showuntrackedfiles &&
 	git status -s -uall >output &&
 	test_cmp expect output
 '
+
 test_expect_success 'status -s (status.showUntrackedFiles all)' '
-	git config status.showuntrackedfiles all
+	git config status.showuntrackedfiles all &&
+	test_when_finished "git config --unset status.showuntrackedfiles" &&
 	git status -s >output &&
-	rm -rf dir3 &&
-	git config --unset status.showuntrackedfiles &&
 	test_cmp expect output
 '
 
-cat >expect <<\EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   ../dir2/added
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	untracked
-#	../dir2/modified
-#	../dir2/untracked
-#	../expect
-#	../output
-#	../untracked
-EOF
+test_expect_success 'setup: done with dir3' '
+	rm -rf dir3
+'
 
 test_expect_success 'status with relative paths' '
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   ../dir2/added
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	untracked
+	#	../dir2/modified
+	#	../dir2/untracked
+	#	../expect
+	#	../output
+	#	../untracked
+	EOF
 
-	(cd dir1 && git status) >output &&
+	(
+		cd dir1 &&
+		git status >../output
+	) &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
- M modified
-A  ../dir2/added
-?? untracked
-?? ../dir2/modified
-?? ../dir2/untracked
-?? ../expect
-?? ../output
-?? ../untracked
-EOF
 test_expect_success 'status -s with relative paths' '
+	cat >expect <<-\EOF &&
+	 M modified
+	A  ../dir2/added
+	?? untracked
+	?? ../dir2/modified
+	?? ../dir2/untracked
+	?? ../expect
+	?? ../output
+	?? ../untracked
+	EOF
 
-	(cd dir1 && git status -s) >output &&
+	(
+		cd dir1 &&
+		git status -s >../output
+	) &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
- M dir1/modified
-A  dir2/added
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
-
 test_expect_success 'status --porcelain ignores relative paths setting' '
+	cat >expect <<-\EOF &&
+	 M dir1/modified
+	A  dir2/added
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
 
-	(cd dir1 && git status --porcelain) >output &&
+	(
+		cd dir1 &&
+		git status --porcelain >../output
+	) &&
 	test_cmp expect output
-
 '
 
-test_expect_success 'setup unique colors' '
-
+test_expect_success 'setup: unique colors' '
 	git config status.color.untracked blue
-
 '
 
-cat >expect <<\EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	<GREEN>new file:   dir2/added<RESET>
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	<RED>modified:   dir1/modified<RESET>
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	<BLUE>dir1/untracked<RESET>
-#	<BLUE>dir2/modified<RESET>
-#	<BLUE>dir2/untracked<RESET>
-#	<BLUE>expect<RESET>
-#	<BLUE>output<RESET>
-#	<BLUE>untracked<RESET>
-EOF
+test_expect_success 'setup: expect colorful output' '
+	cat >expect <<-\EOF
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	<GREEN>new file:   dir2/added<RESET>
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	<RED>modified:   dir1/modified<RESET>
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	<BLUE>actual<RESET>
+	#	<BLUE>dir1/untracked<RESET>
+	#	<BLUE>dir2/modified<RESET>
+	#	<BLUE>dir2/untracked<RESET>
+	#	<BLUE>expect<RESET>
+	#	<BLUE>output<RESET>
+	#	<BLUE>untracked<RESET>
+	EOF
+'
 
 test_expect_success 'status with color.ui' '
-
 	git config color.ui always &&
-	git status | test_decode_color >output &&
+	test_when_finished "git config --unset color.ui" &&
+	git status >actual &&
+	test_decode_color <actual >output &&
 	test_cmp expect output
-
 '
 
 test_expect_success 'status with color.status' '
-
-	git config --unset color.ui &&
+	test_might_fail git config --unset color.ui &&
 	git config color.status always &&
-	git status | test_decode_color >output &&
+	test_when_finished "git config --unset color.status" &&
+	git status >actual &&
+	test_decode_color <actual >output &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
- <RED>M<RESET> dir1/modified
-<GREEN>A<RESET>  dir2/added
-<BLUE>??<RESET> dir1/untracked
-<BLUE>??<RESET> dir2/modified
-<BLUE>??<RESET> dir2/untracked
-<BLUE>??<RESET> expect
-<BLUE>??<RESET> output
-<BLUE>??<RESET> untracked
-EOF
+test_expect_success 'setup: expected colorful short output' '
+	cat >expect <<-\EOF
+	 <RED>M<RESET> dir1/modified
+	<GREEN>A<RESET>  dir2/added
+	<BLUE>??<RESET> actual
+	<BLUE>??<RESET> dir1/untracked
+	<BLUE>??<RESET> dir2/modified
+	<BLUE>??<RESET> dir2/untracked
+	<BLUE>??<RESET> expect
+	<BLUE>??<RESET> output
+	<BLUE>??<RESET> untracked
+	EOF
+'
 
 test_expect_success 'status -s with color.ui' '
-
-	git config --unset color.status &&
+	test_might_fail git config --unset color.status &&
 	git config color.ui always &&
-	git status -s | test_decode_color >output &&
+	test_when_finished "git config --unset color.ui" &&
+	git status -s >actual &&
+	test_decode_color <actual >output &&
 	test_cmp expect output
-
 '
 
 test_expect_success 'status -s with color.status' '
-
-	git config --unset color.ui &&
+	test_might_fail git config --unset color.ui &&
 	git config color.status always &&
-	git status -s | test_decode_color >output &&
+	test_when_finished "git config --unset color.status" &&
+	git status -s >actual &&
+	test_decode_color <actual >output &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
-## <GREEN>master<RESET>
- <RED>M<RESET> dir1/modified
-<GREEN>A<RESET>  dir2/added
-<BLUE>??<RESET> dir1/untracked
-<BLUE>??<RESET> dir2/modified
-<BLUE>??<RESET> dir2/untracked
-<BLUE>??<RESET> expect
-<BLUE>??<RESET> output
-<BLUE>??<RESET> untracked
-EOF
-
 test_expect_success 'status -s -b with color.status' '
+	cat >expect <<-\EOF &&
+	## <GREEN>master<RESET>
+	 <RED>M<RESET> dir1/modified
+	<GREEN>A<RESET>  dir2/added
+	<BLUE>??<RESET> actual
+	<BLUE>??<RESET> dir1/untracked
+	<BLUE>??<RESET> dir2/modified
+	<BLUE>??<RESET> dir2/untracked
+	<BLUE>??<RESET> expect
+	<BLUE>??<RESET> output
+	<BLUE>??<RESET> untracked
+	EOF
 
-	git status -s -b | test_decode_color >output &&
+	git config color.status always &&
+	test_when_finished "git config --unset color.status" &&
+	git status -s -b >actual &&
+	test_decode_color <actual >output &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
- M dir1/modified
-A  dir2/added
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
+test_expect_success 'setup: expect uncolorful status --porcelain output' '
+	cat >expect <<-\EOF
+	 M dir1/modified
+	A  dir2/added
+	?? actual
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
+'
 
 test_expect_success 'status --porcelain ignores color.ui' '
-
-	git config --unset color.status &&
+	test_might_fail git config --unset color.status &&
 	git config color.ui always &&
-	git status --porcelain | test_decode_color >output &&
+	test_when_finished "git config --unset color.ui" &&
+	git status --porcelain >actual &&
+	test_decode_color <actual >output &&
 	test_cmp expect output
-
 '
 
 test_expect_success 'status --porcelain ignores color.status' '
-
-	git config --unset color.ui &&
+	test_might_fail git config --unset color.ui &&
 	git config color.status always &&
-	git status --porcelain | test_decode_color >output &&
+	test_when_finished "git config --unset color.status" &&
+	git status --porcelain >actual &&
+	test_decode_color <actual >output &&
 	test_cmp expect output
-
 '
 
-# recover unconditionally from color tests
-git config --unset color.status
-git config --unset color.ui
+test_expect_success 'setup: recover unconditionally from color tests' '
+	test_might_fail git config --unset color.status &&
+	test_might_fail git config --unset color.ui
+'
 
 test_expect_success 'status --porcelain ignores -b' '
-
 	git status --porcelain -b >output &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   dir2/added
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-EOF
-
-
 test_expect_success 'status without relative paths' '
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	EOF
 
-	git config status.relativePaths false
-	(cd dir1 && git status) >output &&
+	git config status.relativePaths false &&
+	test_when_finished "git config --unset status.relativePaths" &&
+	(
+		cd dir1 &&
+		git status >../output
+	) &&
 	test_cmp expect output
-
 '
 
-cat >expect <<\EOF
- M dir1/modified
-A  dir2/added
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
-
 test_expect_success 'status -s without relative paths' '
+	cat >expect <<-\EOF &&
+	 M dir1/modified
+	A  dir2/added
+	?? actual
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
 
-	(cd dir1 && git status -s) >output &&
+	git config status.relativePaths false &&
+	test_when_finished "git config --unset status.relativePaths" &&
+	(
+		cd dir1 &&
+		git status -s >../output
+	) &&
 	test_cmp expect output
-
 '
 
-cat <<EOF >expect
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/
-#	expect
-#	output
-#	untracked
-EOF
 test_expect_success 'dry-run of partial commit excluding new file in index' '
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/
+	#	expect
+	#	output
+	#	untracked
+	EOF
+
 	git commit --dry-run dir1/modified >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
-:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0000000000000000000000000000000000000000 M	dir1/modified
-EOF
 test_expect_success 'status refreshes the index' '
+	EMPTY_BLOB=$(git hash-object -t blob --stdin </dev/null) &&
+	ZEROES=0000000000000000000000000000000000000000 &&
+	echo ":100644 100644 $EMPTY_BLOB $ZEROES M	dir1/modified" >expect &&
+
 	touch dir2/added &&
 	git status &&
 	git diff-files >output &&
@@ -597,39 +637,42 @@ test_expect_success 'status refreshes the index' '
 '
 
 test_expect_success 'setup status submodule summary' '
-	test_create_repo sm && (
+	test_create_repo sm &&
+	(
 		cd sm &&
 		>foo &&
 		git add foo &&
 		git commit -m "Add foo"
 	) &&
-	git add sm
+	git add sm &&
+
+	cat >expect <<-\EOF
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#	new file:   sm
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	EOF
 '
 
-cat >expect <<EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   dir2/added
-#	new file:   sm
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-EOF
 test_expect_success 'status submodule summary is disabled by default' '
 	git status >output &&
 	test_cmp expect output
@@ -641,17 +684,21 @@ test_expect_success 'status --untracked-files=all does not show submodule' '
 	test_cmp expect output
 '
 
-cat >expect <<EOF
- M dir1/modified
-A  dir2/added
-A  sm
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
+test_expect_success 'setup status -s submodule summary' '
+	cat >expect <<-\EOF
+	 M dir1/modified
+	A  dir2/added
+	A  sm
+	?? actual
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
+'
+
 test_expect_success 'status -s submodule summary is disabled by default' '
 	git status -s >output &&
 	test_cmp expect output
@@ -663,275 +710,323 @@ test_expect_success 'status -s --untracked-files=all does not show submodule' '
 	test_cmp expect output
 '
 
-head=$(cd sm && git rev-parse --short=7 --verify HEAD)
+test_expect_success 'setup: save head' '
+	head=$(
+		cd sm &&
+		git rev-parse --short=7 --verify HEAD
+	)
+'
 
-cat >expect <<EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD <file>..." to unstage)
-#
-#	new file:   dir2/added
-#	new file:   sm
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Submodule changes to be committed:
-#
-# * sm 0000000...$head (1):
-#   > Add foo
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-EOF
 test_expect_success 'status submodule summary' '
+	cat >expect <<-EOF &&
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#	new file:   sm
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Submodule changes to be committed:
+	#
+	# * sm 0000000...$head (1):
+	#   > Add foo
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	EOF
+
 	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
 	git status >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
- M dir1/modified
-A  dir2/added
-A  sm
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
 test_expect_success 'status -s submodule summary' '
+	cat >expect <<-\EOF &&
+	 M dir1/modified
+	A  dir2/added
+	A  sm
+	?? actual
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
+
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
 	git status -s >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
-# On branch master
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-no changes added to commit (use "git add" and/or "git commit -a")
-EOF
 test_expect_success 'status submodule summary (clean submodule)' '
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	no changes added to commit (use "git add" and/or "git commit -a")
+	EOF
+
 	git commit -m "commit submodule" &&
 	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
 	test_must_fail git commit --dry-run >output &&
 	test_cmp expect output &&
 	git status >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
- M dir1/modified
-?? dir1/untracked
-?? dir2/modified
-?? dir2/untracked
-?? expect
-?? output
-?? untracked
-EOF
 test_expect_success 'status -s submodule summary (clean submodule)' '
+	cat >expect <<-\EOF &&
+	 M dir1/modified
+	?? actual
+	?? dir1/untracked
+	?? dir2/modified
+	?? dir2/untracked
+	?? expect
+	?? output
+	?? untracked
+	EOF
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
 	git status -s >output &&
 	test_cmp expect output
 '
 
-cat >expect <<EOF
-# On branch master
-# Changes to be committed:
-#   (use "git reset HEAD^1 <file>..." to unstage)
-#
-#	new file:   dir2/added
-#	new file:   sm
-#
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Submodule changes to be committed:
-#
-# * sm 0000000...$head (1):
-#   > Add foo
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-EOF
 test_expect_success 'commit --dry-run submodule summary (--amend)' '
+	cat >expect <<-EOF &&
+	# On branch master
+	# Changes to be committed:
+	#   (use "git reset HEAD^1 <file>..." to unstage)
+	#
+	#	new file:   dir2/added
+	#	new file:   sm
+	#
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Submodule changes to be committed:
+	#
+	# * sm 0000000...$head (1):
+	#   > Add foo
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	EOF
+
 	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
 	git commit --dry-run --amend >output &&
 	test_cmp expect output
 '
 
 test_expect_success POSIXPERM 'status succeeds in a read-only repository' '
-	(
-		chmod a-w .git &&
-		# make dir1/tracked stat-dirty
-		>dir1/tracked1 && mv -f dir1/tracked1 dir1/tracked &&
-		git status -s >output &&
-		! grep dir1/tracked output &&
-		# make sure "status" succeeded without writing index out
-		git diff-files | grep dir1/tracked
-	)
-	status=$?
-	chmod 775 .git
-	(exit $status)
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
+
+	chmod a-w .git &&
+	test_when_finished "chmod 775 .git" &&
+
+	# make dir1/tracked stat-dirty
+	>dir1/tracked1 &&
+	mv -f dir1/tracked1 dir1/tracked &&
+
+	git status -s >output &&
+	! grep dir1/tracked output &&
+
+	# make sure "status" succeeded without writing index out
+	git diff-files >output &&
+	grep dir1/tracked output
 '
 
-cat > expect << EOF
-# On branch master
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-no changes added to commit (use "git add" and/or "git commit -a")
-EOF
+test_expect_success 'setup: status --ignore-submodules' '
+	cat >expect <<-\EOF
+	# On branch master
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	no changes added to commit (use "git add" and/or "git commit -a")
+	EOF
+'
 
 test_expect_success '--ignore-submodules=untracked suppresses submodules with untracked content' '
-	echo modified > sm/untracked &&
-	git status --ignore-submodules=untracked > output &&
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
+	echo modified >sm/untracked &&
+	git status --ignore-submodules=untracked >output &&
 	test_cmp expect output
 '
 
 test_expect_success '--ignore-submodules=dirty suppresses submodules with untracked content' '
-	git status --ignore-submodules=dirty > output &&
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
+	git status --ignore-submodules=dirty >output &&
 	test_cmp expect output
 '
 
 test_expect_success '--ignore-submodules=dirty suppresses submodules with modified content' '
-	echo modified > sm/foo &&
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
+	echo modified >sm/foo &&
 	git status --ignore-submodules=dirty > output &&
 	test_cmp expect output
 '
 
-cat > expect << EOF
-# On branch master
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#   (commit or discard the untracked or modified content in submodules)
-#
-#	modified:   dir1/modified
-#	modified:   sm (modified content)
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-no changes added to commit (use "git add" and/or "git commit -a")
-EOF
-
 test_expect_success "--ignore-submodules=untracked doesn't suppress submodules with modified content" '
-	git status --ignore-submodules=untracked > output &&
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#   (commit or discard the untracked or modified content in submodules)
+	#
+	#	modified:   dir1/modified
+	#	modified:   sm (modified content)
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	no changes added to commit (use "git add" and/or "git commit -a")
+	EOF
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
+	git status --ignore-submodules=untracked >output &&
 	test_cmp expect output
 '
 
-head2=$(cd sm && git commit -q -m "2nd commit" foo && git rev-parse --short=7 --verify HEAD)
-
-cat > expect << EOF
-# On branch master
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#	modified:   sm (new commits)
-#
-# Submodules changed but not updated:
-#
-# * sm $head...$head2 (1):
-#   > 2nd commit
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-no changes added to commit (use "git add" and/or "git commit -a")
-EOF
+test_expect_success 'setup' '
+	head2=$(
+		cd sm &&
+		git commit -q -m "2nd commit" foo &&
+		git rev-parse --short=7 --verify HEAD
+	) &&
+	cat >expect <<-EOF
+	# On branch master
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#	modified:   sm (new commits)
+	#
+	# Submodules changed but not updated:
+	#
+	# * sm $head...$head2 (1):
+	#   > 2nd commit
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	no changes added to commit (use "git add" and/or "git commit -a")
+	EOF
+'
 
 test_expect_success "--ignore-submodules=untracked doesn't suppress submodule summary" '
-	git status --ignore-submodules=untracked > output &&
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
+	git status --ignore-submodules=untracked >output &&
 	test_cmp expect output
 '
 
 test_expect_success "--ignore-submodules=dirty doesn't suppress submodule summary" '
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
 	git status --ignore-submodules=dirty > output &&
 	test_cmp expect output
 '
 
-cat > expect << EOF
-# On branch master
-# Changed but not updated:
-#   (use "git add <file>..." to update what will be committed)
-#   (use "git checkout -- <file>..." to discard changes in working directory)
-#
-#	modified:   dir1/modified
-#
-# Untracked files:
-#   (use "git add <file>..." to include in what will be committed)
-#
-#	dir1/untracked
-#	dir2/modified
-#	dir2/untracked
-#	expect
-#	output
-#	untracked
-no changes added to commit (use "git add" and/or "git commit -a")
-EOF
-
 test_expect_success "--ignore-submodules=all suppresses submodule summary" '
-	git status --ignore-submodules=all > output &&
+	cat >expect <<-\EOF &&
+	# On branch master
+	# Changed but not updated:
+	#   (use "git add <file>..." to update what will be committed)
+	#   (use "git checkout -- <file>..." to discard changes in working directory)
+	#
+	#	modified:   dir1/modified
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	actual
+	#	dir1/untracked
+	#	dir2/modified
+	#	dir2/untracked
+	#	expect
+	#	output
+	#	untracked
+	no changes added to commit (use "git add" and/or "git commit -a")
+	EOF
+	git config status.submodulesummary 10 &&
+	test_when_finished "git config --unset status.submodulesummary" &&
+	git status --ignore-submodules=all >output &&
 	test_cmp expect output
 '
 
-- 
1.7.2.9.ge3789.dirty

[PATCH 7/9] commit: give empty-commit avoidance code its own function

[Jonathan Nieder]

With v1.7.1.1~16^2 (commit: give advice on empty amend, 2010-06-06),
"git commit" was taught to provide some extra advice in response to
attempts to amend a commit into emptiness, but "git commit --amend
--dry-run" was not updated to match.  Split out a function that could
be used to carry out such an update if it seems to be a good idea.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/commit.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 85e560e..febefee 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -705,6 +705,18 @@ static int something_is_staged(void)
 		return index_differs_from(parent, 0);
 }
 
+static int empty_commit_ok(const char *index_file, const char *prefix,
+				struct wt_status *s)
+{
+	if (in_merge || allow_empty || (amend && is_a_merge(head_sha1)))
+		return 1;
+
+	run_status(stdout, index_file, prefix, 0, s);
+	if (amend)
+		fputs(empty_amend_advice, stderr);
+	return 0;
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct wt_status *s)
 {
@@ -745,13 +757,13 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	fclose(fp);
 
-	if (!commitable && !in_merge && !allow_empty &&
-	    !(amend && is_a_merge(head_sha1))) {
-		run_status(stdout, index_file, prefix, 0, s);
-		if (amend)
-			fputs(empty_amend_advice, stderr);
+	/*
+	 * If there is nothing staged for commit, this is not a
+	 * merge, and --allow-empty was not supplied, dump status
+	 * followed by some hints for staging changes.
+	 */
+	if (!commitable && !empty_commit_ok(index_file, prefix, s))
 		return 0;
-	}
 
 	/*
 	 * Re-read the index as pre-commit hook could have updated it,
-- 
1.7.2.9.ge3789.dirty

[PATCH 8/9] commit --dry-run: give advice on empty amend

[Jonathan Nieder]

Share code from the non-dry-run case to ensure the output from
"commit --amend --dry-run" matches that from "commit --amend"
on attempts to amend away a commit.  The output had fallen
out of synch in v1.7.1.1~16^2 (2010-06-06).

The only change in output is some extra text to stderr.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/commit.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index febefee..9a4ea34 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1016,7 +1016,14 @@ static int dry_run_commit(int argc, const char **argv, const char *prefix,
 	const char *index_file;
 
 	index_file = prepare_index(argc, argv, prefix, 1);
-	commitable = run_status(stdout, index_file, prefix, 0, s);
+
+	/*
+	 * Give extra advice when faced with attempts to amend away a commit.
+	 */
+	if (!something_is_staged() && !empty_commit_ok(index_file, prefix, s))
+		commitable = 0;
+	else
+		commitable = run_status(stdout, index_file, prefix, 0, s);
 	rollback_index_files();
 
 	return commitable ? 0 : 1;
-- 
1.7.2.9.ge3789.dirty

[PATCH 9/9] commit: suppress status summary when no changes staged

[Jonathan Nieder]

Starting out, it can be unnerving that “git commit” spews out
a list of changes instead of just making a commit when the user
has forgotten to stage any changes.

So give some focused advice in that case, by suppressing the
status summary so the existing message about the need to stage
changes can be read more easily.

Example: before:

	$ git commit
	# On branch master
	# Changed but not updated:
	#   (use "git add <file>..." to update what will be committed)
	#   (use "git checkout -- <file>..." to discard changes in working directory)
	#
	#	modified:   dir1/modified
	#
	# Untracked files:
	#   (use "git add <file>..." to include in what will be committed)
	#
	#	actual
	#	dir1/untracked
	#	dir2/modified
	#	dir2/untracked
	#	expect
	#	output
	#	untracked
	no changes added to commit (use "git add" and/or "git commit -a")
	$

After:

	$ git commit
	no changes added to commit (use "git add" and/or "git commit -a")
	$

Cc: Jakub Narebski <jnareb@gmail.com>
Cc: Jeff King <peff@peff.net>
Cc: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That’s the end of the series.  Thanks for reading.

 builtin/commit.c  |   10 +++++++++-
 t/t7508-status.sh |    7 ++++---
 wt-status.c       |    2 +-
 wt-status.h       |    1 +
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 9a4ea34..a2588a9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -96,7 +96,8 @@ static int null_termination;
 static enum {
 	STATUS_FORMAT_LONG,
 	STATUS_FORMAT_SHORT,
-	STATUS_FORMAT_PORCELAIN
+	STATUS_FORMAT_PORCELAIN,
+	STATUS_FORMAT_NOCHANGES
 } status_format = STATUS_FORMAT_LONG;
 static int status_show_branch;
 
@@ -443,6 +444,9 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	case STATUS_FORMAT_LONG:
 		wt_status_print(s);
 		break;
+	case STATUS_FORMAT_NOCHANGES:
+		wt_status_print_nochanges(s);
+		break;
 	}
 
 	return s->commitable;
@@ -711,6 +715,8 @@ static int empty_commit_ok(const char *index_file, const char *prefix,
 	if (in_merge || allow_empty || (amend && is_a_merge(head_sha1)))
 		return 1;
 
+	if (status_format == STATUS_FORMAT_LONG)
+		status_format = STATUS_FORMAT_NOCHANGES;
 	run_status(stdout, index_file, prefix, 0, s);
 	if (amend)
 		fputs(empty_amend_advice, stderr);
@@ -1170,6 +1176,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		s.ignore_submodule_arg = ignore_submodule_arg;
 		wt_status_print(&s);
 		break;
+	case STATUS_FORMAT_NOCHANGES:
+		return error("unexpected status format");
 	}
 	return 0;
 }
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 882e5d7..c41a54c 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -800,10 +800,11 @@ test_expect_success 'status submodule summary (clean submodule)' '
 	git commit -m "commit submodule" &&
 	git config status.submodulesummary 10 &&
 	test_when_finished "git config --unset status.submodulesummary" &&
-	test_must_fail git commit --dry-run >output &&
+	test_must_fail git commit --dry-run >actual &&
+	git status >output &&
 	test_cmp expect output &&
-	git status >output &&
-	test_cmp expect output
+	echo '\''no changes added to commit (use "git add" and/or "git commit -a")'\'' >expect &&
+	test_cmp expect actual
 '
 
 test_expect_success 'status -s submodule summary (clean submodule)' '
diff --git a/wt-status.c b/wt-status.c
index 90a0824..83d2ae2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -612,7 +612,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	run_diff_index(&rev, 1);
 }
 
-static void wt_status_print_nochanges(struct wt_status *s)
+void wt_status_print_nochanges(struct wt_status *s)
 {
 	if (s->amend)
 		fprintf(s->fp, "# No changes\n");
diff --git a/wt-status.h b/wt-status.h
index 9df9c9f..1cee54b 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -65,5 +65,6 @@ void wt_status_collect(struct wt_status *s);
 
 void wt_shortstatus_print(struct wt_status *s, int null_termination, int show_branch);
 void wt_porcelain_print(struct wt_status *s, int null_termination);
+void wt_status_print_nochanges(struct wt_status *s);
 
 #endif /* STATUS_H */
-- 
1.7.2.9.ge3789.dirty

Re: [PATCH 6/9] t7508 (status): modernize style

[Ævar Arnfjörð Bjarmason]

On Sun, Jul 25, 2010 at 00:59, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Some cleanups to make this test script easier to read and use,
> such as:
>
>  - put setup code in test_expect_success scripts to make the
>   test script easier to scan through;
>
>  - avoid a pipe to test_decode_color that forgets the exit status of
>   "git status", by redirecting to a temporary file (named "actual")
>   for that function to consume instead;
>
>  - remove precomputed object IDs;
>
>  - reset configuration after each test, so there is less state
>   for a reader to keep in mind.

These changes are all good. Well spotted with the test_might_fail()
additions, adding forgotten &&, and turning the >expect code setup
into their own tests.

Acked.

Re: [RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case

[Ævar Arnfjörð Bjarmason]

On Sun, Jul 25, 2010 at 00:54, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> When last seen[1], this series was a single patch in very rough form,
> but there have been almost no functional changes since then.
>
> The patches suppress most output when “git commit” is run without
> stages changed.  So instead of
>
>        $ git commit
>        # On branch master
>        # Changed but not updated:
>        #   (use "git add <file>..." to update what will be committed)
>        #   (use "git checkout -- <file>..." to discard changes in working directory)
>        #
>        #       modified:   dir1/modified
>        #
>        # Untracked files:
>        #   (use "git add <file>..." to include in what will be committed)
>        #
>        #       actual
>        #       dir1/untracked
>        #       dir2/modified
>        #       dir2/untracked
>        #       expect
>        #       output
>        #       untracked
>        no changes added to commit (use "git add" and/or "git commit -a")
>
> which may cause a newcomer to panic, you get
>
>        $ git commit
>        no changes added to commit (use "git add" and/or "git commit -a")
>
> which would just cause her to scratch her head or say “oh, right!”
> instead.  Hopefully these patches will at least provide a reminder to
> improve the various "no changes" advice messages.
>
> Ideas for future work:
>
>  - add some tests
>  - give the full traditional output if -a or any paths were passed on
>   the command line.
>
> Most of the patches are code clarity improvements which is not
> strictly related to this topic.
>
> Patch 6 cleans up the most obvious script to add tests for this in,
> though I have not added any tests to it.
>
> Patch 8 changes commit --dry-run output in a more modest way, to
> print the same advice Jeff added to commit proper last month.  I
> suspect this is a good change, but input from people who script
> around commit --dry-run would be welcome.
>
> Patch 9 is the advertised patch.  It should be self-explanatory.
>
> Thoughts?

Firstly. Acked-by on patches 1-8, they're some much needed
cleanup. Especially fixing the hairy wt-status.c code and the test
fixes.

I'm not so sure about 9/9. Every time I make this mistake with "git
commit" I find it helpful to be able to just look up to see what I
need to stage. But perhaps the wall of text can be confusing to
newbies, I don't have a strong opinion on whether it should be
included or not.

As an aside, isn't this sort of thing (i.e. long notices/help
messages) usually hidden behind advice.* nowadays?

With the stripped down message nothing tells you how to find out what
to add, which the old message did just by including the "git status"
output.

Anyway, meh, I don't know :)

Re: [RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case

[Thomas Rast]

Ævar Arnfjörð Bjarmason wrote:
> On Sun, Jul 25, 2010 at 00:54, Jonathan Nieder <jrnieder@gmail.com> wrote:
> > Hi,
> >
> > When last seen[1], this series was a single patch in very rough form,
> > but there have been almost no functional changes since then.
> >
> > The patches suppress most output when “git commit” is run without
> > stages changed.  So instead of
> >
> >        $ git commit
> >        # On branch master
> >        # Changed but not updated:
[...]
> >        # Untracked files:
[...]
> >        no changes added to commit (use "git add" and/or "git commit -a")
> >
> > which may cause a newcomer to panic, you get
> >
> >        $ git commit
> >        no changes added to commit (use "git add" and/or "git commit -a")
> >
> > which would just cause her to scratch her head or say “oh, right!”
> > instead.  Hopefully these patches will at least provide a reminder to
> > improve the various "no changes" advice messages.
[...]
> I'm not so sure about 9/9. Every time I make this mistake with "git
> commit" I find it helpful to be able to just look up to see what I
> need to stage. But perhaps the wall of text can be confusing to
> newbies, I don't have a strong opinion on whether it should be
> included or not.

I tend to agree with Ævar.  I was trying a different direction
yesterday, it's still WIP but you can try it from here:

  git://repo.or.cz/git/trast.git t/color-porcelain-message-output

It does not have any effect in this case because wt-status.c just
printf()s "no changes added to commit", but the idea would be that it
should end up in the error color (bold red by default) so that it
stands out clearly.

[I actually wrote it because for git-rebase it's even worse: if you
have fixed one conflict and immediately hit another, the output is

  Recorded resolution for 'dir/a'.
  [detached HEAD aa9ae6b] related change                          (1)
   1 files changed, 1 insertions(+), 1 deletions(-)
  Automatic cherry-pick failed.  After resolving the conflicts,
  mark the corrected paths with 'git add <paths>', and
  run 'git rebase --continue'
  Recorded preimage for 'dir/a'
  Could not apply 649420f... second                               (2)

(1) is the subject of the just-applied commit, whereas (2) is the
subject of the now-conflicted commit.  In my case (1) caught my eye
for some reason and I had to look three times to figure out that (2)
was the interesting part.  Now it's in red!]

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Making error messages stand out (Re: [RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case)

[Jonathan Nieder]

Thomas Rast wrote:

>   git://repo.or.cz/git/trast.git t/color-porcelain-message-output

My first thought was "that’s a horrible idea; red text is so hard to
read".  My second thought: "oh, maybe it’s not so bad because it’s
bold".  Now I am starting to worry about the sort of distraction that
can sometimes follow from too much formatting (e.g., with certain
syntax highlighting engines).

A little bold text here and there (maybe to highlight the heading
strings like "fatal" and "hint") would be very useful, certainly.  To
deal with messages like

  Recorded resolution for 'dir/a'.
  [detached HEAD aa9ae6b] related change                          (1)
   1 files changed, 1 insertions(+), 1 deletions(-)
  Automatic cherry-pick failed.  After resolving the conflicts,
  mark the corrected paths with 'git add <paths>', and
  run 'git rebase --continue'
  Recorded preimage for 'dir/a'
  Could not apply 649420f... second                               (2)
 
though, I find the best solution is to use short, formulaic messages:

  ...
  Recorded resolution for 'dir/a'.
  [detached HEAD aa9ae6b] related change
   1 files changed, 1 insertions(+), 1 deletions(-)
  fatal: could not apply 649420f... second
  hint: after resolving the conflicts, mark the corrected paths
  hint: with 'git add <paths>' and run 'git rebase --continue'

I do realize this is not a very useful thing to say without attaching
a patch. ;-)

Re: Making error messages stand out (Re: [RFC/PATCH 0/9] commit: more focused advice in the no-changes-staged case)

[Sverre Rabbelier]

Heya,

On Thu, Jul 29, 2010 at 18:51, Jonathan Nieder <jrnieder@gmail.com> wrote:
> though, I find the best solution is to use short, formulaic messages:
>
>  ...
>  Recorded resolution for 'dir/a'.
>  [detached HEAD aa9ae6b] related change
>   1 files changed, 1 insertions(+), 1 deletions(-)
>  fatal: could not apply 649420f... second
>  hint: after resolving the conflicts, mark the corrected paths
>  hint: with 'git add <paths>' and run 'git rebase --continue'

Yes please. This would be _extremely_ helpful!

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH 9/9] commit: suppress status summary when no changes staged

[Thomas Rast]

Jonathan Nieder wrote:
> Example: before:
>
> 	$ git commit
> 	# On branch master
> 	# Changed but not updated:
[...]
> 	no changes added to commit (use "git add" and/or "git commit -a")
> 	$
>
> After:
>
> 	$ git commit
> 	no changes added to commit (use "git add" and/or "git commit -a")
> 	$

Either Junio just picked this up in the last push or I just never
noticed before, but this breaks t6040 which tests for the "On branch
..." stuff:

  ../trash directory.t6040-tracking-info$ git commit --dry-run
  # On branch follower
  # Your branch is ahead of 'master' by 1 commit.
  #
  nothing to commit (use -u to show untracked files)
  ../trash directory.t6040-tracking-info$ ~/g/git-commit --dry-run
  nothing to commit (use -u to show untracked files)

resulting in

  expecting success:
          (
                  cd test &&
                  git checkout b1 >/dev/null &&
                  # reports nothing to commit
                  test_must_fail git commit --dry-run
          ) >actual &&
          grep "have 1 and 1 different" actual

  Already on 'b1'
  not ok - 5 status

--
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH 9/9] commit: suppress status summary when no changes staged

[Jonathan Nieder]

Thomas Rast wrote:
> Jonathan Nieder wrote:

>> Example: before:
>>
>> 	$ git commit
>> 	# On branch master
>> 	# Changed but not updated:
>[...]
>> 	no changes added to commit (use "git add" and/or "git commit -a")
>> 	$
>>
>> After:
>>
>> 	$ git commit
>> 	no changes added to commit (use "git add" and/or "git commit -a")
>> 	$
>
> Either Junio just picked this up in the last push or I just never
> noticed before, but this breaks t6040 which tests for the "On branch
> ..." stuff

Yep, Ævar noticed the same.  That test is meant to check that
when git commit/status gives status information it reflects the
correct tracking info.  So maybe:

-- 8< --
Subject: t6040 (branch tracking): check “status” instead of “commit”

Among the tests for correct branch tracking output is one that
examines “git commit” output:

 $ git commit
 # Your branch and 'origin/maint' have diverged,
 # and have 9 and 69 different commit(s) each, respectively.
 [...]
 no changes added to commit (use "git add" and/or "git commit -a")
 $

But we are experimenting with changing that output.  So drop
that test for now and replace it with a test for “git status”
(which was not being checked yet and shares the same output
format and wt-status backend).

Reported-by: Thomas Rast <trast@student.ethz.ch>
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 1785e17..3bc91b1 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -64,7 +64,7 @@ test_expect_success 'checkout with local tracked branch' '
 	grep "is ahead of" actual
 '
 
-test_expect_success 'status' '
+test_expect_failure 'status' '
 	(
 		cd test &&
 		git checkout b1 >/dev/null &&
-- 

[PATCH v2] t6040 (branch tracking): check “status” instead of “commit”

[Jonathan Nieder]

Among the tests for correct branch tracking output is one that
examines “git commit” output:

 $ git commit
 # Your branch and 'origin/maint' have diverged,
 # and have 9 and 69 different commit(s) each, respectively.
 [...]
 no changes added to commit (use "git add" and/or "git commit -a")
 $

But we are experimenting with changing that output.  So drop
that test for now and replace it with a test for “git status”
(which was not being checked yet and shares the same output
format and wt-status backend).

Reported-by: Thomas Rast <trast@student.ethz.ch>
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Oops, wrong patch.  Here’s the one I meant.  Sane?

diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 1785e17..a5b4489 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -68,8 +68,7 @@ test_expect_success 'status' '
 	(
 		cd test &&
 		git checkout b1 >/dev/null &&
-		# reports nothing to commit
-		test_must_fail git commit --dry-run
+		git status
 	) >actual &&
 	grep "have 1 and 1 different" actual
 '
-- 

[WIP/PATCH 0/4] Re: Making error messages stand out

[Jonathan Nieder]

Sverre Rabbelier wrote:
> On Thu, Jul 29, 2010 at 18:51, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> though, I find the best solution is to use short, formulaic messages:
>>
>>  ...
>>  Recorded resolution for 'dir/a'.
>>  [detached HEAD aa9ae6b] related change
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>  fatal: could not apply 649420f... second
>>  hint: after resolving the conflicts, mark the corrected paths
>>  hint: with 'git add <paths>' and run 'git rebase --continue'
>
> Yes please. This would be _extremely_ helpful!

Ok. :)

This does not suppress the “Could not apply” message at the end yet.

Patches are against cc/revert.

Jonathan Nieder (4):
  Eliminate “Finished cherry-pick/revert” message
  Introduce advise() to print hints
  cherry-pick: Use error() for failure message
  cherry-pick: Use advise() for hints

 Documentation/howto/revert-branch-rebase.txt |    6 ---
 builtin/revert.c                             |   52 ++++++++++++-------------
 contrib/examples/git-revert.sh               |    1 -
 git-rebase--interactive.sh                   |    6 +-
 t/t3507-cherry-pick-conflict.sh              |   20 ++++++++++
 t/t3508-cherry-pick-many-commits.sh          |   42 +++++++++++++++------
 6 files changed, 78 insertions(+), 49 deletions(-)

[PATCH 1/4] Eliminate “Finished cherry-pick/revert” message

[Jonathan Nieder]

When cherry-pick was written (v0.99.6~63, 2005-08-27), “git commit”
was quiet, and the output from cherry-pick provided useful information
about the progress of a rebase.

Now next to the output from “git commit”, the cherry-pick notification
is so much noise (except for the name of the picked commit).

 $ git cherry-pick ..topic
 Finished cherry-pick of 499088b.
 [detached HEAD 17e1ff2] Move glob module to libdpkg
  Author: Guillem Jover <guillem@debian.org>
  8 files changed, 12 insertions(+), 9 deletions(-)
  rename {src => lib/dpkg}/glob.c (98%)
  rename {src => lib/dpkg}/glob.h (93%)
 Finished cherry-pick of ae947e1.
 [detached HEAD 058caa3] libdpkg: Add missing symbols to Versions script
  Author: Guillem Jover <guillem@debian.org>
  1 files changed, 2 insertions(+), 0 deletions(-)
 $

The noise is especially troublesome when sifting through the output of
a rebase or multiple cherry-pick that eventually failed.

With the commit subject, it is already not hard to figure out where
the commit came from.  So drop the “Finished” message.

Cc: Christian Couder <chriscool@tuxfamily.org>
Cc: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/howto/revert-branch-rebase.txt |    6 ----
 builtin/revert.c                             |    2 -
 contrib/examples/git-revert.sh               |    1 -
 t/t3508-cherry-pick-many-commits.sh          |   42 ++++++++++++++++++-------
 4 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/Documentation/howto/revert-branch-rebase.txt b/Documentation/howto/revert-branch-rebase.txt
index 8c32da6..093c656 100644
--- a/Documentation/howto/revert-branch-rebase.txt
+++ b/Documentation/howto/revert-branch-rebase.txt
@@ -112,25 +112,19 @@ $ git tag pu-anchor pu
 $ git rebase master
 * Applying: Redo "revert" using three-way merge machinery.
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: Remove git-apply-patch-script.
 First trying simple merge strategy to cherry-pick.
 Simple cherry-pick fails; trying Automatic cherry-pick.
 Removing Documentation/git-apply-patch-script.txt
 Removing git-apply-patch-script
-Finished one cherry-pick.
 * Applying: Document "git cherry-pick" and "git revert"
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: mailinfo and applymbox updates
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: Show commits in topo order and name all commits.
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 * Applying: More documentation updates.
 First trying simple merge strategy to cherry-pick.
-Finished one cherry-pick.
 ------------------------------------------------
 
 The temporary tag 'pu-anchor' is me just being careful, in case 'git
diff --git a/builtin/revert.c b/builtin/revert.c
index e261fb2..c3d64af 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -521,8 +521,6 @@ static int do_pick_commit(void)
 	} else {
 		if (!no_commit)
 			res = run_git_commit(defmsg);
-		if (!res)
-			fprintf(stderr, "Finished %s.\n", mebuf.buf);
 	}
 
 	strbuf_release(&mebuf);
diff --git a/contrib/examples/git-revert.sh b/contrib/examples/git-revert.sh
index 49f0032..60a05a8 100755
--- a/contrib/examples/git-revert.sh
+++ b/contrib/examples/git-revert.sh
@@ -181,7 +181,6 @@ Conflicts:
 	esac
 	exit 1
 }
-echo >&2 "Finished one $me."
 
 # If we are cherry-pick, and if the merge did not result in
 # hand-editing, we will hit this commit and inherit the original
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 0f61495..8e09fd0 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -35,36 +35,54 @@ test_expect_success setup '
 '
 
 test_expect_success 'cherry-pick first..fourth works' '
-	cat <<-EOF >expected &&
-	Finished cherry-pick of commit $(git rev-parse --short second).
-	Finished cherry-pick of commit $(git rev-parse --short third).
-	Finished cherry-pick of commit $(git rev-parse --short fourth).
+	cat <<-\EOF >expected &&
+	[master OBJID] second
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	[master OBJID] third
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	[master OBJID] fourth
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
 	EOF
 
 	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
-	git cherry-pick first..fourth 2>actual &&
+	git cherry-pick first..fourth >actual &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
-	test_cmp expected actual &&
+
+	sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
+	test_cmp expected actual.fuzzy &&
 	check_head_differs_from fourth
 '
 
 test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
-	cat <<-EOF >expected &&
-	Finished cherry-pick of commit $(git rev-parse --short second) with strategy resolve.
-	Finished cherry-pick of commit $(git rev-parse --short third) with strategy resolve.
-	Finished cherry-pick of commit $(git rev-parse --short fourth) with strategy resolve.
+	cat <<-\EOF >expected &&
+	Trying simple merge.
+	[master OBJID] second
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	Trying simple merge.
+	[master OBJID] third
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	Trying simple merge.
+	[master OBJID] fourth
+	 Author: A U Thor <author@example.com>
+	 1 files changed, 1 insertions(+), 0 deletions(-)
 	EOF
 
 	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
-	git cherry-pick --strategy resolve first..fourth 2>actual &&
+	git cherry-pick --strategy resolve first..fourth >actual &&
 	git diff --quiet other &&
 	git diff --quiet HEAD other &&
-	test_cmp expected actual &&
+	sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
+	test_cmp expected actual.fuzzy &&
 	check_head_differs_from fourth
 '
 
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 2/4] Introduce advise() to print hints

[Jonathan Nieder]

Like error(), warn(), and die(), advise() prints a short message
with a formulaic prefix to stderr.

It is local to revert.c for now because I am not sure this is
the right API (we may want to take an array of advice lines or a
boolean argument for easy suppression of unwanted advice).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/revert.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index c3d64af..74c1581 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -241,6 +241,15 @@ static void set_author_ident_env(const char *message)
 			sha1_to_hex(commit->object.sha1));
 }
 
+static void advise(const char *advice, ...)
+{
+	va_list params;
+
+	va_start(params, advice);
+	vreportf("hint: ", advice, params);
+	va_end(params);
+}
+
 static char *help_msg(void)
 {
 	struct strbuf helpbuf = STRBUF_INIT;
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 3/4] cherry-pick/revert: Use error() for failure message

[Jonathan Nieder]

When cherry-pick fails after picking a large series of commits, it can
be hard to pick out the error message and advice.  Clarify the error
and prefix it with “error: ” to help.

Before:

	Automatic cherry-pick failed.  [...advice...]

After:

	error: could not apply 7ab78c9... Do something neat.
	[...advice...]

Noticed-by: Thomas Rast <trast@student.ethz.ch>
Encouraged-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/revert.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 74c1581..9a7483b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -524,8 +524,11 @@ static int do_pick_commit(void)
 	}
 
 	if (res) {
-		fprintf(stderr, "Automatic %s failed.%s\n",
-			mebuf.buf, help_msg());
+		error("could not %s %s... %s",
+		      action == REVERT ? "revert" : "apply",
+		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+		      msg.subject);
+		fprintf(stderr, help_msg());
 		rerere(allow_rerere_auto);
 	} else {
 		if (!no_commit)
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 4/4] cherry-pick/revert: Use advise() for hints

[Jonathan Nieder]

When cherry-pick fails after picking a large series of commits, it can
be hard to pick out the error message and advice.  Prefix the advice
with “hint: ” to help.

Before:

    error: could not apply 7ab78c9... foo
      After resolving the conflicts,
    mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
    and commit the result with:

            git commit -c 7ab78c9a7898b87127365478431289cb98f8d98f

After:

    error: could not apply 7ab78c9... foo
    hint: after resolving the conflicts, mark the corrected paths
    hint: with 'git add <paths>' or 'git rm <paths>'
    hint: and commit the result with 'git commit -c 7ab78c9'

Noticed-by: Thomas Rast <trast@student.ethz.ch>
Encouraged-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks for reading.

 builtin/revert.c                |   36 ++++++++++++------------------------
 git-rebase--interactive.sh      |    6 +++---
 t/t3507-cherry-pick-conflict.sh |   20 ++++++++++++++++++++
 3 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 9a7483b..7f35cc6 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -250,27 +250,21 @@ static void advise(const char *advice, ...)
 	va_end(params);
 }
 
-static char *help_msg(void)
+static void print_advice(void)
 {
-	struct strbuf helpbuf = STRBUF_INIT;
 	char *msg = getenv("GIT_CHERRY_PICK_HELP");
 
-	if (msg)
-		return msg;
-
-	strbuf_addstr(&helpbuf, "  After resolving the conflicts,\n"
-		"mark the corrected paths with 'git add <paths>' or 'git rm <paths>'\n"
-		"and commit the result");
-
-	if (action == CHERRY_PICK) {
-		strbuf_addf(&helpbuf, " with: \n"
-			"\n"
-			"        git commit -c %s\n",
-			    sha1_to_hex(commit->object.sha1));
+	if (msg) {
+		fprintf(stderr, "%s\n", msg);
+		return;
 	}
-	else
-		strbuf_addch(&helpbuf, '.');
-	return strbuf_detach(&helpbuf, NULL);
+
+	advise("after resolving the conflicts, mark the corrected paths");
+	advise("with 'git add <paths>' or 'git rm <paths>'");
+
+	if (action == CHERRY_PICK)
+		advise("and commit the result with 'git commit -c %s'",
+		       find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
 }
 
 static void write_message(struct strbuf *msgbuf, const char *filename)
@@ -404,7 +398,6 @@ static int do_pick_commit(void)
 	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
 	char *defmsg = NULL;
 	struct strbuf msgbuf = STRBUF_INIT;
-	struct strbuf mebuf = STRBUF_INIT;
 	int res;
 
 	if (no_commit) {
@@ -501,9 +494,6 @@ static int do_pick_commit(void)
 		}
 	}
 
-	strbuf_addf(&mebuf, "%s of commit %s", me,
-		    find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
-
 	if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
 		res = do_recursive_merge(base, next, base_label, next_label,
 					 head, &msgbuf);
@@ -512,7 +502,6 @@ static int do_pick_commit(void)
 		struct commit_list *common = NULL;
 		struct commit_list *remotes = NULL;
 
-		strbuf_addf(&mebuf, " with strategy %s", strategy);
 		write_message(&msgbuf, defmsg);
 
 		commit_list_insert(base, &common);
@@ -528,14 +517,13 @@ static int do_pick_commit(void)
 		      action == REVERT ? "revert" : "apply",
 		      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
 		      msg.subject);
-		fprintf(stderr, help_msg());
+		print_advice();
 		rerere(allow_rerere_auto);
 	} else {
 		if (!no_commit)
 			res = run_git_commit(defmsg);
 	}
 
-	strbuf_release(&mebuf);
 	free_message(&msg);
 	free(defmsg);
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 31e6860..8f6876d 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -113,9 +113,9 @@ REBASE_ROOT=
 AUTOSQUASH=
 NEVER_FF=
 
-GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
-mark the corrected paths with 'git add <paths>', and
-run 'git rebase --continue'"
+GIT_CHERRY_PICK_HELP="\
+hint: after resolving the conflicts, mark the corrected paths
+hint: with 'git add <paths>' and run 'git rebase --continue'"
 export GIT_CHERRY_PICK_HELP
 
 warn () {
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index e25cf80..3f29594 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -38,6 +38,26 @@ test_expect_success 'failed cherry-pick does not advance HEAD' '
 	test "$head" = "$newhead"
 '
 
+test_expect_success 'advice from failed cherry-pick' '
+	git checkout -f initial^0 &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	picked=$(git rev-parse --short picked) &&
+	cat <<-EOF >expected &&
+	error: could not apply $picked... picked
+	hint: after resolving the conflicts, mark the corrected paths
+	hint: with 'git add <paths>' or 'git rm <paths>'
+	hint: and commit the result with 'git commit -c $picked'
+	EOF
+	test_must_fail git cherry-pick picked 2>actual &&
+
+	test_cmp expected actual
+'
+
 test_expect_success 'failed cherry-pick produces dirty index' '
 
 	git checkout -f initial^0 &&
-- 
1.7.2.1.544.ga752d.dirty

Re: [WIP/PATCH 0/4] Re: Making error messages stand out

[Nguyen Thai Ngoc Duy]

On Wed, Aug 11, 2010 at 6:31 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Sverre Rabbelier wrote:
>> On Thu, Jul 29, 2010 at 18:51, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> though, I find the best solution is to use short, formulaic messages:
>>>
>>>  ...
>>>  Recorded resolution for 'dir/a'.
>>>  [detached HEAD aa9ae6b] related change
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>  fatal: could not apply 649420f... second
>>>  hint: after resolving the conflicts, mark the corrected paths
>>>  hint: with 'git add <paths>' and run 'git rebase --continue'
>>
>> Yes please. This would be _extremely_ helpful!
>
> Ok. :)
>
> This does not suppress the “Could not apply” message at the end yet.

Even better, make it available for some time with, say "git hints".
After doing lots of things to resolve conflicts, I simply forget what
it hinted me.
-- 
Duy

Re: [WIP/PATCH 0/4] Re: Making error messages stand out

[Matthieu Moy]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Even better, make it available for some time with, say "git hints".
> After doing lots of things to resolve conflicts, I simply forget what
> it hinted me.

Actually, I don't think we should add a new command for that, but add
something to "git status", like

$ git status
# On branch master
# rebase in progress (use "git rebase --continue" to proceed)
# ...

Same would apply to conflicting merges. It's very common for beginners
(especially when they come from SVN) to start a merge, mis-read the
message telling you to commit once you fixed the conflicts, fix
conflicts, and continue hacking. A message in "git status" when
.git/MERGE_HEAD exists would help a bit, like (depending on whether
the index still has conflicts):

# merge in progress (fix conflicts below and commit)

# merge in progress (use "git commit" to proceed)


If one adds an option to make it all-caps, red, and blinking, then
I'll activate it for my students ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [WIP/PATCH 0/4] Re: Making error messages stand out

[Nguyen Thai Ngoc Duy]

On Wed, Aug 11, 2010 at 7:39 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> Even better, make it available for some time with, say "git hints".
>> After doing lots of things to resolve conflicts, I simply forget what
>> it hinted me.
>
> Actually, I don't think we should add a new command for that, but add
> something to "git status", like
>
> $ git status
> # On branch master
> # rebase in progress (use "git rebase --continue" to proceed)
> # ...
>
> Same would apply to conflicting merges. It's very common for beginners
> (especially when they come from SVN) to start a merge, mis-read the
> message telling you to commit once you fixed the conflicts, fix
> conflicts, and continue hacking. A message in "git status" when
> .git/MERGE_HEAD exists would help a bit, like (depending on whether
> the index still has conflicts):
>
> # merge in progress (fix conflicts below and commit)
>
> # merge in progress (use "git commit" to proceed)

Yes. Looks good. Except that I rarely use git-status these days. Maybe
that will motivate me to use that command more.

> If one adds an option to make it all-caps, red, and blinking, then
> I'll activate it for my students ;-).

I'd suggest you patch git-status with figlet to get some more attention :-)
-- 
Duy

Re: [PATCH 9/9] commit: suppress status summary when no changes staged

[Ævar Arnfjörð Bjarmason]

On Wed, Aug 11, 2010 at 07:30, Jonathan Nieder <jrnieder@gmail.com> wrote:

> -test_expect_success 'status' '
> +test_expect_failure 'status' '
>        (
>                cd test &&
>                git checkout b1 >/dev/null &&

Better to test_expect_success like my patch does and explicitly check
the output, otherwise that test will pass if any part of it fails,
e.g. if the checkout fails.

Not likely, but it's more likely that the output will change again, in
which case the grep tests I did would start failing again.

It's good to have test canaries like that for important parts of our
output.

Re: [WIP/PATCH 0/4] Re: Making error messages stand out

[Sverre Rabbelier]

Heya,

On Wed, Aug 11, 2010 at 03:31, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Jonathan Nieder (4):
>  Eliminate “Finished cherry-pick/revert” message
>  Introduce advise() to print hints
>  cherry-pick: Use error() for failure message
>  cherry-pick: Use advise() for hints

Nice :).

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH 9/9] commit: suppress status summary when no changes staged

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:
> On Wed, Aug 11, 2010 at 07:30, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> -test_expect_success 'status' '
>> +test_expect_failure 'status' '

Oops.  Did you see the follow-up patch?

> Better to test_expect_success like my patch does and explicitly check
> the output, otherwise that test will pass if any part of it fails,
> e.g. if the checkout fails.
>
> Not likely, but it's more likely that the output will change again, in
> which case the grep tests I did would start failing again.

The wt-status output series ought have included a separate test for
the new “git commit --dry-run” output.  But this is not what that test
script is about, and I think including it there would have been
confusing.

Sorry for the breakage, and thanks for reporting it.

Re: [PATCH 9/9] commit: suppress status summary when no changes staged

[Ævar Arnfjörð Bjarmason]

On Wed, Aug 11, 2010 at 23:57, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Aug 11, 2010 at 07:30, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> -test_expect_success 'status' '
>>> +test_expect_failure 'status' '
>
> Oops.  Did you see the follow-up patch?

No, I'm probably missing something. Move along now, nothing to see
here :)

>> Better to test_expect_success like my patch does and explicitly check
>> the output, otherwise that test will pass if any part of it fails,
>> e.g. if the checkout fails.
>>
>> Not likely, but it's more likely that the output will change again, in
>> which case the grep tests I did would start failing again.
>
> The wt-status output series ought have included a separate test for
> the new “git commit --dry-run” output.  But this is not what that test
> script is about, and I think including it there would have been
> confusing.

Sounds like you got this covered, that's good enough for me.

> Sorry for the breakage, and thanks for reporting it.

No problem.

Re: [PATCH 9/9] commit: suppress status summary when no changes staged

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:
> On Wed, Aug 11, 2010 at 23:57, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> Did you see the follow-up patch?
>
> No, I'm probably missing something. Move along now, nothing to see
> here :)

I figured it out.  Gmail’s implementation of threading is (deliberately)
broken.

The follow-up has subject

  [PATCH v2] t6040 (branch tracking): check “status” instead of “commit”

> Sounds like you got this covered

Thanks again for a quick report and patch --- they were helpful
indeed.

Re: [PATCH v2] t6040 (branch tracking): check “status” instead of “commit”

[Ævar Arnfjörð Bjarmason]

On Wed, Aug 11, 2010 at 07:49, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Among the tests for correct branch tracking output is one that
> examines “git commit” output:
>
>  $ git commit
>  # Your branch and 'origin/maint' have diverged,
>  # and have 9 and 69 different commit(s) each, respectively.
>  [...]
>  no changes added to commit (use "git add" and/or "git commit -a")
>  $
>
> But we are experimenting with changing that output.  So drop
> that test for now and replace it with a test for “git status”
> (which was not being checked yet and shares the same output
> format and wt-status backend).
>
> Reported-by: Thomas Rast <trast@student.ethz.ch>
> Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  Oops, wrong patch.  Here’s the one I meant.  Sane?
>
> diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
> index 1785e17..a5b4489 100755
> --- a/t/t6040-tracking-info.sh
> +++ b/t/t6040-tracking-info.sh
> @@ -68,8 +68,7 @@ test_expect_success 'status' '
>        (
>                cd test &&
>                git checkout b1 >/dev/null &&
> -               # reports nothing to commit
> -               test_must_fail git commit --dry-run
> +               git status
>        ) >actual &&
>        grep "have 1 and 1 different" actual
>  '

This looks good. My patch should be dropped in favor of this. It looks
like the extra testing I did is covered by the "git commit --dry-run"
test in t7508-status.sh in your original patch.

[PATCH] tests: fix syntax error in "Use advise() for hints" test

[Ævar Arnfjörð Bjarmason]

Change the test introduced in the "Use advise() for hints" patch by
Jonathan Nieder not to use '' for quotes inside '' delimited code. It
ended up introducing a file called <paths> to the main git repository.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3507-cherry-pick-conflict.sh |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 3f29594..607bf25 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -38,7 +38,7 @@ test_expect_success 'failed cherry-pick does not advance HEAD' '
 	test "$head" = "$newhead"
 '
 
-test_expect_success 'advice from failed cherry-pick' '
+test_expect_success 'advice from failed cherry-pick' "
 	git checkout -f initial^0 &&
 	git read-tree -u --reset HEAD &&
 	git clean -d -f -f -q -x &&
@@ -46,17 +46,17 @@ test_expect_success 'advice from failed cherry-pick' '
 	git update-index --refresh &&
 	git diff-index --exit-code HEAD &&
 
-	picked=$(git rev-parse --short picked) &&
+	picked=\$(git rev-parse --short picked) &&
 	cat <<-EOF >expected &&
-	error: could not apply $picked... picked
+	error: could not apply \$picked... picked
 	hint: after resolving the conflicts, mark the corrected paths
 	hint: with 'git add <paths>' or 'git rm <paths>'
-	hint: and commit the result with 'git commit -c $picked'
+	hint: and commit the result with 'git commit -c \$picked'
 	EOF
 	test_must_fail git cherry-pick picked 2>actual &&
 
 	test_cmp expected actual
-'
+"
 
 test_expect_success 'failed cherry-pick produces dirty index' '
 
-- 
1.7.2.1.414.g9bf49

Re: [PATCH] tests: fix syntax error in "Use advise() for hints" test

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> Change the test introduced in the "Use advise() for hints" patch by
> Jonathan Nieder not to use '' for quotes inside '' delimited code.

Yikes.  Thanks for catching this.

> -test_expect_success 'advice from failed cherry-pick' '
> +test_expect_success 'advice from failed cherry-pick' "
[...]
>  	cat <<-EOF >expected &&
> -	error: could not apply $picked... picked
> +	error: could not apply \$picked... picked

Although the style you chose is arguably the least ugly, nested shell
interpolation can be hard to follow.  How about this?

-- 8< --
Subject: t3507 (cherry-pick): escape quotes in "Use advise()" test

Do not use unescaped '' quotes inside ''-delimited code.  Otherwise,
this test tries to read a file named "paths" in the test repository,
resulting in the error:

 t3507-cherry-pick-conflict.sh: 59: cannot open paths: No such file

Based-on-patch-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t3507-cherry-pick-conflict.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 3f29594..6026e7e 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -50,8 +50,8 @@ test_expect_success 'advice from failed cherry-pick' '
 	cat <<-EOF >expected &&
 	error: could not apply $picked... picked
 	hint: after resolving the conflicts, mark the corrected paths
-	hint: with 'git add <paths>' or 'git rm <paths>'
-	hint: and commit the result with 'git commit -c $picked'
+	hint: with '\''git add <paths>'\'' or '\''git rm <paths>'\''
+	hint: and commit the result with '\''git commit -c $picked'\''
 	EOF
 	test_must_fail git cherry-pick picked 2>actual &&
 
-- 
1.7.2.1.544.ga752d.dirty

Re: [PATCH] tests: fix syntax error in "Use advise() for hints" test

[Ævar Arnfjörð Bjarmason]

On Thu, Aug 19, 2010 at 04:30, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> Change the test introduced in the "Use advise() for hints" patch by
>> Jonathan Nieder not to use '' for quotes inside '' delimited code.
>
> Yikes.  Thanks for catching this.

FWIW prove flagged it:

    $ prove ./t35*.sh
    ./t3500-cherry.sh .................... ok
    ./t3501-revert-cherry-pick.sh ........ ok
    ./t3502-cherry-pick-merge.sh ......... ok
    ./t3503-cherry-pick-root.sh .......... ok
    ./t3504-cherry-pick-rerere.sh ........ ok
    ./t3505-cherry-pick-empty.sh ......... ok
    ./t3506-cherry-pick-ff.sh ............ ok
    ./t3507-cherry-pick-conflict.sh ...... 1/?
./t3507-cherry-pick-conflict.sh: 59: cannot open paths: No such file
    ./t3507-cherry-pick-conflict.sh ...... ok
    ./t3508-cherry-pick-many-commits.sh .. ok
    ./t3509-cherry-pick-merge-df.sh ...... ok
    All tests successful.
    Files=10, Tests=62,  4 wallclock secs ( 0.08 usr  0.06 sys +  0.65
cusr  2.91 csys =  3.70 CPU)
    Result: PASS

> Although the style you chose is arguably the least ugly, nested shell
> interpolation can be hard to follow.  How about this?

I think '\'' is harder to follow than \" and \$, but each to his own
:)

Re: [PATCH] tests: fix syntax error in "Use advise() for hints" test

[Raja R Harinath]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Thu, Aug 19, 2010 at 04:30, Jonathan Nieder <jrnieder@gmail.com> wrote:
[snip]
>> Although the style you chose is arguably the least ugly, nested shell
>> interpolation can be hard to follow.  How about this?
>
> I think '\'' is harder to follow than \" and \$, but each to his own
> :)

There's also the slightly longer but somewhat prettier '"'"'.

- Hari

Re: [PATCH] tests: fix syntax error in "Use advise() for hints" test

[Ævar Arnfjörð Bjarmason]

On Fri, Aug 20, 2010 at 10:13, Raja R Harinath <harinath@hurrynot.org> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Thu, Aug 19, 2010 at 04:30, Jonathan Nieder <jrnieder@gmail.com> wrote:
> [snip]
>>> Although the style you chose is arguably the least ugly, nested shell
>>> interpolation can be hard to follow.  How about this?
>>
>> I think '\'' is harder to follow than \" and \$, but each to his own
>> :)
>
> There's also the slightly longer but somewhat prettier '"'"'.

I must say, you guys have an odd sense of aesthetics :)

Re: [PATCH] tests: fix syntax error in "Use advise() for hints" test

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Fri, Aug 20, 2010 at 10:13, Raja R Harinath <harinath@hurrynot.org> wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> On Thu, Aug 19, 2010 at 04:30, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> [snip]
>>>> Although the style you chose is arguably the least ugly, nested shell
>>>> interpolation can be hard to follow.  How about this?
>>>
>>> I think '\'' is harder to follow than \" and \$, but each to his own
>>> :)
>>
>> There's also the slightly longer but somewhat prettier '"'"'.
>
> I must say, you guys have an odd sense of aesthetics :)

I'd have to agree.  If I were writing this I would probably use '\''
myself but that is not because it looks good (it does not) but in my
experience it tends to be the least error prone.

But your original is just fine.