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
