Re: Please default to 'commit -a' when no changes were added

[Jonathan Nieder <jrnieder@gmail.com>]

[topic: making ‘git commit’ more helpful when there are no changes
registered in the index]

Hi Goswin,

Goswin von Brederlow wrote:

> in most (all but git?) RCS a plain 'commit' without any arguments
> commits all changes (to registered files).

Yes, but they are wrong. :)

> no changes added to commit (use "git add" and/or "git commit -a")
[...]
> Imho in most cases where no changes
> were added people do want to commit all modified files. And if not
> then exiting the editor to abort is easy enough.

I absent-mindedly type ‘git commit’ having forgotten to update the
index with my changes fairly often.  Then I add the appropriate
changes, which is almost never all of them.  I don’t think this is so
unusual.

Starting out, I can see how it would be comforting to people if
‘git commit’ would default to -a behavior if they ignore the index.
That is logically a different operation, though, so it would also send
a wrong message and make it harder in the long run to get used to the
interface.

Instead, I think it would be better to focus on making the error
message more helpful.  Right now there is a screen full of status
before the advice, which might make it easy to get scared before
reading it.

Here’s a very rough patch to suppress that screenful.  What do you
think?

diff --git a/builtin/commit.c b/builtin/commit.c
index c5ab683..9cb5489 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -396,7 +396,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
 }
 
 static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,
-		      struct wt_status *s)
+		      struct wt_status *s, int simple)
 {
 	unsigned char sha1[20];
 
@@ -415,6 +415,13 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 
 	wt_status_collect(s);
 
+	if (simple) {
+		if (s->commitable)
+			die("internal error: are there changes or not?");
+		wt_status_print_nochanges(s);
+		return 0;
+	}
+
 	switch (status_format) {
 	case STATUS_FORMAT_SHORT:
 		wt_shortstatus_print(s, null_termination);
@@ -670,7 +677,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 		saved_color_setting = s->use_color;
 		s->use_color = 0;
-		commitable = run_status(fp, index_file, prefix, 1, s);
+		commitable = run_status(fp, index_file, prefix, 1, s, 0);
 		s->use_color = saved_color_setting;
 	} else {
 		unsigned char sha1[20];
@@ -692,7 +699,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	if (!commitable && !in_merge && !allow_empty &&
 	    !(amend && is_a_merge(head_sha1))) {
-		run_status(stdout, index_file, prefix, 0, s);
+		run_status(stdout, index_file, prefix, 0, s, 1);
 		return 0;
 	}
 
@@ -946,7 +953,7 @@ 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);
+	commitable = run_status(stdout, index_file, prefix, 0, s, 0);
 	rollback_index_files();
 
 	return commitable ? 0 : 1;
diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..b50bf71 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -589,6 +589,24 @@ static void wt_status_print_tracking(struct wt_status *s)
 	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
 }
 
+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 (use \"git add\" and/or \"git commit -a\")\n");
+	else if (s->untracked.nr)
+		printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
+	else if (s->is_initial)
+		printf("nothing to commit (create/copy files and use \"git add\" to track)\n");
+	else if (!s->show_untracked_files)
+		printf("nothing to commit (use -u to show untracked files)\n");
+	else
+		printf("nothing to commit (working directory clean)\n");
+}
+
 void wt_status_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_HEADER, s);
@@ -629,22 +647,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 (use \"git add\" and/or \"git commit -a\")\n");
-		else if (s->untracked.nr)
-			printf("nothing added to commit but untracked files present (use \"git add\" to track)\n");
-		else if (s->is_initial)
-			printf("nothing to commit (create/copy files and use \"git add\" to track)\n");
-		else if (!s->show_untracked_files)
-			printf("nothing to commit (use -u to show untracked files)\n");
-		else
-			printf("nothing to commit (working directory clean)\n");
-	}
+	if (!s->commitable)
+		wt_status_print_nochanges(s);
 }
 
 static void wt_shortstatus_unmerged(int null_termination, struct string_list_item *it,
diff --git a/wt-status.h b/wt-status.h
index 9120673..f249955 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -59,6 +59,8 @@ void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
 
+void wt_status_print_nochanges(struct wt_status *s);
+
 void wt_shortstatus_print(struct wt_status *s, int null_termination);
 void wt_porcelain_print(struct wt_status *s, int null_termination);