Re: [PATCH v3 7/7] add--interactive.perl: use add--helper --show-help for help_cmd
- From
- Slavica Djukic <slavicadj.ip2018@gmail.com>
- Date
- Jan 21, 2019, 11:59 UTC
- Message-ID
- <1224d852-6009-7c91-764a-f1015ed253b5@gmail.com>
- In-Reply-To
- <87fttm49hv.fsf@evledraar.gmail.com>
Hello Ævar,
thanks for taking time and making review.
On 21-Jan-19 10:59 AM, Ævar Arnfjörð Bjarmason wrote:
Show 48 quoted lines
> On Mon, Jan 21 2019, Slavica Djukic via GitGitGadget wrote:
>
>> From: Slavica Djukic <slawica92@hotmail.com>
>>
>> Change help_cmd sub in git-add--interactive.perl to use
>> show-help command from builtin add--helper.
>>
>> Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
>> ---
>> git-add--interactive.perl | 11 +----------
>> 1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
>> index a6536f9cf3..32ee729a58 100755
>> --- a/git-add--interactive.perl
>> +++ b/git-add--interactive.perl
>> @@ -1717,16 +1717,7 @@ sub quit_cmd {
>> }
>>
>> sub help_cmd {
>> -# TRANSLATORS: please do not translate the command names
>> -# 'status', 'update', 'revert', etc.
>> - print colored $help_color, __ <<'EOF' ;
>> -status - show paths with changes
>> -update - add working tree state to the staged set of changes
>> -revert - revert staged set of changes back to the HEAD version
>> -patch - pick hunks and update selectively
>> -diff - view diff between HEAD and index
>> -add untracked - add contents of untracked files to the staged set of changes
>> -EOF
>> + system(qw(git add--helper --show-help));
>> }
>>
>> sub process_args {
> Both this and an earlier change in this series replaces a callback
> command with an invocation of system() without any error checking. So if
> this add-helper fails for whatever reason we'll silently fail to report
> it.
>
> I think it makes sense to put something like the following WIP code
> earlier in the series. Then if the command was e.g. ["false", "git",
> "status"] we'd see:
>
> What now> s
> oh noes when running 'false git status': returned error '1'
>
> WIP patch. Obviously not ready as-is, but feel free to consider this to
> have my SOB & adapt it.And thank you for writing up this WIP patch. I will adapt it and have your SOB.
-Slavica
Show 44 quoted lines
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 20eb81cc92..1cd5f8122b 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1777,7 +1777,7 @@ sub process_args {
> }
>
> sub main_loop {
> - my @cmd = ([ 'status', \&status_cmd, ],
> + my @cmd = ([ 'status', ["git", "status"] ],
> [ 'update', \&update_cmd, ],
> [ 'revert', \&revert_cmd, ],
> [ 'add untracked', \&add_untracked_cmd, ],
> @@ -1794,11 +1794,23 @@ sub main_loop {
> ON_EOF => \&quit_cmd,
> IMMEDIATE => 1 }, @cmd);
> if ($it) {
> - eval {
> - $it->[1]->();
> - };
> - if ($@) {
> - print "$@";
> + my $cb = $it->[1];
> + if (ref $cb eq 'CODE') {
> + eval {
> + $cb->();
> + 1;
> + } or do {
> + print "$@";
> + };
> + } else {
> + if (system(@$cb) != 0) {
> + if ($? == -1 || $? & 127) {
> + print STDERR "oh noes when running '@$cb': unexpected '$?'\n";
> + } else {
> + my $ret = $? >> 8;
> + print STDERR "oh noes when running '@$cb': returned error '$ret'\n";
> + }
> + }
> }
> }
> }
>