Re: [PATCH v2] Add support for -i/--interactive to git-clean

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/4/29 Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx>:
> Jiang Xin <worldhello.net@xxxxxxxxx> writes:
>
>> Show what would be done and a confirmation dialog before actually
>> cleaning. In the confirmation dialog, the user can input a space
>> separated prefix list, and each clean candidate that matches with
>> one of prefix, will be excluded from cleaning.
>
> That seems a really weird interface. In particular, that means people
> typing "no" or "n" mechanically will have everything not starting with
> "n" or "no" deleted. We've learnt from the "git send-email" interface
> that people (including me ;-) ) do type "yes" mechanically to some
> questions.

Because of this (typing "no" or "n" not stop the whole cleaning),
I write a loop, only when user press Enter (input nothing), begin
to clean files/directories, and there would be a warning if nothing
excluded by the prefix/pattern.

That means when user input "n" or "no", the interactive git-clean
will not delete entries starting with "n" or "no" (instead of delete
nothing), but it still give the user a hint what happened. It will
show the filtered cleaning list or a warning message that there
are no entries starting with "n" or "no" to be filtered out (may be
the warning messages should be cached and displayed in
the end, not at the beginning).

>
> My feeling is that you're doing a bad compromise between a confirmation
> dialog (y/n) and a really interactive command (like "git add -i") with a
> rich interface.

It's not confirmation, but a last chance to save your cleaning files.
The use can input a space-seperated prefixes list, but I think most
people would like to input a space-seperated patterns (like .gitignore).
I try to implement like this, but still need sometime to refector the code.

+                               el = add_exclude_list(&dir, EXC_CMDL,
"exclude by interactive git clean");
+                               add_exclude((*prefix_list)->buf, "",
0, el, -(++i));

git-clean is a simple task, I doubt write a rich interface has no much value,
and make things complicated. But render prompt, error outputs in different
color like interactive git-add is interesting.


>> @@ -34,7 +34,17 @@ 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 a confirmation dialog before actually
>> +  cleaning. In the confirmation dialog, the user can input a space
>> +  separated prefix list, and each clean candidate that matches with
>> +  one of prefix, will be excluded from cleaning. When the user feels
>> +  it's OK, press ENTER to start cleaning. If the user wants to cancel
>> +  the whole cleaning, simply input ctrl-c in the confirmation dialog.
>
> Broken indentation. It seems you've set your tab-width to 2, in which
> case you can't see it, but you've indented with 2 spaces where the rest
> of the file is indented with tabs.

Oops, I code lots of ruby recently, and vim is not smart to handle txt file.
I will correct them.

>>               if (S_ISDIR(st.st_mode)) {
>> -                     strbuf_addstr(&directory, ent->name);
>>                       if (remove_directories || (matches == MATCHED_EXACTLY)) {
>> -                             if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))
>> -                                     errors++;
>> -                             if (gone && !quiet) {
>> -                                     qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
>> -                                     printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
>> -                             }
>> +                             string_list_append(&dels, ent->name);
>
> The patch would be much easier to read if split into a first refactoring
> patch that would introduce this "dels" list, and a second patch that
> would introduce -i. Reading this, I wondered why the code was removed,
> while it was actually just moved.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/



-- 
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]