{"thread":{"id":"33614","subject":"[PATCH] clean: confirm before cleaning files and directories","startedAt":"2013-04-26T08:07:56Z","lastAt":"2013-05-03T12:54:59Z","messageCount":30,"participants":["Jiang Xin","Matthieu Moy","Thomas Rast","Junio C Hamano","Eric Sunshine","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"215567","messageId":"1826d070612808b301f9295838e226e02d8097ad.1366963586.git.worldhello.net@gmail.com","threadId":"33614","inReplyTo":null,"subject":"[PATCH] clean: confirm before cleaning files and directories","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-04-26T08:07:56Z","receivedAt":"2013-04-26T08:07:56Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"When running `git clean`, it will be convenient and safe to show a\nconfirm dialog and only delete files and directories when confirmed.\nThe confirm dialog will popup when:\n\n * run `git clean` in interactive sessions,\n * not a dry run,\n * and not quiet.\n\nThere may be existing scripts that call `git clean -f` while leave the\nstandard input and the standard output of the `git clean` connected to\nwhatever environment the scripts were started, and such invocation might\ntrigger the confirm dialog. In this case, add a `-q` option, such as\n`git clean -q -f`, then the confirm dialog won't popup.\n\nSigned-off-by: Jiang Xin \n---\n builtin/clean.c | 66 +++++++++++++++++++++++++++++++++++++++++++++------------\n 1 file changed, 52 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 04e39..e31a1 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -169,6 +169,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\tN_(\"remove only ignored files\")),\n \t\tOPT_END()\n \t};\n+\tstruct string_list dels = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *item;\n+\tstruct strbuf confirm = STRBUF_INIT;\n+\tint confirmed = 0;\n \n \tgit_config(git_clean_config, NULL);\n \tif (force < 0)\n@@ -257,33 +261,67 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tstrbuf_addstr(&directory, ent->name);\n \t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n+\t\t\t\tstring_list_append(&dels, ent->name);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tif (pathspec && !matches)\n+\t\t\t\tcontinue;\n+\t\t\tstring_list_append(&dels, ent->name);\n+\t\t}\n+\t}\n+\n+\tif (!isatty(0) || !isatty(1) || quiet || dry_run) {\n+\t\tconfirmed = 1;\n+\t}\n+\n+\tif (!confirmed && dels.nr > 0) {\n+\t\tfor_each_string_list_item(item, &dels) {\n+\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\t\tprintf(_(msg_would_remove), qname);\n+\t\t}\n+\t\tprintf(_(\"Are you sure [y/N]? \"));\n+\t\tstrbuf_getline(&confirm, stdin, '\\n');\n+\t\tstrbuf_trim(&confirm);\n+\t\tif (!strcasecmp(confirm.buf, \"y\")) {\n+\t\t\tconfirmed = 1;\n+\t\t}\n+\t}\n+\n+\tif (confirmed) {\n+\t\tfor_each_string_list_item(item, &dels) {\n+\t\t\tstruct stat st;\n+\n+\t\t\tif (lstat(item->string, &st))\n+\t\t\t\tcontinue;\n+\n+\t\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\t\tstrbuf_addstr(&directory, item->string);\n \t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n \t\t\t\t\terrors++;\n \t\t\t\tif (gone && !quiet) {\n \t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n \t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t\t}\n-\t\t\t}\n-\t\t\tstrbuf_reset(&directory);\n-\t\t} else {\n-\t\t\tif (pathspec && !matches)\n-\t\t\t\tcontinue;\n-\t\t\tres = dry_run ? 0 : unlink(ent->name);\n-\t\t\tif (res) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n-\t\t\t\twarning(_(msg_warn_remove_failed), qname);\n-\t\t\t\terrors++;\n-\t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n-\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n+\t\t\t\tstrbuf_reset(&directory);\n+\t\t\t} else {\n+\t\t\t\tres = dry_run ? 0 : unlink(item->string);\n+\t\t\t\tif (res) {\n+\t\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\t\t\t\twarning(_(msg_warn_remove_failed), qname);\n+\t\t\t\t\terrors++;\n+\t\t\t\t} else if (!quiet) {\n+\t\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n+\t\t\t\t}\n \t\t\t}\n \t\t}\n \t}\n \tfree(seen);\n \n \tstrbuf_release(&directory);\n+\tstrbuf_release(&confirm);\n \tstring_list_clear(&exclude_list, 0);\n+\tstring_list_clear(&dels, 0);\n \treturn (errors != 0);\n }\n-- \n1.8.2.1.628.gcd33b41\n"},{"id":"215569","messageId":"vpqfvydhfbx.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"1826d070612808b301f9295838e226e02d8097ad.1366963586.git.worldhello.net@gmail.com","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-26T08:21:54Z","receivedAt":"2013-04-26T08:21:54Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> * run `git clean` in interactive sessions,\n> * not a dry run,\n> * and not quiet.\n\nErr, does this mean I'll have:\n\n$ git clean\nfatal: clean.requireForce defaults to true and neither -n nor -f given; refusing to clean\n$ git clean --force\nAre you sure [y/n]?\n\nAn optional confirmation dialog seems interesting, but activating it by\ndefault even with --force seems really weird.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215572","messageId":"CANYiYbFzEoEgJzKsB_hiKNy2JCxaTDX30wXNjnzComOzJJF_cw@mail.gmail.com","threadId":"33614","inReplyTo":"vpqfvydhfbx.fsf@grenoble-inp.fr","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-04-26T08:41:19Z","receivedAt":"2013-04-26T08:41:19Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/4/26 Matthieu Moy \n>\n> Jiang Xin writes:\n>\n> > * run `git clean` in interactive sessions,\n> > * not a dry run,\n> > * and not quiet.\n>\n> Err, does this mean I'll have:\n>\n> $ git clean\n> fatal: clean.requireForce defaults to true and neither -n nor -f given; refusing to clean\n> $ git clean --force\n\n( you omit something, because nothing to clean won't trigger this\nconfirm dialog)\n\n> Are you sure [y/n]?\n>\n> An optional confirmation dialog seems interesting, but activating it by\n> default even with --force seems really weird.\n\nI don't know how many programmers had been bitten by runing `git clean -fdx`,\nbut I bet there were some. I think safety should be put to the 1st place.\nIt is because \"clean.requireForce\" defaults to true, all people trend to run\n'git clean' with the '--force/-f' option.\n\nMaybe we can do like this:\n\n1. Set the default value of 'clean.requireForce' to false.\n2. Show a error message and do nothing, if there is not 'clean.requireForce'\n setting, but the user called with a '--force' flag.\n ( like a transition for the change of push.default in git 2.0)\n\nany opinions?\n\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n\n\n\n--\n蒋鑫\n\n北京群英汇信息技术有限公司\n邮件: worldhello.net@gmail.com\n网址: http://www.ossxp.com/\n博客: http://www.worldhello.net/\n微博: http://weibo.com/gotgit/\n电话: 18601196889\n"},{"id":"215574","messageId":"vpqwqrpfzez.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"CANYiYbFzEoEgJzKsB_hiKNy2JCxaTDX30wXNjnzComOzJJF_cw@mail.gmail.com","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-26T08:51:00Z","receivedAt":"2013-04-26T08:51:00Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> Maybe we can do like this:\n>\n> 1. Set the default value of 'clean.requireForce' to false.\n> 2. Show a error message and do nothing, if there is not 'clean.requireForce'\n> setting, but the user called with a '--force' flag.\n> ( like a transition for the change of push.default in git 2.0)\n\nPerhaps introducing a new value for 'clean.requireForce':\n\n$ git config --global clean.requireForce ask\n$ git clean\nwill remove ...\nare you sure [y/N]?\n\nThe error message when clean.requireForce is unset and --force is not\ngiven could point the user to clean.requireForce=ask.\n\nThen, maybe, later, this could become the default. But I tend to like\nthe non-interactive nature of most Git commands, so I'm a bit reluctant\nhere. My way of doing the confirmation dialog is\n\n$ git clean -n\nwould remove ...\n$ git clean -f\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215575","messageId":"87y5c565ca.fsf@linux-k42r.v.cablecom.net","threadId":"33614","inReplyTo":"CANYiYbFzEoEgJzKsB_hiKNy2JCxaTDX30wXNjnzComOzJJF_cw@mail.gmail.com","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-26T08:53:09Z","receivedAt":"2013-04-26T08:53:09Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jiang Xin writes:\n\n> I don't know how many programmers had been bitten by runing `git clean -fdx`,\n> but I bet there were some. I think safety should be put to the 1st place.\n> It is because \"clean.requireForce\" defaults to true, all people trend to run\n> 'git clean' with the '--force/-f' option.\n\nBut if that's the real problem, wouldn't it be better to introduce\nsomething like clean.requireForce=interactive (or 'ask', or whatever you\nmake up) which will be like clean.requireForce=true except that when on\na TTY, we ask the user?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"215585","messageId":"CANYiYbFTTGds3yU+-9BL=oTXFk5X4jR_MMR3cQVE4=qPgutWEg@mail.gmail.com","threadId":"33614","inReplyTo":"vpqwqrpfzez.fsf@grenoble-inp.fr","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-04-26T10:00:24Z","receivedAt":"2013-04-26T10:00:24Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/4/26 Matthieu Moy :\n> Jiang Xin writes:\n>\n>> Maybe we can do like this:\n>>\n>> 1. Set the default value of 'clean.requireForce' to false.\n>> 2. Show a error message and do nothing, if there is not 'clean.requireForce'\n>> setting, but the user called with a '--force' flag.\n>> ( like a transition for the change of push.default in git 2.0)\n>\n> Perhaps introducing a new value for 'clean.requireForce':\n>\n> $ git config --global clean.requireForce ask\n> $ git clean\n> .will remove ...\n> are you sure [y/N]?\n>\n> The error message when clean.requireForce is unset and --force is not\n> given could point the user to clean.requireForce=ask.\n\nAdd new value for clean.requireForce would break old git clent.\n\n $ git clean\n fatal: bad config value for 'clean.requireforce' in .git/config\n\n>\n> Then, maybe, later, this could become the default. But I tend to like\n> the non-interactive nature of most Git commands, so I'm a bit reluctant\n> here. My way of doing the confirmation dialog is\n>\n> $ git clean -n\n> would remove ...\n> $ git clean -f\n>\n\nI try to put all cases in one table, but still looks weird.\n\n | clean.requireForce | git clean | git clean --force |\n +---------------------+-------------------|-------------------|\n | TRUE | error | delete... |\n +---------------------+-------------------|-------------------|\n | FALSE | delete... | delete... |\n +---------------------+-------------------|-------------------|\n | unset | confirm | warn + confirm |\n +---------------------+-------------------|-------------------|\n\nDoes anyone really set and use \"clean.requireForce\" setting?\nAnd if there is a confirm dialog, do we need 'clean.requireForce' any more?\nOr we can add a `--no-ask` option, in order to override the confirm dialog.\n\n\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n\n\n--\n蒋鑫\n\n北京群英汇信息技术有限公司\n邮件: worldhello.net@gmail.com\n网址: http://www.ossxp.com/\n博客: http://www.worldhello.net/\n微博: http://weibo.com/gotgit/\n电话: 18601196889\n"},{"id":"215596","messageId":"7vbo91z30e.fsf@alter.siamese.dyndns.org","threadId":"33614","inReplyTo":"CANYiYbFzEoEgJzKsB_hiKNy2JCxaTDX30wXNjnzComOzJJF_cw@mail.gmail.com","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-26T16:10:41Z","receivedAt":"2013-04-26T16:10:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> I don't know how many programmers had been bitten by runing `git clean -fdx`,\n> but I bet there were some. I think safety should be put to the 1st place.\n\n\"git clean\" without -n/f errors out, hinting the availablilty of -n\nwhile mentioning -f; that is the safety, isn't it? Once the user\ndecides to give -f, the user _wants_ to remove cruft, and it is a\nhinderance to require any further confirmation.\n\nYour problem description sounds like that you want to drive without\nwearing seatbelts but in case you get in a collision, you suggest to\nmake the seat automatically stick to the back of anybody who sits on\nit with superglue to make it safer.\n\nThat approach might give it an alternative safety, but it would make\nit extremely annoying if you apply it also to people who do wear\nbelts, no?\n\nThis extra confirmation should never be the default.\n"},{"id":"215597","messageId":"vpqzjwl9se7.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"7vbo91z30e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-26T16:19:12Z","receivedAt":"2013-04-26T16:19:12Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> \"git clean\" without -n/f errors out, hinting the availablilty of -n\n> while mentioning -f; that is the safety, isn't it? Once the user\n> decides to give -f, the user _wants_ to remove cruft, and it is a\n> hinderance to require any further confirmation.\n\nThis is only half true: because \"git clean\" does nothing without -f,\npeople's fingers get trained to type \"git clean -f\" without really\nthinking about it. Just like people alias rm='rm -i' and then type\n\"rm -fr *\" mechanically.\n\nThe nice thing with the confirmation dialog is that it shows the list\nbefore asking (and unlike 'rm -i', it asks only once).\n\nBut as I said in another thread, I fully agree that adding the\nconfirmation by default even with -f is nonsense.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215601","messageId":"7vppxhxltk.fsf@alter.siamese.dyndns.org","threadId":"33614","inReplyTo":"vpqzjwl9se7.fsf@grenoble-inp.fr","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-26T17:07:19Z","receivedAt":"2013-04-26T17:07:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> The nice thing with the confirmation dialog is that it shows the list\n> before asking (and unlike 'rm -i', it asks only once).\n\nI wouldn't object to having \"clean -i\", which automatically defeats\nthe requireforce option.\n\nAs to a huge single list you have to approve or reject as a whole, I\nam on the fence. When running \"rm -i\", I often wished to see\nsomething like that, but I am fairly sure that I'll call it unusable\nthe first time I see a list with a few items I want to keep while\nremoving all others.\n"},{"id":"215616","messageId":"7vmwslw1py.fsf@alter.siamese.dyndns.org","threadId":"33614","inReplyTo":"7vppxhxltk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-26T19:06:49Z","receivedAt":"2013-04-26T19:06:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> The nice thing with the confirmation dialog is that it shows the list\n>> before asking (and unlike 'rm -i', it asks only once).\n>\n> I wouldn't object to having \"clean -i\", which automatically defeats\n> the requireforce option.\n>\n> As to a huge single list you have to approve or reject as a whole, I\n> am on the fence. When running \"rm -i\", I often wished to see\n> something like that, but I am fairly sure that I'll call it unusable\n> the first time I see a list with a few items I want to keep while\n> removing all others.\n\nElaborating on this a bit more, hoping it would help people who want\nto design the \"--confirm-before-doing\" option...\n\nThe primary reason I think the user will find \"We are going to\nremove these. OK?\" irritating is that most of the time, there are\nonly a few items that the user would want to keep.\n\n\t$ rm --confirm-before-doing -r path\n\t... list of three dozens of items, among which\n ... there may be two items that should be kept\n Remove all? [Y/n]\n\nAfter seeing this prompt and saying 'n', the user would _not_ thank\nthe command for reminding about these precious two items, because\nthe only next step available to the user is to remove the remaining\n34 items manually.\n\n\"Confirm in bulk before doing\" feature can become useful if it had a\n\"line item veto\" option in the confirmation time. The interaction\nthen could go like this:\n\n\t$ rm --confirm-before-doing -r path\n\tpath/foo path/frotz/nitfol path/sotto\n path/bar path/frotz/xyzzy path/trail\n ... ... ... \n Remove (list items you want to keep)? path/frotz\n\nand the user could instruct it to remove everything other than those\ninside path/fortz. If the user do not want to remove anything,\nthere is an option to ^C out of the command.\n"},{"id":"215672","messageId":"CANYiYbEjcPMnzZvFgJON-K0WekucGOQVn0dk9_W9HkrNZJN3Lw@mail.gmail.com","threadId":"33614","inReplyTo":"7vmwslw1py.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] clean: confirm before cleaning files and directories","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-04-27T02:09:10Z","receivedAt":"2013-04-27T02:09:10Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/4/27 Junio C Hamano :\n> Junio C Hamano writes:\n>\n>> Matthieu Moy writes:\n>>\n>>> The nice thing with the confirmation dialog is that it shows the list\n>>> before asking (and unlike 'rm -i', it asks only once).\n>>\n>> I wouldn't object to having \"clean -i\", which automatically defeats\n>> the requireforce option.\n>>\n>> As to a huge single list you have to approve or reject as a whole, I\n>> am on the fence. When running \"rm -i\", I often wished to see\n>> something like that, but I am fairly sure that I'll call it unusable\n>> the first time I see a list with a few items I want to keep while\n>> removing all others.\n>\n> Elaborating on this a bit more, hoping it would help people who want\n> to design the \"--confirm-before-doing\" option...\n>\n> The primary reason I think the user will find \"We are going to\n> remove these. OK?\" irritating is that most of the time, there are\n> only a few items that the user would want to keep.\n>\n> $ rm --confirm-before-doing -r path\n> ... list of three dozens of items, among which\n> ... there may be two items that should be kept\n> Remove all? [Y/n]\n>\n> After seeing this prompt and saying 'n', the user would _not_ thank\n> the command for reminding about these precious two items, because\n> the only next step available to the user is to remove the remaining\n> 34 items manually.\n>\n> \"Confirm in bulk before doing\" feature can become useful if it had a\n> \"line item veto\" option in the confirmation time. The interaction\n> then could go like this:\n>\n> $ rm --confirm-before-doing -r path\n> path/foo path/frotz/nitfol path/sotto\n> path/bar path/frotz/xyzzy path/trail\n> ... ... ...\n> Remove (list items you want to keep)? path/frotz\n>\n> and the user could instruct it to remove everything other than those\n> inside path/fortz. If the user do not want to remove anything,\n> there is an option to ^C out of the command.\n\nAgree. I will send a reroll latter.\n\n-- \nJiang Xin\n"},{"id":"215715","messageId":"2a68239bfd3e6b9ea1842dd8e468ee3d3217af5b.1367079089.git.worldhello.net@gmail.com","threadId":"33614","inReplyTo":"7vmwslw1py.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] Add support for -i/--interactive to git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-04-27T16:13:26Z","receivedAt":"2013-04-27T16:13:26Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Show what would be done and a confirmation dialog before actually\ncleaning. In the confirmation dialog, the user can input a space\nseparated prefix list, and each clean candidate that matches with\none of prefix, will be excluded from cleaning. When the user feels\nit's OK, press ENTER to start cleaning. If the user wants to cancel\nthe whole cleaning, simply input ctrl-c in the confirmation dialog.\n\nSigned-off-by: Jiang Xin \nReviewed-by: Matthieu Moy \nSuggested-by: Junio C Hamano \n---\n Documentation/git-clean.txt | 14 +++++-\n builtin/clean.c | 101 +++++++++++++++++++++++++++++++++++++-------\n 2 files changed, 98 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt\nindex bdc3a..60a30 100644\n--- a/Documentation/git-clean.txt\n+++ b/Documentation/git-clean.txt\n@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n SYNOPSIS\n --------\n [verse]\n-'git clean' [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\n+'git clean' [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\n \n DESCRIPTION\n -----------\n@@ -34,7 +34,17 @@ OPTIONS\n -f::\n --force::\n \tIf the Git configuration variable clean.requireForce is not set\n-\tto false, 'git clean' will refuse to run unless given -f or -n.\n+\tto false, 'git clean' will refuse to run unless given -f, -n or\n+\t-i.\n+\n+-i::\n+--interactive::\n+ Show what would be done and a confirmation dialog before actually\n+ cleaning. In the confirmation dialog, the user can input a space\n+ separated prefix list, and each clean candidate that matches with\n+ one of prefix, will be excluded from cleaning. When the user feels\n+ it's OK, press ENTER to start cleaning. If the user wants to cancel\n+ the whole cleaning, simply input ctrl-c in the confirmation dialog.\n \n -n::\n --dry-run::\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 04e39..eee04 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -15,9 +15,10 @@\n #include \"quote.h\"\n \n static int force = -1; /* unset */\n+static int interactive;\n \n static const char *const builtin_clean_usage[] = {\n-\tN_(\"git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n+\tN_(\"git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n \tNULL\n };\n \n@@ -154,12 +155,15 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n \tstruct exclude_list *el;\n+\tstruct string_list dels = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *item;\n \tconst char *qname;\n \tchar *seen = NULL;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"do not print names of files removed\")),\n \t\tOPT__DRY_RUN(&dry_run, N_(\"dry run\")),\n \t\tOPT__FORCE(&force, N_(\"force\")),\n+\t\tOPT_BOOL('i', \"interactive\", &interactive, N_(\"interactive cleaning\")),\n \t\tOPT_BOOLEAN('d', NULL, &remove_directories,\n \t\t\t\tN_(\"remove whole directories\")),\n \t\t{ OPTION_CALLBACK, 'e', \"exclude\", &exclude_list, N_(\"pattern\"),\n@@ -186,12 +190,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (ignored && ignored_only)\n \t\tdie(_(\"-x and -X cannot be used together\"));\n \n-\tif (!dry_run && !force) {\n+\tif (!dry_run && !force && !interactive) {\n \t\tif (config_set)\n-\t\t\tdie(_(\"clean.requireForce set to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce set to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t\telse\n-\t\t\tdie(_(\"clean.requireForce defaults to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce defaults to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t}\n \n@@ -257,26 +261,92 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tstrbuf_addstr(&directory, ent->name);\n \t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n-\t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n-\t\t\t\t\terrors++;\n-\t\t\t\tif (gone && !quiet) {\n-\t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n-\t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n-\t\t\t\t}\n+\t\t\t\tstring_list_append(&dels, ent->name);\n \t\t\t}\n-\t\t\tstrbuf_reset(&directory);\n \t\t} else {\n \t\t\tif (pathspec && !matches)\n \t\t\t\tcontinue;\n-\t\t\tres = dry_run ? 0 : unlink(ent->name);\n+\t\t\tstring_list_append(&dels, ent->name);\n+\t\t}\n+\t}\n+\n+\tif (interactive && dels.nr > 0 && !dry_run && isatty(0) && isatty(1)) {\n+\t\tstruct strbuf confirm = STRBUF_INIT;\n+\n+\t\twhile (1) {\n+\t\t\tstruct strbuf **prefix_list, **prefix_list_head;\n+\n+\t\t\t/* dels list may become empty when we run string_list_remove_empty_items latter */\n+\t\t\tif (!dels.nr)\n+\t\t\t\tbreak;\n+\n+\t\t\tfor_each_string_list_item(item, &dels) {\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\t\t\tprintf(_(msg_would_remove), qname);\n+\t\t\t}\n+\n+\t\t\tprintf(_(\"Remove (press enter to confirm or input items you want to keep)? \"));\n+\t\t\tstrbuf_getline(&confirm, stdin, '\\n');\n+\t\t\tstrbuf_trim(&confirm);\n+\n+\t\t\tif (!confirm.len)\n+\t\t\t\tbreak;\n+\n+\t\t\tprintf(\"\\n\");\n+\n+\t\t\tprefix_list_head = strbuf_split_buf(confirm.buf, confirm.len, ' ', 0);\n+\t\t\tfor (prefix_list = prefix_list_head; *prefix_list; *prefix_list++)\n+\t\t\t{\n+\t\t\t\tint prefix_matched = 0;\n+\n+\t\t\t\tstrbuf_trim(*prefix_list);\n+\t\t\t\tif (!(*prefix_list)->len)\n+\t\t\t\t\tcontinue;\n+\n+\t\t\t\tfor_each_string_list_item(item, &dels) {\n+\t\t\t\t\tif (!strncasecmp(item->string, (*prefix_list)->buf, (*prefix_list)->len)) {\n+\t\t\t\t\t\t*item->string = '\\0';\n+\t\t\t\t\t\tprefix_matched++;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tif (!prefix_matched) {\n+\t\t\t\t\twarning(_(\"Cannot find items start with the given prefix: %s\"), (*prefix_list)->buf);\n+\t\t\t\t\tprintf(\"\\n\");\n+\t\t\t\t} else {\n+\t\t\t\t\tstring_list_remove_empty_items(&dels, 0);\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tstrbuf_reset(&confirm);\n+\t\t\tstrbuf_list_free(prefix_list_head);\n+\t\t}\n+\t\tstrbuf_release(&confirm);\n+\t}\n+\n+\tfor_each_string_list_item(item, &dels) {\n+\t\tstruct stat st;\n+\n+\t\tif (lstat(item->string, &st))\n+\t\t\tcontinue;\n+\n+\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\tstrbuf_addstr(&directory, item->string);\n+\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n+\t\t\t\terrors++;\n+\t\t\tif (gone && !quiet) {\n+\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n+\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n+\t\t\t}\n+\t\t\tstrbuf_reset(&directory);\n+\t\t} else {\n+\t\t\tres = dry_run ? 0 : unlink(item->string);\n \t\t\tif (res) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n \t\t\t\twarning(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\n@@ -285,5 +355,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tstrbuf_release(&directory);\n \tstring_list_clear(&exclude_list, 0);\n+\tstring_list_clear(&dels, 0);\n \treturn (errors != 0);\n }\n-- \n1.8.2.1.921.g1826d07\n"},{"id":"215763","messageId":"vpqip37abyq.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"2a68239bfd3e6b9ea1842dd8e468ee3d3217af5b.1367079089.git.worldhello.net@gmail.com","subject":"Re: [PATCH v2] Add support for -i/--interactive to git-clean","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-27T21:41:01Z","receivedAt":"2013-04-27T21:41:01Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> Reviewed-by: Matthieu Moy \n\nErr, no: I commented on the intention of the first patch, but did not\n_review_ it, and didn't read this version yet.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215771","messageId":"7vhairsh58.fsf@alter.siamese.dyndns.org","threadId":"33614","inReplyTo":"vpqip37abyq.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2] Add support for -i/--interactive to git-clean","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-27T23:11:47Z","receivedAt":"2013-04-27T23:11:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Jiang Xin writes:\n>\n>> Reviewed-by: Matthieu Moy \n>\n> Err, no: I commented on the intention of the first patch, but did not\n> _review_ it, and didn't read this version yet.\n\nYeah I suspected as such.\n\nAnd thanks for saying it loud in public, so that other contributors\ncan learn from this.\n"},{"id":"215781","messageId":"CAPig+cTRqB7pGcxmW_m=AGT_nRWxgR+X8z6FR3PgONJ-0kG2fA@mail.gmail.com","threadId":"33614","inReplyTo":"2a68239bfd3e6b9ea1842dd8e468ee3d3217af5b.1367079089.git.worldhello.net@gmail.com","subject":"Re: [PATCH v2] Add support for -i/--interactive to git-clean","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-04-28T02:03:26Z","receivedAt":"2013-04-28T02:03:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 27, 2013 at 12:13 PM, Jiang Xin wrote:\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -257,26 +261,92 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> }\n>\n> if (S_ISDIR(st.st_mode)) {\n> - strbuf_addstr(&directory, ent->name);\n> if (remove_directories || (matches == MATCHED_EXACTLY)) {\n> - if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n> - errors++;\n> - if (gone && !quiet) {\n> - qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n> - printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n> - }\n> + string_list_append(&dels, ent->name);\n> }\n> - strbuf_reset(&directory);\n> } else {\n> if (pathspec && !matches)\n> continue;\n> - res = dry_run ? 0 : unlink(ent->name);\n> + string_list_append(&dels, ent->name);\n> + }\n> + }\n> +\n> + if (interactive && dels.nr > 0 && !dry_run && isatty(0) && isatty(1)) {\n> + struct strbuf confirm = STRBUF_INIT;\n> +\n> + while (1) {\n> + struct strbuf **prefix_list, **prefix_list_head;\n> +\n> + /* dels list may become empty when we run string_list_remove_empty_items latter */\n\ns/latter/later/\n\n> + if (!dels.nr)\n> + break;\n> +\n> + for_each_string_list_item(item, &dels) {\n> + qname = quote_path_relative(item->string, -1, &buf, prefix);\n> + printf(_(msg_would_remove), qname);\n> + }\n> +\n> + printf(_(\"Remove (press enter to confirm or input items you want to keep)? \"));\n> + strbuf_getline(&confirm, stdin, '\\n');\n> + strbuf_trim(&confirm);\n> +\n> + if (!confirm.len)\n> + break;\n> +\n> + printf(\"\\n\");\n> +\n> + prefix_list_head = strbuf_split_buf(confirm.buf, confirm.len, ' ', 0);\n> + for (prefix_list = prefix_list_head; *prefix_list; *prefix_list++)\n> + {\n> + int prefix_matched = 0;\n> +\n> + strbuf_trim(*prefix_list);\n> + if (!(*prefix_list)->len)\n> + continue;\n> +\n> + for_each_string_list_item(item, &dels) {\n> + if (!strncasecmp(item->string, (*prefix_list)->buf, (*prefix_list)->len)) {\n> + *item->string = '\\0';\n> + prefix_matched++;\n> + }\n> + }\n> + if (!prefix_matched) {\n> + warning(_(\"Cannot find items start with the given prefix: %s\"), (*prefix_list)->buf);\n\ns/start/starting/\n[...or...]\ns/items start with the given prefix/items prefixed by/\n\n> + printf(\"\\n\");\n> + } else {\n> + string_list_remove_empty_items(&dels, 0);\n> + }\n> + }\n> +\n> + strbuf_reset(&confirm);\n> + strbuf_list_free(prefix_list_head);\n> + }\n> + strbuf_release(&confirm);\n> + }\n> +\n> + for_each_string_list_item(item, &dels) {\n> + struct stat st;\n> +\n> + if (lstat(item->string, &st))\n> + continue;\n> +\n> + if (S_ISDIR(st.st_mode)) {\n> + strbuf_addstr(&directory, item->string);\n> + if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n> + errors++;\n> + if (gone && !quiet) {\n> + qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n> + printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n> + }\n> + strbuf_reset(&directory);\n> + } else {\n> + res = dry_run ? 0 : unlink(item->string);\n> if (res) {\n> - qname = quote_path_relative(ent->name, -1, &buf, prefix);\n> + qname = quote_path_relative(item->string, -1, &buf, prefix);\n> warning(_(msg_warn_remove_failed), qname);\n> errors++;\n> } else if (!quiet) {\n> - qname = quote_path_relative(ent->name, -1, &buf, prefix);\n> + qname = quote_path_relative(item->string, -1, &buf, prefix);\n> printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n> }\n> }\n"},{"id":"215834","messageId":"vpqwqrl931e.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"2a68239bfd3e6b9ea1842dd8e468ee3d3217af5b.1367079089.git.worldhello.net@gmail.com","subject":"Re: [PATCH v2] Add support for -i/--interactive to git-clean","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-29T08:03:41Z","receivedAt":"2013-04-29T08:03:41Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> Show what would be done and a confirmation dialog before actually\n> cleaning. In the confirmation dialog, the user can input a space\n> separated prefix list, and each clean candidate that matches with\n> one of prefix, will be excluded from cleaning.\n\nThat seems a really weird interface. In particular, that means people\ntyping \"no\" or \"n\" mechanically will have everything not starting with\n\"n\" or \"no\" deleted. We've learnt from the \"git send-email\" interface\nthat people (including me ;-) ) do type \"yes\" mechanically to some\nquestions.\n\nYou may argue that users are not stupid and know what they do, but the\npoint of this -i is to protect users against their fingers (typing -f\nwithout thinking) for a potentially very destructive command ...\n\nMy feeling is that you're doing a bad compromise between a confirmation\ndialog (y/n) and a really interactive command (like \"git add -i\") with a\nrich interface.\n\n> @@ -34,7 +34,17 @@ OPTIONS\n> -f::\n> --force::\n> \tIf the Git configuration variable clean.requireForce is not set\n> -\tto false, 'git clean' will refuse to run unless given -f or -n.\n> +\tto false, 'git clean' will refuse to run unless given -f, -n or\n> +\t-i.\n> +\n> +-i::\n> +--interactive::\n> + Show what would be done and a confirmation dialog before actually\n> + cleaning. In the confirmation dialog, the user can input a space\n> + separated prefix list, and each clean candidate that matches with\n> + one of prefix, will be excluded from cleaning. When the user feels\n> + it's OK, press ENTER to start cleaning. If the user wants to cancel\n> + the whole cleaning, simply input ctrl-c in the confirmation dialog.\n\nBroken indentation. It seems you've set your tab-width to 2, in which\ncase you can't see it, but you've indented with 2 spaces where the rest\nof the file is indented with tabs.\n\n> \t\tif (S_ISDIR(st.st_mode)) {\n> -\t\t\tstrbuf_addstr(&directory, ent->name);\n> \t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n> -\t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n> -\t\t\t\t\terrors++;\n> -\t\t\t\tif (gone && !quiet) {\n> -\t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n> -\t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n> -\t\t\t\t}\n> +\t\t\t\tstring_list_append(&dels, ent->name);\n\nThe patch would be much easier to read if split into a first refactoring\npatch that would introduce this \"dels\" list, and a second patch that\nwould introduce -i. Reading this, I wondered why the code was removed,\nwhile it was actually just moved.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215852","messageId":"6733a9080da07ace27c3252de2399be339b4e349.1367245712.git.git@drmicha.warpmail.net","threadId":"33614","inReplyTo":"vpqwqrl931e.fsf@grenoble-inp.fr","subject":"[PATCH] clean: Introduce -z for machine readable output","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-04-29T14:30:57Z","receivedAt":"2013-04-29T14:30:57Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"-z makes clean output only the names of paths which are or would be\ndeleted, and separates them with \\0.\n\nUse as \"xargs -0 -a <(git clean -nz [-d]) rm -ri\", e.g., as a quick\n\"git clean -i\".\n\nSigned-off-by: Michael J Gruber \n---\nHere's an alternative approach to that problem with a complete different attach\nvector but a similar purpose. I've been using it for a while, but it's kind of\nunpolished. My \"git-clean-i\" looks like:\n\n--->%---\n#!/bin/bash\nxargs -0 -a <(git clean -n -z \"$@\") rm -ri\n--->%---\n\n Documentation/git-clean.txt | 9 ++++++++-\n builtin/clean.c | 27 +++++++++++++++++++--------\n 2 files changed, 27 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt\nindex bdc3ab8..849e775 100644\n--- a/Documentation/git-clean.txt\n+++ b/Documentation/git-clean.txt\n@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n SYNOPSIS\n --------\n [verse]\n-'git clean' [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\n+'git clean' [-d] [-f] [-n] [-q] [-z] [-e ] [-x | -X] [--] ...\n \n DESCRIPTION\n -----------\n@@ -63,6 +63,13 @@ OPTIONS\n \tRemove only files ignored by Git. This may be useful to rebuild\n \teverything from scratch, but keep manually created files.\n \n+-z::\n+\tUse machine readable output for (to be) removed paths: Output the paths\n+\twhich are or would be removed only (without extra wording) and\n+\tseparate them with \\0.\n++\n+This does not imply `-n` but can be combined with it.\n+\n SEE ALSO\n --------\n linkgit:gitignore[5]\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 04e396b..0ebba24 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -41,8 +41,18 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n+void write_name(const char *fmt, const char *name, int nul_terminated)\n+{\n+\tif (nul_terminated) {\n+\t\tfputs(name, stdout);\n+\t\tfputc(0, stdout);\n+\t} else {\n+\t\tprintf(fmt, name);\n+\t}\n+}\n+\n static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n-\t\tint dry_run, int quiet, int *dir_gone)\n+\t\tint dry_run, int quiet, int *dir_gone, int nul_terminated)\n {\n \tDIR *dir;\n \tstruct strbuf quoted = STRBUF_INIT;\n@@ -57,8 +67,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\t!resolve_gitlink_ref(path->buf, \"HEAD\", submodule_head)) {\n \t\tif (!quiet) {\n \t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n-\t\t\tprintf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),\n-\t\t\t\t\tquoted.buf);\n+\t\t\twrite_name(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),\n+\t\t\t\t\tquoted.buf, nul_terminated);\n \t\t}\n \n \t\t*dir_gone = 0;\n@@ -91,7 +101,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\tif (lstat(path->buf, &st))\n \t\t\t; /* fall thru */\n \t\telse if (S_ISDIR(st.st_mode)) {\n-\t\t\tif (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))\n+\t\t\tif (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone, nul_terminated))\n \t\t\t\tret = 1;\n \t\t\tif (gone) {\n \t\t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n@@ -136,7 +146,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \n \tif (!*dir_gone && !quiet) {\n \t\tfor (i = 0; i < dels.nr; i++)\n-\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), dels.items[i].string);\n+\t\t\twrite_name(dry_run ? _(msg_would_remove) : _(msg_remove), dels.items[i].string, nul_terminated);\n \t}\n \tstring_list_clear(&dels, 0);\n \treturn ret;\n@@ -146,7 +156,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i, res;\n \tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n-\tint ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n+\tint ignored_only = 0, config_set = 0, errors = 0, gone = 1, nul_terminated = 0;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n \tstruct strbuf directory = STRBUF_INIT;\n \tstruct dir_struct dir;\n@@ -167,6 +177,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN('x', NULL, &ignored, N_(\"remove ignored files, too\")),\n \t\tOPT_BOOLEAN('X', NULL, &ignored_only,\n \t\t\t\tN_(\"remove only ignored files\")),\n+\t\tOPT_BOOLEAN('z', NULL, &nul_terminated, \"(actually or to be) removed paths are separated with NUL character\"),\n \t\tOPT_END()\n \t};\n \n@@ -259,7 +270,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tif (S_ISDIR(st.st_mode)) {\n \t\t\tstrbuf_addstr(&directory, ent->name);\n \t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n-\t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n+\t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone, nul_terminated))\n \t\t\t\t\terrors++;\n \t\t\t\tif (gone && !quiet) {\n \t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n@@ -277,7 +288,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n \t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n-\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n+\t\t\t\twrite_name(dry_run ? _(msg_would_remove) : _(msg_remove), qname, nul_terminated);\n \t\t\t}\n \t\t}\n \t}\n-- \n1.8.3.rc0.297.g8c90ec5\n"},{"id":"215853","messageId":"vpqy5c1xuof.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"6733a9080da07ace27c3252de2399be339b4e349.1367245712.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] clean: Introduce -z for machine readable output","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-29T14:45:04Z","receivedAt":"2013-04-29T14:45:04Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Michael J Gruber writes:\n\n> -z makes clean output only the names of paths which are or would be\n> deleted, and separates them with \\0.\n\nMy first reaction was: Is it not a job for \"git ls-files\"?\n\nActually, you already almost have it:\n\ngit clean -d => git ls-files --exclude-standard --directory -o\ngit clean -dx => git ls-files -z --exclude-standard --directory -io\n\nOTOH, the \"git clean\" without -d are not exposed directly AFAICT, and\nthe set of options of \"git clean\" may make more sense.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"215861","messageId":"CANYiYbHKWv6R2vtwG=bTNhj8q0iC4EBt8usC3posBCtYBTXOvA@mail.gmail.com","threadId":"33614","inReplyTo":"vpqwqrl931e.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2] Add support for -i/--interactive to git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-04-29T16:15:58Z","receivedAt":"2013-04-29T16:15:58Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/4/29 Matthieu Moy :\n> Jiang Xin writes:\n>\n>> Show what would be done and a confirmation dialog before actually\n>> cleaning. In the confirmation dialog, the user can input a space\n>> separated prefix list, and each clean candidate that matches with\n>> one of prefix, will be excluded from cleaning.\n>\n> That seems a really weird interface. In particular, that means people\n> typing \"no\" or \"n\" mechanically will have everything not starting with\n> \"n\" or \"no\" deleted. We've learnt from the \"git send-email\" interface\n> that people (including me ;-) ) do type \"yes\" mechanically to some\n> questions.\n\nBecause of this (typing \"no\" or \"n\" not stop the whole cleaning),\nI write a loop, only when user press Enter (input nothing), begin\nto clean files/directories, and there would be a warning if nothing\nexcluded by the prefix/pattern.\n\nThat means when user input \"n\" or \"no\", the interactive git-clean\nwill not delete entries starting with \"n\" or \"no\" (instead of delete\nnothing), but it still give the user a hint what happened. It will\nshow the filtered cleaning list or a warning message that there\nare no entries starting with \"n\" or \"no\" to be filtered out (may be\nthe warning messages should be cached and displayed in\nthe end, not at the beginning).\n\n>\n> My feeling is that you're doing a bad compromise between a confirmation\n> dialog (y/n) and a really interactive command (like \"git add -i\") with a\n> rich interface.\n\nIt's not confirmation, but a last chance to save your cleaning files.\nThe use can input a space-seperated prefixes list, but I think most\npeople would like to input a space-seperated patterns (like .gitignore).\nI try to implement like this, but still need sometime to refector the code.\n\n+ el = add_exclude_list(&dir, EXC_CMDL,\n\"exclude by interactive git clean\");\n+ add_exclude((*prefix_list)->buf, \"\",\n0, el, -(++i));\n\ngit-clean is a simple task, I doubt write a rich interface has no much value,\nand make things complicated. But render prompt, error outputs in different\ncolor like interactive git-add is interesting.\n\n\n>> @@ -34,7 +34,17 @@ OPTIONS\n>> -f::\n>> --force::\n>> If the Git configuration variable clean.requireForce is not set\n>> - to false, 'git clean' will refuse to run unless given -f or -n.\n>> + to false, 'git clean' will refuse to run unless given -f, -n or\n>> + -i.\n>> +\n>> +-i::\n>> +--interactive::\n>> + Show what would be done and a confirmation dialog before actually\n>> + cleaning. In the confirmation dialog, the user can input a space\n>> + separated prefix list, and each clean candidate that matches with\n>> + one of prefix, will be excluded from cleaning. When the user feels\n>> + it's OK, press ENTER to start cleaning. If the user wants to cancel\n>> + the whole cleaning, simply input ctrl-c in the confirmation dialog.\n>\n> Broken indentation. It seems you've set your tab-width to 2, in which\n> case you can't see it, but you've indented with 2 spaces where the rest\n> of the file is indented with tabs.\n\nOops, I code lots of ruby recently, and vim is not smart to handle txt file.\nI will correct them.\n\n>> if (S_ISDIR(st.st_mode)) {\n>> - strbuf_addstr(&directory, ent->name);\n>> if (remove_directories || (matches == MATCHED_EXACTLY)) {\n>> - if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n>> - errors++;\n>> - if (gone && !quiet) {\n>> - qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n>> - printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n>> - }\n>> + string_list_append(&dels, ent->name);\n>\n> The patch would be much easier to read if split into a first refactoring\n> patch that would introduce this \"dels\" list, and a second patch that\n> would introduce -i. Reading this, I wondered why the code was removed,\n> while it was actually just moved.\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n\n\n-- \nJiang Xin\n"},{"id":"216046","messageId":"3ecc9ca1b1363b5bd27ae53cbf5899ce6d44cd48.1367349734.git.worldhello.net@gmail.com","threadId":"33614","inReplyTo":"CANYiYbHKWv6R2vtwG=bTNhj8q0iC4EBt8usC3posBCtYBTXOvA@mail.gmail.com","subject":"[PATCH v3] Add support for -i/--interactive to git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-04-30T19:25:11Z","receivedAt":"2013-04-30T19:25:11Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Show what would be done and the user must confirm before actually\ncleaning. In the confirmation dialog, the user has three choices:\n\n * Yes: Start to do cleaning.\n * No: Nothing will be deleted.\n * Edit (default): Enter edit mode.\n\nWhen the user chooses the edit mode, the user can input space-\nseparated patterns (the same syntax as gitignore), and each clean\ncandidate that matches with one of the patterns will be excluded\nfrom cleaning. When the user feels it's OK, presses ENTER to start\ncleaning.\n\nWhen in the edit mode, if the user wants to cancel the whole\ncleaning, simply inputs ctrl-c to abort the cleaning.\n\nSigned-off-by: Jiang Xin \nSuggested-by: Junio C Hamano \nReviewed-by: Eric Sunshine \nReviewed-by: Matthieu Moy \n---\n Documentation/git-clean.txt | 16 ++++-\n builtin/clean.c | 141 +++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 140 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt\nindex bdc3a..6bc99 100644\n--- a/Documentation/git-clean.txt\n+++ b/Documentation/git-clean.txt\n@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n SYNOPSIS\n --------\n [verse]\n-'git clean' [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\n+'git clean' [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\n \n DESCRIPTION\n -----------\n@@ -34,7 +34,19 @@ OPTIONS\n -f::\n --force::\n \tIf the Git configuration variable clean.requireForce is not set\n-\tto false, 'git clean' will refuse to run unless given -f or -n.\n+\tto false, 'git clean' will refuse to run unless given -f, -n or\n+\t-i.\n+\n+-i::\n+--interactive::\n+\tShow what would be done and the user must confirm before actually\n+\tcleaning. In the confirmation dialog, the user can choose to abort\n+\tthe cleaning, or enter into an edit mode. In the edit mode, the\n+\tuser can input space-separated patterns (the same syntax as\n+\tgitignore), and each clean candidate that matches with one of the\n+\tpatterns will be excluded from cleaning. When the user feels it's\n+\tOK, presses ENTER to start cleaning. If the user wants to cancel\n+\tthe whole cleaning in the edit mode, simply inputs ctrl-c to abort.\n \n -n::\n --dry-run::\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 04e39..b26c0 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -15,9 +15,10 @@\n #include \"quote.h\"\n \n static int force = -1; /* unset */\n+static int interactive;\n \n static const char *const builtin_clean_usage[] = {\n-\tN_(\"git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n+\tN_(\"git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n \tNULL\n };\n \n@@ -142,6 +143,96 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \treturn ret;\n }\n \n+void interactive_clean(struct string_list *dels, const char *prefix)\n+{\n+\tstruct dir_struct dir;\n+\tstruct strbuf confirm = STRBUF_INIT;\n+\tstruct strbuf message = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf **ignore_list;\n+\tstruct string_list_item *item;\n+\tstruct exclude_list *el;\n+\tconst char *qname;\n+\tint edit_mode = 0, i;\n+\n+\twhile (1) {\n+\t\tint matches = 0;\n+\n+\t\t/* dels list may become empty when we run string_list_remove_empty_items later */\n+\t\tif (!dels->nr)\n+\t\t\tbreak;\n+\n+\t\tfor_each_string_list_item(item, dels) {\n+\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\t\tprintf(_(msg_would_remove), qname);\n+\t\t}\n+\n+\t\tif (message.len) {\n+\t\t\tprintf(\"\\n%s\\n\", message.buf);\n+\t\t\tstrbuf_reset(&message);\n+\t\t}\n+\n+\t\tif (!edit_mode) {\n+\t\t\tprintf(_(\"Remove (yes/no/Edit) ? \"));\n+\t\t\tstrbuf_getline(&confirm, stdin, '\\n');\n+\t\t\tstrbuf_trim(&confirm);\n+\t\t\tif (confirm.len) {\n+\t\t\t\tif (!strncasecmp(confirm.buf, \"yes\", confirm.len)) {\n+\t\t\t\t\tbreak;\n+\t\t\t\t} else if (!strncasecmp(confirm.buf, \"no\", confirm.len)) {\n+\t\t\t\t\tstring_list_clear(dels, 0);\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tedit_mode = 1;\n+\t\t}\n+\n+\t\tprintf(_(\"Input ignore patterns to keep items, or press ENTER to confirm: \"));\n+\t\tstrbuf_getline(&confirm, stdin, '\\n');\n+\t\tstrbuf_trim(&confirm);\n+\t\tprintf(\"\\n\");\n+\n+\t\tif (!confirm.len)\n+\t\t\tbreak;\n+\n+\t\tmemset(&dir, 0, sizeof(dir));\n+\t\tel = add_exclude_list(&dir, EXC_CMDL, \"manual exclude\");\n+\t\tignore_list = strbuf_split_buf(confirm.buf, confirm.len, ' ', 0);\n+\n+\t\tfor (i = 0; ignore_list[i]; i++)\n+\t\t{\n+\n+\t\t\tstrbuf_trim(*ignore_list);\n+\t\t\tif (!(*ignore_list)->len)\n+\t\t\t\tcontinue;\n+\n+\t\t\tadd_exclude(ignore_list[i]->buf, \"\", 0, el, -(i+1));\n+\t\t}\n+\n+\t\tfor_each_string_list_item(item, dels) {\n+\t\t\tint dtype = DT_UNKNOWN;\n+\t\t\tif (is_excluded(&dir, item->string, &dtype)) {\n+\t\t\t\t*item->string = '\\0';\n+\t\t\t\tmatches++;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (!matches) {\n+\t\t\tstrbuf_addf(&message, _(\"WARNING: Cannot find items prefixed by: %s\"), confirm.buf);\n+\t\t\tstrbuf_addch(&message, '\\n');\n+\t\t} else {\n+\t\t\tstring_list_remove_empty_items(dels, 0);\n+\t\t}\n+\n+\t\tstrbuf_reset(&confirm);\n+\t\tstrbuf_list_free(ignore_list);\n+\t\tclear_directory(&dir);\n+\t}\n+\n+\tstrbuf_release(&confirm);\n+\tstrbuf_release(&message);\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i, res;\n@@ -154,12 +245,15 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n \tstruct exclude_list *el;\n+\tstruct string_list dels = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *item;\n \tconst char *qname;\n \tchar *seen = NULL;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"do not print names of files removed\")),\n \t\tOPT__DRY_RUN(&dry_run, N_(\"dry run\")),\n \t\tOPT__FORCE(&force, N_(\"force\")),\n+\t\tOPT_BOOL('i', \"interactive\", &interactive, N_(\"interactive cleaning\")),\n \t\tOPT_BOOLEAN('d', NULL, &remove_directories,\n \t\t\t\tN_(\"remove whole directories\")),\n \t\t{ OPTION_CALLBACK, 'e', \"exclude\", &exclude_list, N_(\"pattern\"),\n@@ -186,12 +280,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (ignored && ignored_only)\n \t\tdie(_(\"-x and -X cannot be used together\"));\n \n-\tif (!dry_run && !force) {\n+\tif (!dry_run && !force && !interactive) {\n \t\tif (config_set)\n-\t\t\tdie(_(\"clean.requireForce set to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce set to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t\telse\n-\t\t\tdie(_(\"clean.requireForce defaults to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce defaults to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t}\n \n@@ -257,26 +351,42 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tstrbuf_addstr(&directory, ent->name);\n \t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n-\t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n-\t\t\t\t\terrors++;\n-\t\t\t\tif (gone && !quiet) {\n-\t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n-\t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n-\t\t\t\t}\n+\t\t\t\tstring_list_append(&dels, ent->name);\n \t\t\t}\n-\t\t\tstrbuf_reset(&directory);\n \t\t} else {\n \t\t\tif (pathspec && !matches)\n \t\t\t\tcontinue;\n-\t\t\tres = dry_run ? 0 : unlink(ent->name);\n+\t\t\tstring_list_append(&dels, ent->name);\n+\t\t}\n+\t}\n+\n+\tif (interactive && dels.nr > 0 && !dry_run && isatty(0) && isatty(1))\n+\t\tinteractive_clean(&dels, prefix);\n+\n+\tfor_each_string_list_item(item, &dels) {\n+\t\tstruct stat st;\n+\n+\t\tif (lstat(item->string, &st))\n+\t\t\tcontinue;\n+\n+\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\tstrbuf_addstr(&directory, item->string);\n+\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n+\t\t\t\terrors++;\n+\t\t\tif (gone && !quiet) {\n+\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n+\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n+\t\t\t}\n+\t\t\tstrbuf_reset(&directory);\n+\t\t} else {\n+\t\t\tres = dry_run ? 0 : unlink(item->string);\n \t\t\tif (res) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n \t\t\t\twarning(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\n@@ -285,5 +395,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tstrbuf_release(&directory);\n \tstring_list_clear(&exclude_list, 0);\n+\tstring_list_clear(&dels, 0);\n \treturn (errors != 0);\n }\n-- \n1.8.3.rc0.306.gee4c747\n"},{"id":"216146","messageId":"vpq38u6n397.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"3ecc9ca1b1363b5bd27ae53cbf5899ce6d44cd48.1367349734.git.worldhello.net@gmail.com","subject":"Re: [PATCH v3] Add support for -i/--interactive to git-clean","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-05-01T15:12:04Z","receivedAt":"2013-05-01T15:12:04Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> Show what would be done and the user must confirm before actually\n> cleaning. In the confirmation dialog, the user has three choices:\n>\n> * Yes: Start to do cleaning.\n> * No: Nothing will be deleted.\n> * Edit (default): Enter edit mode.\n\nI like this much more than the previous one. I played with it a bit, and\nfound it much more pleasant than \"rm -i\": by default, only one querry,\nbut still an option to select which files to clean.\n\nI'm wondering whether \"Enter\" in the edit mode should return to the\nyes/no/Edit querry instead of applying the clean. It would make it clear\nfor the user that it's still possible to cancel completely (the\nControl-C hint is not visible in the UI otherwise).\n\n> Reviewed-by: Matthieu Moy \n\nPlease, no. I already mentionned it in my previous patch, but I did not\nreview the patch. See SubmittingPatches:\n\n 3. \"Reviewed-by:\", unlike the other tags, can only be offered by the\n reviewer and means that she is completely satisfied that the patch\n is ready for application. It is usually offered only after a\n detailed review.\n\nCommenting != reviewing.\n\n> +\t\t/* dels list may become empty when we run string_list_remove_empty_items later */\n> +\t\tif (!dels->nr)\n> +\t\t\tbreak;\n\nThis happens when the user removed everything from the list in the edit\nmode. This could print something before breaking (and then exiting\nsilently). Maybe \"No more files to clean, exiting.\" or so.\n\n> +\t\t\tprintf(_(\"Remove (yes/no/Edit) ? \"));\n> +\t\t\tstrbuf_getline(&confirm, stdin, '\\n');\n> +\t\t\tstrbuf_trim(&confirm);\n> +\t\t\tif (confirm.len) {\n> +\t\t\t\tif (!strncasecmp(confirm.buf, \"yes\", confirm.len)) {\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t} else if (!strncasecmp(confirm.buf, \"no\", confirm.len)) {\n> +\t\t\t\t\tstring_list_clear(dels, 0);\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\tedit_mode = 1;\n\nIt's weird that anything but \"yes\" and \"no\" enter the edit mode without\ncomplaining. It's safe, but surprising. If I type \"foo\", I'd rather get\nan error and be asked again.\n\n> +\t\tif (!matches) {\n> +\t\t\tstrbuf_addf(&message, _(\"WARNING: Cannot find items prefixed by: %s\"), confirm.buf);\n\n\"prefixed\" seems a remainder of the previous version of the patch. You\nprobably mean \"matched by: %s\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"216247","messageId":"6e795640952cad0eb25cb0db4243b2712f11e957.1367500374.git.worldhello.net@gmail.com","threadId":"33614","inReplyTo":"vpq38u6n397.fsf@grenoble-inp.fr","subject":"[PATCH v4 1/3] Add support for -i/--interactive to git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-05-02T13:15:02Z","receivedAt":"2013-05-02T13:15:02Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Show what would be done and the user must confirm before actually\ncleaning. In the confirmation dialog, the user has three choices:\n\n * Yes: Start to do cleaning.\n * No: Nothing will be deleted.\n * Edit (default for the first time): Enter edit mode.\n\nWhen the user chooses the edit mode, the user can input space-\nseparated patterns (the same syntax as gitignore), and each clean\ncandidate that matches with one of the patterns will be excluded\nfrom cleaning. When the user feels it's OK, presses ENTER and back\nto the confirmation dialog.\n\nSigned-off-by: Jiang Xin \nSuggested-by: Junio C Hamano \nSpelling-check-by: Eric Sunshine \nComments-by: Matthieu Moy \n---\n Documentation/git-clean.txt | 15 +++-\n builtin/clean.c | 183 ++++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 181 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt\nindex bdc3a..f5572 100644\n--- a/Documentation/git-clean.txt\n+++ b/Documentation/git-clean.txt\n@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n SYNOPSIS\n --------\n [verse]\n-'git clean' [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\n+'git clean' [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\n \n DESCRIPTION\n -----------\n@@ -34,7 +34,18 @@ OPTIONS\n -f::\n --force::\n \tIf the Git configuration variable clean.requireForce is not set\n-\tto false, 'git clean' will refuse to run unless given -f or -n.\n+\tto false, 'git clean' will refuse to run unless given -f, -n or\n+\t-i.\n+\n+-i::\n+--interactive::\n+\tShow what would be done and the user must confirm before actually\n+\tcleaning. In the confirmation dialog, the user can choose to abort\n+\tthe cleaning, or enter into an edit mode. In the edit mode, the\n+\tuser can input space-separated patterns (the same syntax as\n+\tgitignore), and each clean candidate that matches with one of the\n+\tpatterns will be excluded from cleaning. When the user feels it's\n+\tOK, presses ENTER and back to the confirmation dialog.\n \n -n::\n --dry-run::\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 04e39..407744e5 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -15,9 +15,10 @@\n #include \"quote.h\"\n \n static int force = -1; /* unset */\n+static int interactive;\n \n static const char *const builtin_clean_usage[] = {\n-\tN_(\"git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n+\tN_(\"git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n \tNULL\n };\n \n@@ -142,6 +143,138 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \treturn ret;\n }\n \n+void interactive_clean_edit(struct string_list *dels, const char *prefix)\n+{\n+\tstruct dir_struct dir;\n+\tstruct strbuf confirm = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf **ignore_list;\n+\tstruct string_list_item *item;\n+\tstruct exclude_list *el;\n+\tconst char *qname;\n+\tint changed = -1, i;\n+\n+\twhile (1) {\n+\t\t/* dels list may become empty when we run string_list_remove_empty_items later */\n+\t\tif (!dels->nr) {\n+\t\t\tprintf_ln(_(\"No more files to clean, exiting.\"));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tif (changed) {\n+\t\t\tprintf(_(\n+\t\t\t\t \"NOTE: Will remove the following items. You can input space-seperated\\n\"\n+\t\t\t\t \"NOTE: patterns (just like .gitignore) to exclude items from deletion,\\n\"\n+\t\t\t\t \"NOTE: or press ENTER to continue.\"\n+\t\t\t\t));\n+\t\t\tputchar('\\n');\n+\t\t\tputchar('\\n');\n+\n+\t\t\t/* Display dels in \"Would remove ...\" format */\n+\t\t\tfor_each_string_list_item(item, dels) {\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\t\t\tprintf(_(msg_would_remove), qname);\n+\t\t\t}\n+\t\t\tputchar('\\n');\n+\t\t}\n+\n+\t\tprintf(_(\"Input ignore patterns> \"));\n+\t\tstrbuf_getline(&confirm, stdin, '\\n');\n+\t\tstrbuf_trim(&confirm);\n+\t\tputchar('\\n');\n+\n+\t\t/* Quit edit mode */\n+\t\tif (!confirm.len)\n+\t\t\tbreak;\n+\n+\t\tmemset(&dir, 0, sizeof(dir));\n+\t\tel = add_exclude_list(&dir, EXC_CMDL, \"manual exclude\");\n+\t\tignore_list = strbuf_split_buf(confirm.buf, confirm.len, ' ', 0);\n+\n+\t\tfor (i = 0; ignore_list[i]; i++) {\n+\t\t\tstrbuf_trim(*ignore_list);\n+\t\t\tif (!(*ignore_list)->len)\n+\t\t\t\tcontinue;\n+\n+\t\t\tadd_exclude(ignore_list[i]->buf, \"\", 0, el, -(i+1));\n+\t\t}\n+\n+\t\tchanged = 0;\n+\t\tfor_each_string_list_item(item, dels) {\n+\t\t\tint dtype = DT_UNKNOWN;\n+\t\t\tconst char *qname;\n+\n+\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\n+\t\t\tif (is_excluded(&dir, qname, &dtype)) {\n+\t\t\t\t*item->string = '\\0';\n+\t\t\t\tchanged++;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (changed) {\n+\t\t\tstring_list_remove_empty_items(dels, 0);\n+\t\t} else {\n+\t\t\tprintf(_(\"WARNING: Cannot find items matched by: %s\"), confirm.buf);\n+\t\t\tputchar('\\n');\n+\t\t}\n+\n+\t\tstrbuf_list_free(ignore_list);\n+\t\tclear_directory(&dir);\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&confirm);\n+}\n+\n+void interactive_clean(struct string_list *dels, const char *prefix)\n+{\n+\tstruct strbuf confirm = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list_item *item;\n+\tconst char *qname;\n+\tint count = 0;\n+\n+\t/* dels list may become empty after return back from edit mode */\n+\twhile (dels->nr) {\n+\t\t/* Display dels in \"Would remove ...\" format */\n+\t\tfor_each_string_list_item(item, dels) {\n+\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\t\tprintf(_(msg_would_remove), qname);\n+\t\t}\n+\t\tputchar('\\n');\n+\n+\t\t/* Confirmation dialog */\n+\t\tprintf(count > 0 ? _(\"Remove (Yes/no/edit) ? \") : _(\"Remove (yes/no/Edit) ? \"));\n+\t\tstrbuf_getline(&confirm, stdin, '\\n');\n+\t\tstrbuf_trim(&confirm);\n+\t\tputchar('\\n');\n+\n+\t\tif (confirm.len) {\n+\t\t\tif (!strncasecmp(confirm.buf, \"yes\", confirm.len)) {\n+\t\t\t\tbreak;\n+\t\t\t} else if (!strncasecmp(confirm.buf, \"no\", confirm.len)) {\n+\t\t\t\tstring_list_clear(dels, 0);\n+\t\t\t\tbreak;\n+\t\t\t} else if (!strncasecmp(confirm.buf, \"edit\", confirm.len)) {\n+\t\t\t\tinteractive_clean_edit(dels, prefix);\n+\t\t\t} else {\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t} else if (count > 0) {\n+\t\t\t/* If back from edit_mode, confirmation dialog defaults to \"yes\" */\n+\t\t\tbreak;\n+\t\t} else {\n+\t\t\t/* For the first time, confirmation dialog defaults to \"edit\" */\n+\t\t\tinteractive_clean_edit(dels, prefix);\n+\t\t}\n+\t\tcount++;\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&confirm);\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i, res;\n@@ -154,12 +287,15 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n \tstruct exclude_list *el;\n+\tstruct string_list dels = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *item;\n \tconst char *qname;\n \tchar *seen = NULL;\n \tstruct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"do not print names of files removed\")),\n \t\tOPT__DRY_RUN(&dry_run, N_(\"dry run\")),\n \t\tOPT__FORCE(&force, N_(\"force\")),\n+\t\tOPT_BOOL('i', \"interactive\", &interactive, N_(\"interactive cleaning\")),\n \t\tOPT_BOOLEAN('d', NULL, &remove_directories,\n \t\t\t\tN_(\"remove whole directories\")),\n \t\t{ OPTION_CALLBACK, 'e', \"exclude\", &exclude_list, N_(\"pattern\"),\n@@ -186,12 +322,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (ignored && ignored_only)\n \t\tdie(_(\"-x and -X cannot be used together\"));\n \n-\tif (!dry_run && !force) {\n+\tif (!dry_run && !force && !interactive) {\n \t\tif (config_set)\n-\t\t\tdie(_(\"clean.requireForce set to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce set to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t\telse\n-\t\t\tdie(_(\"clean.requireForce defaults to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce defaults to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t}\n \n@@ -257,26 +393,42 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tstrbuf_addstr(&directory, ent->name);\n \t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n-\t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n-\t\t\t\t\terrors++;\n-\t\t\t\tif (gone && !quiet) {\n-\t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n-\t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n-\t\t\t\t}\n+\t\t\t\tstring_list_append(&dels, ent->name);\n \t\t\t}\n-\t\t\tstrbuf_reset(&directory);\n \t\t} else {\n \t\t\tif (pathspec && !matches)\n \t\t\t\tcontinue;\n-\t\t\tres = dry_run ? 0 : unlink(ent->name);\n+\t\t\tstring_list_append(&dels, ent->name);\n+\t\t}\n+\t}\n+\n+\tif (interactive && dels.nr > 0 && !dry_run && isatty(0) && isatty(1))\n+\t\tinteractive_clean(&dels, prefix);\n+\n+\tfor_each_string_list_item(item, &dels) {\n+\t\tstruct stat st;\n+\n+\t\tif (lstat(item->string, &st))\n+\t\t\tcontinue;\n+\n+\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\tstrbuf_addstr(&directory, item->string);\n+\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n+\t\t\t\terrors++;\n+\t\t\tif (gone && !quiet) {\n+\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n+\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n+\t\t\t}\n+\t\t\tstrbuf_reset(&directory);\n+\t\t} else {\n+\t\t\tres = dry_run ? 0 : unlink(item->string);\n \t\t\tif (res) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n \t\t\t\twarning(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\n@@ -285,5 +437,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \n \tstrbuf_release(&directory);\n \tstring_list_clear(&exclude_list, 0);\n+\tstring_list_clear(&dels, 0);\n \treturn (errors != 0);\n }\n-- \n1.8.3.rc0.364.gbb5463f\n"},{"id":"216248","messageId":"3b80db3d2e8f31b1d9373406cce3cbdee2094a29.1367500374.git.worldhello.net@gmail.com","threadId":"33614","inReplyTo":"6e795640952cad0eb25cb0db4243b2712f11e957.1367500374.git.worldhello.net@gmail.com","subject":"[PATCH v4 2/3] Show items of interactive git-clean in columns","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-05-02T13:15:03Z","receivedAt":"2013-05-02T13:15:03Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Signed-off-by: Jiang Xin \n---\n builtin/clean.c | 63 +++++++++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 46 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 407744e5..ac48e 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -13,9 +13,11 @@\n #include \"refs.h\"\n #include \"string-list.h\"\n #include \"quote.h\"\n+#include \"column.h\"\n \n static int force = -1; /* unset */\n static int interactive;\n+static unsigned int colopts;\n \n static const char *const builtin_clean_usage[] = {\n \tN_(\"git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n@@ -30,8 +32,12 @@ static const char *msg_warn_remove_failed = N_(\"failed to remove %s\");\n \n static int git_clean_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"clean.requireforce\"))\n+\tif (!prefixcmp(var, \"column.\"))\n+\t\treturn git_column_config(var, value, \"clean\", &colopts);\n+\tif (!strcmp(var, \"clean.requireforce\")) {\n \t\tforce = !git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -143,6 +149,34 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \treturn ret;\n }\n \n+void pretty_print_dels(struct string_list *dels, const char *prefix)\n+{\n+\tstruct string_list list = STRING_LIST_INIT_DUP;\n+\tstruct string_list_item *item;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *qname;\n+\tstruct column_options copts;\n+\n+\tfor_each_string_list_item(item, dels) {\n+\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n+\t\tstring_list_append(&list, qname);\n+\t}\n+\n+\t/*\n+\t * always enable column display, we only consult column.*\n+\t * about layout strategy and stuff\n+\t */\n+\tcolopts = (colopts & ~COL_ENABLE_MASK) | COL_ENABLED;\n+\tmemset(&copts, 0, sizeof(copts));\n+\tcopts.indent = \" \";\n+\tcopts.padding = 2;\n+\tprint_columns(&list, colopts, &copts);\n+\n+\tputchar('\\n');\n+\tstrbuf_release(&buf);\n+\tstring_list_clear(&list, 0);\n+}\n+\n void interactive_clean_edit(struct string_list *dels, const char *prefix)\n {\n \tstruct dir_struct dir;\n@@ -151,7 +185,6 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)\n \tstruct strbuf **ignore_list;\n \tstruct string_list_item *item;\n \tstruct exclude_list *el;\n-\tconst char *qname;\n \tint changed = -1, i;\n \n \twhile (1) {\n@@ -170,12 +203,8 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)\n \t\t\tputchar('\\n');\n \t\t\tputchar('\\n');\n \n-\t\t\t/* Display dels in \"Would remove ...\" format */\n-\t\t\tfor_each_string_list_item(item, dels) {\n-\t\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n-\t\t\t\tprintf(_(msg_would_remove), qname);\n-\t\t\t}\n-\t\t\tputchar('\\n');\n+\t\t\t/* Display dels in columns */\n+\t\t\tpretty_print_dels(dels, prefix);\n \t\t}\n \n \t\tprintf(_(\"Input ignore patterns> \"));\n@@ -230,20 +259,21 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)\n void interactive_clean(struct string_list *dels, const char *prefix)\n {\n \tstruct strbuf confirm = STRBUF_INIT;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tstruct string_list_item *item;\n-\tconst char *qname;\n \tint count = 0;\n \n \t/* dels list may become empty after return back from edit mode */\n \twhile (dels->nr) {\n-\t\t/* Display dels in \"Would remove ...\" format */\n-\t\tfor_each_string_list_item(item, dels) {\n-\t\t\tqname = quote_path_relative(item->string, -1, &buf, prefix);\n-\t\t\tprintf(_(msg_would_remove), qname);\n-\t\t}\n+\t\tprintf_ln(_(\n+\t\t\t \"WARNING: The following items will be removed permanently. Press \\\"y\\\"\\n\"\n+\t\t\t \"WARNING: to start cleaning, and press \\\"n\\\" to abort the cleaning.\\n\"\n+\t\t\t \"WARNING: You can also enter the \\\"edit\\\" mode, and select items\\n\"\n+\t\t\t \"WARNING: to be excluded from the cleaning.\"\n+\t\t\t ));\n \t\tputchar('\\n');\n \n+\t\t/* Display dels in columns */\n+\t\tpretty_print_dels(dels, prefix);\n+\n \t\t/* Confirmation dialog */\n \t\tprintf(count > 0 ? _(\"Remove (Yes/no/edit) ? \") : _(\"Remove (yes/no/Edit) ? \"));\n \t\tstrbuf_getline(&confirm, stdin, '\\n');\n@@ -271,7 +301,6 @@ void interactive_clean(struct string_list *dels, const char *prefix)\n \t\tcount++;\n \t}\n \n-\tstrbuf_release(&buf);\n \tstrbuf_release(&confirm);\n }\n \n-- \n1.8.3.rc0.364.gbb5463f\n"},{"id":"216249","messageId":"bb5463fa4ef09df08ad3bf5abf4660a5df414e64.1367500374.git.worldhello.net@gmail.com","threadId":"33614","inReplyTo":"3b80db3d2e8f31b1d9373406cce3cbdee2094a29.1367500374.git.worldhello.net@gmail.com","subject":"[PATCH v4 3/3] Add colors to interactive git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-05-02T13:15:04Z","receivedAt":"2013-05-02T13:15:04Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Show help, error messages, and prompt in colors for interactive\ngit-clean. Re-use config variables from git-add--interactive:\n\n * color.interactive: When set to always, always use colors for\n interactive prompts and displays. When false (or never),\n never. When set to true or auto, use colors only when the\n output is to the terminal.\n\n * color.interactive.: Use customized color for interactive\n git-clean output (like git add --interactive). may be\n prompt, header, help or error.\n\nSigned-off-by: Jiang Xin \n---\n builtin/clean.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 70 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex ac48e..c85b 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -14,6 +14,7 @@\n #include \"string-list.h\"\n #include \"quote.h\"\n #include \"column.h\"\n+#include \"color.h\"\n \n static int force = -1; /* unset */\n static int interactive;\n@@ -30,15 +31,71 @@ static const char *msg_skip_git_dir = N_(\"Skipping repository %s\\n\");\n static const char *msg_would_skip_git_dir = N_(\"Would skip repository %s\\n\");\n static const char *msg_warn_remove_failed = N_(\"failed to remove %s\");\n \n+static int clean_use_color = -1;\n+static char clean_colors[][COLOR_MAXLEN] = {\n+\tGIT_COLOR_RESET,\n+\tGIT_COLOR_NORMAL,\t/* PLAIN */\n+\tGIT_COLOR_BOLD_BLUE,\t/* PROMPT */\n+\tGIT_COLOR_BOLD_RED,\t/* HELP */\n+\tGIT_COLOR_BOLD_RED,\t/* ERROR */\n+};\n+enum color_clean {\n+\tCLEAN_COLOR_RESET = 0,\n+\tCLEAN_COLOR_PLAIN = 1,\n+\tCLEAN_COLOR_PROMPT = 2,\n+\tCLEAN_COLOR_HELP = 3,\n+\tCLEAN_COLOR_ERROR = 4\n+};\n+\n+static int parse_clean_color_slot(const char *var, int ofs)\n+{\n+\tif (!strcasecmp(var+ofs, \"reset\"))\n+\t\treturn CLEAN_COLOR_RESET;\n+\tif (!strcasecmp(var+ofs, \"plain\"))\n+\t\treturn CLEAN_COLOR_PLAIN;\n+\tif (!strcasecmp(var+ofs, \"prompt\"))\n+\t\treturn CLEAN_COLOR_PROMPT;\n+\tif (!strcasecmp(var+ofs, \"help\"))\n+\t\treturn CLEAN_COLOR_HELP;\n+\tif (!strcasecmp(var+ofs, \"error\"))\n+\t\treturn CLEAN_COLOR_ERROR;\n+\treturn -1;\n+}\n+\n static int git_clean_config(const char *var, const char *value, void *cb)\n {\n \tif (!prefixcmp(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"clean\", &colopts);\n+\tif (!strcmp(var, \"color.interactive\")) {\n+\t\tclean_use_color = git_config_colorbool(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!prefixcmp(var, \"color.interactive.\")) {\n+\t\tint slot = parse_clean_color_slot(var, 18);\n+\t\tif (slot < 0)\n+\t\t\treturn 0;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tcolor_parse(value, var, clean_colors[slot]);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"clean.requireforce\")) {\n \t\tforce = !git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n+}\n+\n+static const char *clean_get_color(enum color_clean ix)\n+{\n+\tif (want_color(clean_use_color))\n+\t\treturn clean_colors[ix];\n+\treturn \"\";\n+}\n+\n+static void clean_print_color(enum color_clean ix)\n+{\n+\tprintf(\"%s\", clean_get_color(ix));\n }\n \n static int exclude_cb(const struct option *opt, const char *arg, int unset)\n@@ -190,16 +247,20 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)\n \twhile (1) {\n \t\t/* dels list may become empty when we run string_list_remove_empty_items later */\n \t\tif (!dels->nr) {\n+\t\t\tclean_print_color(CLEAN_COLOR_ERROR);\n \t\t\tprintf_ln(_(\"No more files to clean, exiting.\"));\n+\t\t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\t\tbreak;\n \t\t}\n \n \t\tif (changed) {\n+\t\t\tclean_print_color(CLEAN_COLOR_HELP);\n \t\t\tprintf(_(\n \t\t\t\t \"NOTE: Will remove the following items. You can input space-seperated\\n\"\n \t\t\t\t \"NOTE: patterns (just like .gitignore) to exclude items from deletion,\\n\"\n \t\t\t\t \"NOTE: or press ENTER to continue.\"\n \t\t\t\t));\n+\t\t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\t\tputchar('\\n');\n \t\t\tputchar('\\n');\n \n@@ -207,7 +268,9 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)\n \t\t\tpretty_print_dels(dels, prefix);\n \t\t}\n \n+\t\tclean_print_color(CLEAN_COLOR_PROMPT);\n \t\tprintf(_(\"Input ignore patterns> \"));\n+\t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\tstrbuf_getline(&confirm, stdin, '\\n');\n \t\tstrbuf_trim(&confirm);\n \t\tputchar('\\n');\n@@ -244,7 +307,9 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)\n \t\tif (changed) {\n \t\t\tstring_list_remove_empty_items(dels, 0);\n \t\t} else {\n+\t\t\tclean_print_color(CLEAN_COLOR_ERROR);\n \t\t\tprintf(_(\"WARNING: Cannot find items matched by: %s\"), confirm.buf);\n+\t\t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\t\tputchar('\\n');\n \t\t}\n \n@@ -263,6 +328,7 @@ void interactive_clean(struct string_list *dels, const char *prefix)\n \n \t/* dels list may become empty after return back from edit mode */\n \twhile (dels->nr) {\n+\t\tclean_print_color(CLEAN_COLOR_HELP);\n \t\tprintf_ln(_(\n \t\t\t \"WARNING: The following items will be removed permanently. Press \\\"y\\\"\\n\"\n \t\t\t \"WARNING: to start cleaning, and press \\\"n\\\" to abort the cleaning.\\n\"\n@@ -270,12 +336,15 @@ void interactive_clean(struct string_list *dels, const char *prefix)\n \t\t\t \"WARNING: to be excluded from the cleaning.\"\n \t\t\t ));\n \t\tputchar('\\n');\n+\t\tclean_print_color(CLEAN_COLOR_RESET);\n \n \t\t/* Display dels in columns */\n \t\tpretty_print_dels(dels, prefix);\n \n \t\t/* Confirmation dialog */\n+\t\tclean_print_color(CLEAN_COLOR_PROMPT);\n \t\tprintf(count > 0 ? _(\"Remove (Yes/no/edit) ? \") : _(\"Remove (yes/no/Edit) ? \"));\n+\t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\tstrbuf_getline(&confirm, stdin, '\\n');\n \t\tstrbuf_trim(&confirm);\n \t\tputchar('\\n');\n-- \n1.8.3.rc0.364.gbb5463f\n"},{"id":"216260","messageId":"CANYiYbFXFM5Z394b+9gWxJ3=OfXHmnwjH44V0tNOp3Vf+G2niw@mail.gmail.com","threadId":"33614","inReplyTo":"vpq38u6n397.fsf@grenoble-inp.fr","subject":"Re: [PATCH v3] Add support for -i/--interactive to git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-05-02T13:43:54Z","receivedAt":"2013-05-02T13:43:54Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/5/1 Matthieu Moy :\n> Jiang Xin writes:\n>\n>> Show what would be done and the user must confirm before actually\n>> cleaning. In the confirmation dialog, the user has three choices:\n>>\n>> * Yes: Start to do cleaning.\n>> * No: Nothing will be deleted.\n>> * Edit (default): Enter edit mode.\n>\n> I like this much more than the previous one. I played with it a bit, and\n> found it much more pleasant than \"rm -i\": by default, only one querry,\n> but still an option to select which files to clean.\n\nI add columns and colors display for interactive git-clean, and wish you\nlike them too.\n\n> I'm wondering whether \"Enter\" in the edit mode should return to the\n> yes/no/Edit querry instead of applying the clean. It would make it clear\n> for the user that it's still possible to cancel completely (the\n> Control-C hint is not visible in the UI otherwise).\n\nIn patch v4 1/3, I make it a three stages cleaning, and it is also\neasy to do real cleaning -- just press three ENTERs.\n\n1. Remove (yes/no/Edit) ? -- Press the 1st ENTER (default to EDIT mode)\n2. Input ignore patterns> -- Press the 2nd ENTER (back to confirm)\n3. Remove (Yes/no/edit) ? -- Press the 3rd ENTER (start to do cleaning)\n\n>> Reviewed-by: Matthieu Moy \n>\n> Please, no. I already mentionned it in my previous patch, but I did not\n> review the patch. See SubmittingPatches:\n>\n> 3. \"Reviewed-by:\", unlike the other tags, can only be offered by the\n> reviewer and means that she is completely satisfied that the patch\n> is ready for application. It is usually offered only after a\n> detailed review.\n>\n> Commenting != reviewing.\n\nI will add you with 'Comments-by:' tag temporarily, and invent a\n'Spelling-check-by:' tag (now there are 100+ kinds of tags in Git) to\nsay thank you to Eric. After this series of patches reach a consensus,\nI may change the tag from 'Comments-by: to 'Reviewed-by:' with\nyour permissions.\n\n>\n>> + /* dels list may become empty when we run string_list_remove_empty_items later */\n>> + if (!dels->nr)\n>> + break;\n>\n> This happens when the user removed everything from the list in the edit\n> mode. This could print something before breaking (and then exiting\n> silently). Maybe \"No more files to clean, exiting.\" or so.\n\nadded in patch v4 1/3..\n\n>> + printf(_(\"Remove (yes/no/Edit) ? \"));\n>> + strbuf_getline(&confirm, stdin, '\\n');\n>> + strbuf_trim(&confirm);\n>> + if (confirm.len) {\n>> + if (!strncasecmp(confirm.buf, \"yes\", confirm.len)) {\n>> + break;\n>> + } else if (!strncasecmp(confirm.buf, \"no\", confirm.len)) {\n>> + string_list_clear(dels, 0);\n>> + break;\n>> + }\n>> + }\n>> + edit_mode = 1;\n>\n> It's weird that anything but \"yes\" and \"no\" enter the edit mode without\n> complaining. It's safe, but surprising. If I type \"foo\", I'd rather get\n> an error and be asked again.\n>\n>> + if (!matches) {\n>> + strbuf_addf(&message, _(\"WARNING: Cannot find items prefixed by: %s\"), confirm.buf);\n>\n> \"prefixed\" seems a remainder of the previous version of the patch. You\n> probably mean \"matched by: %s\".\n\nUpdated in patch v4 1/3.\n\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\n\n-- \n蒋鑫\n\n北京群英汇信息技术有限公司\n邮件: worldhello.net@gmail.com\n网址: http://www.ossxp.com/\n博客: http://www.worldhello.net/\n微博: http://weibo.com/gotgit/\n电话: 18601196889\n"},{"id":"216264","messageId":"vpqppx9fmpu.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"3b80db3d2e8f31b1d9373406cce3cbdee2094a29.1367500374.git.worldhello.net@gmail.com","subject":"Re: [PATCH v4 2/3] Show items of interactive git-clean in columns","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-05-02T15:03:25Z","receivedAt":"2013-05-02T15:03:25Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> Signed-off-by: Jiang Xin \n\nThis lacks a proper commit message (why is this a good thing?), and\ndocumentation (you introduce column.clean) but the code sounds good\n(that's a very quick look from me, not a \"review\" sorry).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"216265","messageId":"vpqhailfmj9.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"bb5463fa4ef09df08ad3bf5abf4660a5df414e64.1367500374.git.worldhello.net@gmail.com","subject":"Re: [PATCH v4 3/3] Add colors to interactive git-clean","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-05-02T15:07:22Z","receivedAt":"2013-05-02T15:07:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> Show help, error messages, and prompt in colors for interactive\n> git-clean.\n\nI find the red WARNING a bit agressive. Also, the NOTE: is the same\ncolor as the WARNING, hence visually similar. I first thought it was\nrepeating the same message. I think it would make more sense to use\ndefault color, black, for the NOTE, by analogy with \"git add -p\" which\nuses it for patch hunk headers (somehow, this is the question you're\nreplying to after the blue prompt below).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"216318","messageId":"CANYiYbGsMbJ=Ra6_fv12fnC_-Y5YfdWMgnROFWxo+Y=mkPKsiA@mail.gmail.com","threadId":"33614","inReplyTo":"vpqppx9fmpu.fsf@grenoble-inp.fr","subject":"Re: [PATCH v4 2/3] Show items of interactive git-clean in columns","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-05-03T01:26:21Z","receivedAt":"2013-05-03T01:26:21Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/5/2 Matthieu Moy :\n> Jiang Xin writes:\n>\n>> Signed-off-by: Jiang Xin \n>\n> This lacks a proper commit message (why is this a good thing?), and\n\nRewrite the log as following:\n\nShow items of interactive git-clean in columns\n\nWhen there are lots of items to be cleaned, it is hard to see them all\nin one screen. Show them in columns instead of in one column will solve\nthis problem.\n\nSince no longer show items to be cleaned using the \"Would remove ...\"\nformat (only plain filenames), we add instructions/warnings as header\nbefore them.\n\n> documentation (you introduce column.clean) but the code sounds good\n> (that's a very quick look from me, not a \"review\" sorry).\n\nWill honors column.ui config variable only, not introduce new config variable.\nSo no more documentations needed ;-)\n\nFix-up looks like this:\n\n static int git_clean_config(const char *var, const char *value, void *cb)\n {\n+ /* honors the column.ui config variable only */\n if (!prefixcmp(var, \"column.\"))\n- return git_column_config(var, value, \"clean\", &colopts);\n+ return git_column_config(var, value, NULL, &colopts);\n if (!strcmp(var, \"color.interactive\")) {\n clean_use_color = git_config_colorbool(var, value);\n return 0;\n\n-- \nJiang Xin\n"},{"id":"216322","messageId":"CANYiYbG5r=sO7qqU4hnOpCUUrgVYPZH6XF7xO4+6TqCGTjC6pg@mail.gmail.com","threadId":"33614","inReplyTo":"vpqhailfmj9.fsf@grenoble-inp.fr","subject":"Re: [PATCH v4 3/3] Add colors to interactive git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-05-03T02:53:01Z","receivedAt":"2013-05-03T02:53:01Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/5/2 Matthieu Moy :\n> Jiang Xin writes:\n>\n>> Show help, error messages, and prompt in colors for interactive\n>> git-clean.\n>\n> I find the red WARNING a bit agressive. Also, the NOTE: is the same\n> color as the WARNING, hence visually similar. I first thought it was\n> repeating the same message. I think it would make more sense to use\n> default color, black,\n\nWill use new style -- CLEAN_COLOR_HEADER (GIT_COLOR_BOLD)\nas header for interactive git-clean (i.e. WARNING)\n\n> for the NOTE, by analogy with \"git add -p\" which\n> uses it for patch hunk headers (somehow, this is the question you're\n> replying to after the blue prompt below).\n\nI agree. NOTE should only be displayed once, and will be fixed\nin patch 3/1 of next reroll.\n\n--\nJiang Xin\n"},{"id":"216358","messageId":"vpqfvy4b4v0.fsf@grenoble-inp.fr","threadId":"33614","inReplyTo":"CANYiYbGsMbJ=Ra6_fv12fnC_-Y5YfdWMgnROFWxo+Y=mkPKsiA@mail.gmail.com","subject":"Re: [PATCH v4 2/3] Show items of interactive git-clean in columns","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-05-03T12:54:59Z","receivedAt":"2013-05-03T12:54:59Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jiang Xin writes:\n\n> Rewrite the log as following:\n\nThat's probably more than needed ;-). Thanks,\n\n> Show items of interactive git-clean in columns\n>\n> When there are lots of items to be cleaned, it is hard to see them all\n> in one screen. Show them in columns instead of in one column will solve\n> this problem.\n>\n> Since no longer show items to be cleaned using the \"Would remove ...\"\n\n\"Since _we_ no longer ...\"?\n\n> Will honors column.ui config variable only, not introduce new config variable.\n> So no more documentations needed ;-)\n\nI don't think it's a good idea. Usually, the *.ui variables are\nshortcuts for \"set all the individual variables\", and having things\ncustomizeable by column.ui and not by anything else breaks this.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}