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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 2, 2018, 19:16 UTC
Message-ID: <CAPig+cQF2HzmtVdHqtQcOf0B-yA8Kpj-CZbPmdQotGwtdYpxvw@mail.gmail.com>
In-Reply-To: <52737deb-a5dc-27d6-3c0c-0d8b8de991c5@suse.com>

On Fri, Feb 2, 2018 at 11:48 AM, Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com> wrote:

Show 9 quoted lines

> What message do you suggest ?  As I said in a previous mail, a
> simple "Editor failure, cancelling {commit, tag}" should be enough
> as launch_editor already outputs error messages describing what
> issue the editor had.
>
> I don't think suggesting moving to --no-edit || -m || -F is that
> helpful.  It's basically saying your setup is broken, but you can
> workaround by setting those options (and not saying that you're
> going to have some more issues later one).

If it's the case the launch_editor() indeed outputs an appropriate error message, then the existing error message from tag.c is already appropriate when --edit is not specified. It's only the --edit case that the tag.c's additional message is somewhat weird. And, in fact, suppressing tag.c's message might be the correct thing to do in the --edit case:

    static void create_tag(...) {
        ...
if (launch_editor(...)) {
   if (!opt->use_editor)
       fprintf(stderr, _("... use either -m or -F ..."));
            exit(1);
}

I don't feel strongly about it either way and am fine with just punting on the issue until someone actually complains about it.