Re: Please default to 'commit -a' when no changes were added
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:48:41
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: Please default to 'commit -a' when no changes were added · Goswin von Brederlow <hidden>
- 2016-06-15 · Re: Please default to 'commit -a' when no changes were added · Jonathan Nieder <hidden>
- 2016-06-15 · Re: Please default to 'commit -a' when no changes were added · Goswin von Brederlow <hidden>
- 2016-06-15 · Re: Please default to 'commit -a' when no changes were added · Jonathan Nieder <hidden>
- 2016-06-15 · Re: Please default to 'commit -a' when no changes were added · Miles Bader <hidden>
[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);