2013/5/1 Matthieu Moy <matthieu....@grenoble-inp.fr>:
> Jiang Xin <worldhello....@gmail.com> 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 <matthieu....@imag.fr>
> 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

Reply via email to