{"thread":{"id":"48304","subject":"[PATCH] worktree: accept -f as short for --force for removal","startedAt":"2018-04-16T22:17:04Z","lastAt":"2018-04-18T05:04:16Z","messageCount":6,"participants":["Stefan Beller","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"344826","messageId":"20180416221654.49454-1-sbeller@google.com","threadId":"48304","inReplyTo":null,"subject":"[PATCH] worktree: accept -f as short for --force for removal","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-16T22:16:54Z","receivedAt":"2018-04-16T22:17:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The force flag is very common in many commands and is commonly\nabbreviated with '-f', so add that to worktree removal, too\n\nSigned-off-by: Stefan Beller \n---\n\nAs a side note:\n\n $ git worktree add ../test HEAD^\n $ cd ../test\n $ make\n $ git worktree remove test\n fatal: working trees containing submodules cannot be moved or removed\n \n (This is on git.git, which does have submodules, but they should not \n be relevant here? Instead we rather want to point out files left over.\n Not sure. I also plan on having worktrees and submodules to work well\n together eventually)\n\n\n builtin/worktree.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 40a438ed6c..d734874d58 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -783,7 +783,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n {\n \tint force = 0;\n \tstruct option options[] = {\n-\t\tOPT_BOOL(0, \"force\", &force,\n+\t\tOPT_BOOL('f', \"force\", &force,\n \t\t\t N_(\"force removing even if the worktree is dirty\")),\n \t\tOPT_END()\n \t};\n-- \n2.17.0.484.g0c8726318c-goog\n\n"},{"id":"344851","messageId":"CAPig+cSHmxtsz2NNAFvbqXRFKE87ZVyUYpRigjgXE5p8LcR09A@mail.gmail.com","threadId":"48304","inReplyTo":"20180416221654.49454-1-sbeller@google.com","subject":"Re: [PATCH] worktree: accept -f as short for --force for removal","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-17T00:21:58Z","receivedAt":"2018-04-17T00:22:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Apr 16, 2018 at 6:16 PM, Stefan Beller wrote:\n> The force flag is very common in many commands and is commonly\n> abbreviated with '-f', so add that to worktree removal, too\n>\n> Signed-off-by: Stefan Beller \n> ---\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> @@ -783,7 +783,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n> {\n> int force = 0;\n> struct option options[] = {\n> - OPT_BOOL(0, \"force\", &force,\n> + OPT_BOOL('f', \"force\", &force,\n> N_(\"force removing even if the worktree is dirty\")),\n\nShould this be using OPT__FORCE, rather than OPT_BOOL, to be\nconsistent with worktree.c:add()?\n\nAlso, can you amend the commit message to mention that \"git worktree\nremove -f\" was already documented, and that it was only the\nimplementation which was lacking? Thanks.\n\n> OPT_END()\n> };\n"},{"id":"344918","messageId":"20180417181939.247898-1-sbeller@google.com","threadId":"48304","inReplyTo":"CAPig+cSHmxtsz2NNAFvbqXRFKE87ZVyUYpRigjgXE5p8LcR09A@mail.gmail.com","subject":"[PATCH] worktree: accept -f as short for --force for removal","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-17T18:19:39Z","receivedAt":"2018-04-17T18:19:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The force flag is very common in many commands and is commonly\nabbreviated with '-f', so add that to worktree removal, too by using\nOPT__FORCE instead of a self cooked OPT_BOOL for force.\nWhile at it, add the PARSE_OPT_NOCOMPLETE flag to the force command line\noption as forcing a removal may lose files.\n\nThe short form of \"-f\" is already documented in the man page,\nso we do not have to adjust the docs.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Stefan Beller \n---\n\n> Should this be using OPT__FORCE, rather than OPT_BOOL, to be\n> consistent with worktree.c:add()?\n\nyes it should.\n\n> Also, can you amend the commit message to mention that \"git worktree\n> remove -f\" was already documented, and that it was only the\n> implementation which was lacking? Thanks.\n\ndone.\nI tried to clear up the docs, but after reading them a couple times,\nit looks good as-is.\n\n Documentation/git-worktree.txt | 2 +-\n builtin/worktree.c | 5 +++--\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex e7eb24ab85..99b713c849 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n 'git worktree lock' [--reason ] \n 'git worktree move' \n 'git worktree prune' [-n] [-v] [--expire ]\n-'git worktree remove' [--force] \n+'git worktree remove' [-f] \n 'git worktree unlock' \n \n DESCRIPTION\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 40a438ed6c..30647b30c5 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -783,8 +783,9 @@ static int remove_worktree(int ac, const char **av, const char *prefix)\n {\n \tint force = 0;\n \tstruct option options[] = {\n-\t\tOPT_BOOL(0, \"force\", &force,\n-\t\t\t N_(\"force removing even if the worktree is dirty\")),\n+\t\tOPT__FORCE(&force,\n+\t\t\t N_(\"force removing even if the worktree is dirty\"),\n+\t\t\t PARSE_OPT_NOCOMPLETE),\n \t\tOPT_END()\n \t};\n \tstruct worktree **worktrees, *wt;\n-- \n2.17.0.255.g8bfb7c0704\n\n"},{"id":"344921","messageId":"CAPig+cTgoD-GvpoBQ6tcGX4T2XhuKccJAZ40P76vxVD_PfEc9A@mail.gmail.com","threadId":"48304","inReplyTo":"20180417181939.247898-1-sbeller@google.com","subject":"Re: [PATCH] worktree: accept -f as short for --force for removal","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-17T18:46:49Z","receivedAt":"2018-04-17T18:46:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 17, 2018 at 2:19 PM, Stefan Beller wrote:\n> The force flag is very common in many commands and is commonly\n> abbreviated with '-f', so add that to worktree removal, too by using\n> OPT__FORCE instead of a self cooked OPT_BOOL for force.\n\nThe missing bit of this sentence:\n\n ...self cooked OPT_BOOL for force which forgets '-f'.\n\n> While at it, add the PARSE_OPT_NOCOMPLETE flag to the force command line\n> option as forcing a removal may lose files.\n>\n> The short form of \"-f\" is already documented in the man page,\n> so we do not have to adjust the docs.\n\nMakes sense. A possible rewrite (of the entire commit message):\n\n worktree: remove: recognize -f as short for --force\n\n Many commands support a --force option, frequently abbreviated as\n -f, however, \"git worktree remove\"'s hand-rolled OPT_BOOL forgets\n to recognize the short form, despite git-worktree.txt documenting\n -f as supported. Replace OPT_BOOL with OPT__FORCE, which provides\n -f for free, and makes 'remove' consistent with 'add' option\n parsing (which also specifies the PARSE_OPT_NOCOMPLETE flag).\n\n> Helped-by: Eric Sunshine \n> Signed-off-by: Stefan Beller \n\nThe patch itself looks good. Thanks. With or without the above rewrite\nor minor adjustment, this patch is:\n\nReviewed-by: Eric Sunshine \n"},{"id":"344959","messageId":"xmqq4lk9l64o.fsf@gitster-ct.c.googlers.com","threadId":"48304","inReplyTo":"CAPig+cTgoD-GvpoBQ6tcGX4T2XhuKccJAZ40P76vxVD_PfEc9A@mail.gmail.com","subject":"Re: [PATCH] worktree: accept -f as short for --force for removal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-18T00:17:59Z","receivedAt":"2018-04-18T00:18:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Tue, Apr 17, 2018 at 2:19 PM, Stefan Beller wrote:\n>> The force flag is very common in many commands and is commonly\n>> abbreviated with '-f', so add that to worktree removal, too by using\n>> OPT__FORCE instead of a self cooked OPT_BOOL for force.\n>\n> The missing bit of this sentence:\n>\n> ...self cooked OPT_BOOL for force which forgets '-f'.\n>\n>> While at it, add the PARSE_OPT_NOCOMPLETE flag to the force command line\n>> option as forcing a removal may lose files.\n>>\n>> The short form of \"-f\" is already documented in the man page,\n>> so we do not have to adjust the docs.\n>\n> Makes sense. A possible rewrite (of the entire commit message):\n>\n> worktree: remove: recognize -f as short for --force\n>\n> Many commands support a --force option, frequently abbreviated as\n> -f, however, \"git worktree remove\"'s hand-rolled OPT_BOOL forgets\n> to recognize the short form, despite git-worktree.txt documenting\n> -f as supported. Replace OPT_BOOL with OPT__FORCE, which provides\n> -f for free, and makes 'remove' consistent with 'add' option\n> parsing (which also specifies the PARSE_OPT_NOCOMPLETE flag).\n>\n>> Helped-by: Eric Sunshine \n>> Signed-off-by: Stefan Beller \n\nLooks better. I am not sure if s/--force/-f/ in the synopsis\nsection is warranted, but '-f' is commonly understood as '--force'\n(and that is the point of this patch after all), so it is probably\nan improvement to be briefer.\n\nThanks, both.\n\n>\n> The patch itself looks good. Thanks. With or without the above rewrite\n> or minor adjustment, this patch is:\n>\n> Reviewed-by: Eric Sunshine \n"},{"id":"344969","messageId":"CAPig+cRHW3TwrqwrY9R0-v50Z5U8uz99Hn+ERtP_1Ey2K_Vk=Q@mail.gmail.com","threadId":"48304","inReplyTo":"xmqq4lk9l64o.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] worktree: accept -f as short for --force for removal","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-18T05:01:39Z","receivedAt":"2018-04-18T05:04:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 17, 2018 at 8:17 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>> Makes sense. A possible rewrite (of the entire commit message):\n>>\n>> worktree: remove: recognize -f as short for --force\n>>\n>> Many commands support a --force option, frequently abbreviated as\n>> -f, however, \"git worktree remove\"'s hand-rolled OPT_BOOL forgets\n>> to recognize the short form, despite git-worktree.txt documenting\n>> -f as supported. Replace OPT_BOOL with OPT__FORCE, which provides\n>> -f for free, and makes 'remove' consistent with 'add' option\n>> parsing (which also specifies the PARSE_OPT_NOCOMPLETE flag).\n>>\n> Looks better. I am not sure if s/--force/-f/ in the synopsis\n> section is warranted, but '-f' is commonly understood as '--force'\n> (and that is the point of this patch after all), so it is probably\n> an improvement to be briefer.\n\nI meant to mention the synopsis change in the rewritten commit\nmessage. The s/--force/-f/ for 'remove' makes it consistent with the\nhow it's shown for 'add' in the synopsis. (Changing it the other way,\nso 'add' shows --force would also work.)\n"}]}