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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 1, 2018, 10:56 UTC
Message-ID: <CAPig+cTEoM3qYRN6j=zSVv1LPx_ggVuxqTMMK0SpjtYEqOiBMA@mail.gmail.com>
In-Reply-To: <bbc03595-cb31-63b8-3f4c-cfa04bb9ff27@suse.de>

On Thu, Feb 1, 2018 at 5:43 AM, Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de> wrote:

Show 20 quoted lines

> 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.