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

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

Le 01/02/2018 à 11:34, Nicolas Morey-Chaisemartin a écrit :
>
> Le 01/02/2018 à 11:16, Eric Sunshine a écrit :
>> On Thu, Feb 1, 2018 at 4:49 AM, Nicolas Morey-Chaisemartin
>> <nmoreychaisemartin@suse.com> wrote:
>>> Add a --edit option whichs allows modifying the messages provided by -m or -F,
>>> the same way git commit --edit does.
>>>
>>> Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.com>
>>> ---
>>> diff --git a/builtin/tag.c b/builtin/tag.c
>>> @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, const char *tag,
>>> -       if (!opt->message_given) {
>>> +       if (!opt->message_given || opt->use_editor) {
>>>
>>> -               if (!is_null_oid(prev)) {
>>> +               if (opt->message_given) {
>>> +                       write_or_die(fd, buf->buf, buf->len);
>>> +                       strbuf_reset(buf);
>>> +               } else if (!is_null_oid(prev)) {
>>>                         write_tag_body(fd, prev);
>>>                 } else {
>> 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"));
            exit(1);
        }


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.

Nicolas