Re: [PATCH v3] Add support for -i/--interactive to git-clean
2013/5/1 Matthieu Moy :
> Jiang Xin writes:
>
>> Show what would be done and the user must confirm before actually
>> cleaning. In the confirmation dialog, the user has three choices:
>>
>> * Yes: Start to do cleaning.
>> * No: Nothing will be deleted.
>> * Edit (default): Enter edit mode.
>
> I like this much more than the previous one. I played with it a bit, and
> found it much more pleasant than "rm -i": by default, only one querry,
> but still an option to select which files to clean.
I add columns and colors display for interactive git-clean, and wish you
like them too.
> I'm wondering whether "Enter" in the edit mode should return to the
> yes/no/Edit querry instead of applying the clean. It would make it clear
> for the user that it's still possible to cancel completely (the
> Control-C hint is not visible in the UI otherwise).
In patch v4 1/3, I make it a three stages cleaning, and it is also
easy to do real cleaning -- just press three ENTERs.
1. Remove (yes/no/Edit) ? -- Press the 1st ENTER (default to EDIT mode)
2. Input ignore patterns> -- Press the 2nd ENTER (back to confirm)
3. Remove (Yes/no/edit) ? -- Press the 3rd ENTER (start to do cleaning)
>> Reviewed-by: Matthieu Moy
>
> Please, no. I already mentionned it in my previous patch, but I did not
> review the patch. See SubmittingPatches:
>
> 3. "Reviewed-by:", unlike the other tags, can only be offered by the
> reviewer and means that she is completely satisfied that the patch
> is ready for application. It is usually offered only after a
> detailed review.
>
> Commenting != reviewing.
I will add you with 'Comments-by:' tag temporarily, and invent a
'Spelling-check-by:' tag (now there are 100+ kinds of tags in Git) to
say thank you to Eric. After this series of patches reach a consensus,
I may change the tag from 'Comments-by: to 'Reviewed-by:' with
your permissions.
>
>> + /* dels list may become empty when we run
>> string_list_remove_empty_items later */
>> + if (!dels->nr)
>> + break;
>
> This happens when the user removed everything from the list in the edit
> mode. This could print something before breaking (and then exiting
> silently). Maybe "No more files to clean, exiting." or so.
added in patch v4 1/3..
>> + printf(_("Remove (yes/no/Edit) ? "));
>> + strbuf_getline(&confirm, stdin, '\n');
>> + strbuf_trim(&confirm);
>> + if (confirm.len) {
>> + if (!strncasecmp(confirm.buf, "yes",
>> confirm.len)) {
>> + break;
>> + } else if (!strncasecmp(confirm.buf, "no",
>> confirm.len)) {
>> + string_list_clear(dels, 0);
>> + break;
>> + }
>> + }
>> + edit_mode = 1;
>
> It's weird that anything but "yes" and "no" enter the edit mode without
> complaining. It's safe, but surprising. If I type "foo", I'd rather get
> an error and be asked again.
>
>> + if (!matches) {
>> + strbuf_addf(&message, _("WARNING: Cannot find items
>> prefixed by: %s"), confirm.buf);
>
> "prefixed" seems a remainder of the previous version of the patch. You
> probably mean "matched by: %s".
Updated in patch v4 1/3.
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
--
蒋鑫
北京群英汇信息技术有限公司
邮件: worldhello@gmail.com
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 18601196889
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Add support for -i/--interactive to git-clean
Jiang Xin writes:
> Show what would be done and the user must confirm before actually
> cleaning. In the confirmation dialog, the user has three choices:
>
> * Yes: Start to do cleaning.
> * No: Nothing will be deleted.
> * Edit (default): Enter edit mode.
I like this much more than the previous one. I played with it a bit, and
found it much more pleasant than "rm -i": by default, only one querry,
but still an option to select which files to clean.
I'm wondering whether "Enter" in the edit mode should return to the
yes/no/Edit querry instead of applying the clean. It would make it clear
for the user that it's still possible to cancel completely (the
Control-C hint is not visible in the UI otherwise).
> Reviewed-by: Matthieu Moy
Please, no. I already mentionned it in my previous patch, but I did not
review the patch. See SubmittingPatches:
3. "Reviewed-by:", unlike the other tags, can only be offered by the
reviewer and means that she is completely satisfied that the patch
is ready for application. It is usually offered only after a
detailed review.
Commenting != reviewing.
> + /* dels list may become empty when we run
> string_list_remove_empty_items later */
> + if (!dels->nr)
> + break;
This happens when the user removed everything from the list in the edit
mode. This could print something before breaking (and then exiting
silently). Maybe "No more files to clean, exiting." or so.
> + printf(_("Remove (yes/no/Edit) ? "));
> + strbuf_getline(&confirm, stdin, '\n');
> + strbuf_trim(&confirm);
> + if (confirm.len) {
> + if (!strncasecmp(confirm.buf, "yes",
> confirm.len)) {
> + break;
> + } else if (!strncasecmp(confirm.buf, "no",
> confirm.len)) {
> + string_list_clear(dels, 0);
> + break;
> + }
> + }
> + edit_mode = 1;
It's weird that anything but "yes" and "no" enter the edit mode without
complaining. It's safe, but surprising. If I type "foo", I'd rather get
an error and be asked again.
> + if (!matches) {
> + strbuf_addf(&message, _("WARNING: Cannot find items
> prefixed by: %s"), confirm.buf);
"prefixed" seems a remainder of the previous version of the patch. You
probably mean "matched by: %s".
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] Add support for -i/--interactive to git-clean
Show what would be done and the user must confirm before actually
cleaning. In the confirmation dialog, the user has three choices:
* Yes: Start to do cleaning.
* No: Nothing will be deleted.
* Edit (default): Enter edit mode.
When the user chooses the edit mode, the user can input space-
separated patterns (the same syntax as gitignore), and each clean
candidate that matches with one of the patterns will be excluded
from cleaning. When the user feels it's OK, presses ENTER to start
cleaning.
When in the edit mode, if the user wants to cancel the whole
cleaning, simply inputs ctrl-c to abort the cleaning.
Signed-off-by: Jiang Xin
Suggested-by: Junio C Hamano
Reviewed-by: Eric Sunshine
Reviewed-by: Matthieu Moy
---
Documentation/git-clean.txt | 16 -
builtin/clean.c | 141 +++-
2 files changed, 140 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index bdc3a..6bc99 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree
SYNOPSIS
[verse]
-'git clean' [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...
+'git clean' [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...
DESCRIPTION
---
@@ -34,7 +34,19 @@ OPTIONS
-f::
--force::
If the Git configuration variable clean.requireForce is not set
- to false, 'git clean' will refuse to run unless given -f or -n.
+ to false, 'git clean' will refuse to run unless given -f, -n or
+ -i.
+
+-i::
+--interactive::
+ Show what would be done and the user must confirm before actually
+ cleaning. In the confirmation dialog, the user can choose to abort
+ the cleaning, or enter into an edit mode. In the edit mode, the
+ user can input space-separated patterns (the same syntax as
+ gitignore), and each clean candidate that matches with one of the
+ patterns will be excluded from cleaning. When the user feels it's
+ OK, presses ENTER to start cleaning. If the user wants to cancel
+ the whole cleaning in the edit mode, simply inputs ctrl-c to abort.
-n::
--dry-run::
diff --git a/builtin/clean.c b/builtin/clean.c
index 04e39..b26c0 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,9 +15,10 @@
#include "quote.h"
static int force = -1; /* unset */
+static int interactive;
static const char *const builtin_clean_usage[] = {
- N_("git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--]
..."),
+ N_("git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--]
..."),
NULL
};
@@ -142,6 +143,96 @@ static int remove_dirs(struct strbuf *path, const char
*prefix, int force_flag,
return ret;
}
+void interactive_clean(struct string_list *dels, const char *prefix)
+{
+ struct dir_struct dir;
+ struct strbuf confirm = STRBUF_INIT;
+ struct strbuf message = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf **ignore_list;
+ struct string_list_item *item;
+ struct exclude_list *el;
+ const char *qname;
+ int edit_mode = 0, i;
+
+ while (1) {
+ int matches = 0;
+
+ /* dels list may become empty when we run
string_list_remove_empty_items later */
+ if (!dels->nr)
+ break;
+
+ for_each_string_list_item(item, dels) {
+ qname = quote_path_relative(item->string, -1, &buf,
prefix);
+ printf(_(msg_would_remove), qname);
+ }
+
+ if (message.len) {
+ printf("\n%s\n", message.buf);
+ strbuf_reset(&message);
+ }
+
+ if (!edit_mode) {
+ printf(_("Remove (yes/no/Edit) ? "));
+ strbuf_getline(&confirm, stdin, '\n');
+ strbuf_trim(&confirm);
+ if (confirm.len) {
+ if (!strncasecmp(confirm.buf, "yes",
confirm.len)) {
+ break;
+ } else if (!strncasecmp(confirm.buf, "no",
confirm.len)) {
+ string_list_clear(dels, 0);
+ break;
+ }
+ }
+ edit_mode = 1;
+ }
+
+ printf(_("Input ignore patterns to keep items, or press ENTER
to confirm: "));
+ strbuf_getline(&confirm, stdin, '\n');
+ strbuf_trim(&confirm);
+ printf("\n");
+
+ if (!confirm.len)
+ break;
+
+ memset(&dir, 0, sizeof(dir));
+ el = add_exclude_list(&dir, EXC_CMDL, "manual exclude");
+ ignore_list = strbuf_split_buf(confirm.buf, confirm.len, ' ',
0);
+
