Re: [PATCH] tag: add --edit option

[Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de>]

Le 01/02/2018 à 11:56, Eric Sunshine a écrit :
> On Thu, Feb 1, 2018 at 5:43 AM, Nicolas Morey-Chaisemartin
> <NMoreyChaisemartin@suse.de> wrote:
>> Le 01/02/2018 à 11:34, Nicolas Morey-Chaisemartin a écrit :
>>> Le 01/02/2018 à 11:16, Eric Sunshine a écrit :
>>>> A little below this change is where launch_editor() is actually
>>>> invoked. If it fails for some reason, it prints:
>>>>
>>>>     Please supply the message using either -m or -F option.
>>>>
>>>> which seems a bit counterintuitive if the user *did* specify one of
>>>> those options along with --edit. I wonder if that message needs to be
>>>> adjusted.
>>>>
>>> Yes I'll fix this.
>> I just checked what commit.c does and it seems to behave as my patch:
>>         if (launch_editor(git_path_commit_editmsg(), NULL, env.argv)) {
>>             fprintf(stderr,
>>             _("Please supply the message using either -m or -F option.\n"));
>>
>> To be honest the message is not that clear either.
>> If I'm reading launch_editor right most (or all) its falire are du to a failure to launch the editor or the editor crashed/exited with an error.
>> In this case, I wouldn't advise the user to use -m or -F but to fix its editor.
> Indeed, I also looked at the implementation of launch_editor(), and my
> "wondering" about whether the message needed adjustment was just that.
> The message seems somewhat counterintuitive in this case, but I didn't
> necessarily have a better suggestion. A valid response, therefore,
> might be to punt on it and leave that change for the future, or
> perhaps take it on as a second patch which adjusts the message in both
> commands. I don't have strong feelings about it at this time.

It seems all the error paths from launch_editor have an error message.
A simple "Editor failure, cancelling {commit, tag}" would probably be a better error message.
I'll post another series for that.