[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);
 



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to