{"thread":{"id":"51363","subject":"[PATCH] rm: add --intent-to-add, to be used with --cached","startedAt":"2019-06-22T12:24:32Z","lastAt":"2019-06-24T15:02:51Z","messageCount":3,"participants":["Nguyễn Thái Ngọc Duy","Johannes Schindelin","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"377814","messageId":"20190622122417.28178-1-pclouds@gmail.com","threadId":"51363","inReplyTo":null,"subject":"[PATCH] rm: add --intent-to-add, to be used with --cached","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-06-22T12:24:17Z","receivedAt":"2019-06-22T12:24:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"An index entry serves two purposes: to keep the content to be committed,\nand to mark that the same path on worktree is tracked. When\n\n    git rm --cached foo\n\nis called and there is \"foo\" in worktree, its status is changed from\ntracked to untracked. Which I think is not intended, at least from the\nuser perspective because we almost always tell people \"Git is about the\ncontent\" (*).\n\nAdd --intent-to-add, which will replace the current index entry with an\nintent-to-add one. \"git commit -m gone\" will record \"foo\" gone, while\n\"git commit -am not-gone\" will ignore the index as usual and keep \"foo\".\nBefore this, \"commit -am\" will also remove \"foo\".\n\nWhile I think --intent-to-add (and also the one in git-reset) should be\nthe default behavior, changing it flip the test suite out because it\nrelies on the current behavior. Let's leave that for later. At least\nhaving the ability to just remove the staged content is the right thing\nto do.\n\n(*) From the developer perspective, keeping the tool dumb actually\n    sounds good. When you tell git to remove something from the index,\n    it should go do just that, no trying to be clever. But that's more\n    suitable for plumbing commands like update-index than rm, in my\n    opinion.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n This occurred to me while adding intent-to-add support to git-restore.\n It's not related to nd/switch-and-restore though and I originally\n wanted to make it default, so I post it separately here.\n\n Documentation/git-rm.txt |  6 ++++++\n builtin/rm.c             | 10 ++++++++--\n t/t3600-rm.sh            |  9 +++++++++\n 3 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex b5c46223c4..aa0aa6063f 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -60,6 +60,12 @@ OPTIONS\n \tWorking tree files, whether modified or not, will be\n \tleft alone.\n \n+--intent-to-add::\n+--no-intent-to-add::\n+\tWhen `--cached` is used, if the given path also exist as a\n+\tworking tree file, the index is updated to record the fact\n+\tthat the path will be added later, similar to `git add -N`.\n+\n --ignore-unmatch::\n \tExit with a zero status even if no files matched.\n \ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex be8edc6d1e..135bc4b76e 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -235,12 +235,14 @@ static int check_local_mod(struct object_id *head, int index_only)\n }\n \n static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;\n-static int ignore_unmatch = 0;\n+static int ignore_unmatch = 0, intent_to_add = 0;\n \n static struct option builtin_rm_options[] = {\n \tOPT__DRY_RUN(&show_only, N_(\"dry run\")),\n \tOPT__QUIET(&quiet, N_(\"do not list removed files\")),\n \tOPT_BOOL( 0 , \"cached\",         &index_only, N_(\"only remove from the index\")),\n+\tOPT_BOOL( 0 , \"intent-to-add\",  &intent_to_add,\n+\t\t  N_(\"record that the path will be added later if needed\")),\n \tOPT__FORCE(&force, N_(\"override the up-to-date check\"), PARSE_OPT_NOCOMPLETE),\n \tOPT_BOOL('r', NULL,             &recursive,  N_(\"allow recursive removal\")),\n \tOPT_BOOL( 0 , \"ignore-unmatch\", &ignore_unmatch,\n@@ -262,7 +264,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tif (!argc)\n \t\tusage_with_options(builtin_rm_usage, builtin_rm_options);\n \n-\tif (!index_only)\n+\tif (!index_only || intent_to_add)\n \t\tsetup_work_tree();\n \n \thold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);\n@@ -344,6 +346,10 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \n \t\tif (remove_file_from_cache(path))\n \t\t\tdie(_(\"git rm: unable to remove %s\"), path);\n+\n+\t\tif (index_only && intent_to_add && file_exists(path) &&\n+\t\t    add_file_to_index(the_repository->index, path, ADD_CACHE_INTENT))\n+\t\t\terror(_(\"failed to mark '%s' as intent-to-add\"), path);\n \t}\n \n \tif (show_only)\ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 85ae7dc1e4..04233b7dc4 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -34,6 +34,15 @@ test_expect_success 'Test that git rm foo succeeds' '\n \tgit rm --cached foo\n '\n \n+test_expect_success 'Test that git rm --cached --intent-to-add foo succeeds' '\n+\techo content >foo &&\n+\tgit add foo &&\n+\tgit rm --cached --intent-to-add foo &&\n+\tgit diff --summary -- foo >actual &&\n+\techo \" create mode 100644 foo\" >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Test that git rm --cached foo succeeds if the index matches the file' '\n \techo content >foo &&\n \tgit add foo &&\n-- \n2.22.0.rc0.322.g2b0371e29a\n\n"},{"id":"377860","messageId":"nycvar.QRO.7.76.6.1906241251040.44@tvgsbejvaqbjf.bet","threadId":"51363","inReplyTo":"20190622122417.28178-1-pclouds@gmail.com","subject":"Re: [PATCH] rm: add --intent-to-add, to be used with --cached","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-06-24T10:52:47Z","receivedAt":"2019-06-24T10:52:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Sat, 22 Jun 2019, Nguyễn Thái Ngọc Duy wrote:\n\n> An index entry serves two purposes: to keep the content to be committed,\n> and to mark that the same path on worktree is tracked. When\n>\n>     git rm --cached foo\n>\n> is called and there is \"foo\" in worktree, its status is changed from\n> tracked to untracked. Which I think is not intended, at least from the\n> user perspective because we almost always tell people \"Git is about the\n> content\" (*).\n\nI can buy that rationale. However, I do not think that \"remove intent to\nadd\" (which is how I read `git rm --intent-to-add`) is a particularly good\nway to express this. I could see `--keep-intent-to-add` as a better\nalternative, though.\n\nCiao,\nJohannes\n"},{"id":"377892","messageId":"CACsJy8DOAWw7TSi1jPqSNV-41h8R6nUAjVC+osTjaDpmxnPa-g@mail.gmail.com","threadId":"51363","inReplyTo":"nycvar.QRO.7.76.6.1906241251040.44@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] rm: add --intent-to-add, to be used with --cached","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-06-24T15:02:21Z","receivedAt":"2019-06-24T15:02:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jun 24, 2019 at 5:52 PM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> Hi Duy,\n>\n> On Sat, 22 Jun 2019, Nguyễn Thái Ngọc Duy wrote:\n>\n> > An index entry serves two purposes: to keep the content to be committed,\n> > and to mark that the same path on worktree is tracked. When\n> >\n> >     git rm --cached foo\n> >\n> > is called and there is \"foo\" in worktree, its status is changed from\n> > tracked to untracked. Which I think is not intended, at least from the\n> > user perspective because we almost always tell people \"Git is about the\n> > content\" (*).\n>\n> I can buy that rationale. However, I do not think that \"remove intent to\n> add\" (which is how I read `git rm --intent-to-add`) is a particularly good\n> way to express this. I could see `--keep-intent-to-add` as a better\n> alternative, though.\n\nOh good. I also dislike --intent-to-add for a different reasosn but I\ndidn't bring it up: This \"rm --intent-to-cache\" removes the content\nand keeps ita, but there's also the potential use case to keep the\ncontent and remove ita (e.g. you just want to undo intent-to-add\neffect, but if you have already staged some content, leave it).\n\nThis case is about the worktree. But \"git rm\" is not designed for\nthat. It either handles both index and worktree, or just index. \"Just\nworktree\" is delegated to system's \"rm\". But system \"rm\" can't do\nanything about ita bit. And if we make \"git rm --worktree\" do that,\nthen \"rm --worktree --intent-to-add\" would be a much better name and\nshould not be taken by this patch. But \"git rm --worktree\" is not real\nto even start a discussion about that...\n\nEnd of rambling. I guess what I'm saying is, --keep-intent-to-add is a\ngood name (that I also didn't think of).\n-- \nDuy\n"}]}