builtin-rm.c: explain and clarify the "local change" logic

Explain the logic to check local modification a bit more in the comment,
especially because the existing comment that talks about "git rm --cached"
was placed in a part that was not about "--cached" at all.

Also clarify "if .. else if .." structure.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-rm.c b/builtin-rm.c
index b7126e3e25d4f52ae8c4a120d524a24c11129785..3d03da09df0b2831dd5a6bcbc09e75db356260a1 100644 (file)
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -31,7 +31,8 @@ static void add_list(const char *name)
 
 static int check_local_mod(unsigned char *head, int index_only)
 {
-       /* items in list are already sorted in the cache order,
+       /*
+        * Items in list are already sorted in the cache order,
         * so we could do this a lot more efficiently by using
         * tree_desc based traversal if we wanted to, but I am
         * lazy, and who cares if removal of files is a tad
@@ -71,25 +72,55 @@ static int check_local_mod(unsigned char *head, int index_only)
                         */
                        continue;
                }
+
+               /*
+                * "rm" of a path that has changes need to be treated
+                * carefully not to allow losing local changes
+                * accidentally.  A local change could be (1) file in
+                * work tree is different since the index; and/or (2)
+                * the user staged a content that is different from
+                * the current commit in the index.
+                *
+                * In such a case, you would need to --force the
+                * removal.  However, "rm --cached" (remove only from
+                * the index) is safe if the index matches the file in
+                * the work tree or the HEAD commit, as it means that
+                * the content being removed is available elsewhere.
+                */
+
+               /*
+                * Is the index different from the file in the work tree?
+                */
                if (ce_match_stat(ce, &st, 0))
                        local_changes = 1;
+
+               /*
+                * Is the index different from the HEAD commit?  By
+                * definition, before the very initial commit,
+                * anything staged in the index is treated by the same
+                * way as changed from the HEAD.
+                */
                if (no_head
                     || get_tree_entry(head, name, sha1, &mode)
                     || ce->ce_mode != create_ce_mode(mode)
                     || hashcmp(ce->sha1, sha1))
                        staged_changes = 1;
 
-               if (local_changes && staged_changes &&
-                   !(index_only && is_empty_blob_sha1(ce->sha1)))
-                       errs = error("'%s' has staged content different "
-                                    "from both the file and the HEAD\n"
-                                    "(use -f to force removal)", name);
+               /*
+                * If the index does not match the file in the work
+                * tree and if it does not match the HEAD commit
+                * either, (1) "git rm" without --cached definitely
+                * will lose information; (2) "git rm --cached" will
+                * lose information unless it is about removing an
+                * "intent to add" entry.
+                */
+               if (local_changes && staged_changes) {
+                       if (!index_only || !is_empty_blob_sha1(ce->sha1))
+                               errs = error("'%s' has staged content different "
+                                            "from both the file and the HEAD\n"
+                                            "(use -f to force removal)", name);
+               }
                else if (!index_only) {
-                       /* It's not dangerous to "git rm --cached" a
-                        * file if the index matches the file or the
-                        * HEAD, since it means the deleted content is
-                        * still available somewhere.
-                        */
                        if (staged_changes)
                                errs = error("'%s' has changes staged in the index\n"
                                             "(use --cached to keep the file, "