Feature request: Add --no-edit to git tag command

> Similar to git commit, it would be nice to have a --no-edit option for
> git tag. Use case is when I force-recreate a tag:
> 
> $ git tag -af 1.0 123abc
> 
> An editor will be prompted with the previous annotated tag message. I
> would like to add --no-edit to instruct it to use any previously
> provided message and without prompting the editor:
> 
> $ git tag --no-edit -af 1.0 123abc

> On Wed, Apr 03, 2019 at 09:38:02AM -0500, Robert Dailey wrote:
>
> > Similar to git commit, it would be nice to have a --no-edit option for
> > git tag. Use case is when I force-recreate a tag:
> >
> > $ git tag -af 1.0 123abc
> >
> > An editor will be prompted with the previous annotated tag message. I
> > would like to add --no-edit to instruct it to use any previously
> > provided message and without prompting the editor:
> >
> > $ git tag --no-edit -af 1.0 123abc
>
> Yeah, that sounds like a good idea.

> Agreed.
> 
> I think that the implement is a little different than "add a --no-edit"
> flag, though. 'git tag' already has a OPT_BOOL for '--edit', which means
> that '--no-edit' exists, too.
> 
> But, when we look and see how the edit option is passed around, we find
> that the check whether or not to launch the editor (again, in
> builtin/tag.c within 'create_tag()') is:
> 
>   if (!opt->message_given || opt->use_editor)
> 
> So, it's not that we didn't take '--no-edit', it's that we didn't get a
> _message_, so we'll open the editor to get one (even if '--no-edit' was
> given).

>
> On Wed, Apr 03, 2019 at 08:26:06PM -0700, Taylor Blau wrote:
>
> > Agreed.
> >
> > I think that the implement is a little different than "add a --no-edit"
> > flag, though. 'git tag' already has a OPT_BOOL for '--edit', which means
> > that '--no-edit' exists, too.
> >
> > But, when we look and see how the edit option is passed around, we find
> > that the check whether or not to launch the editor (again, in
> > builtin/tag.c within 'create_tag()') is:
> >
> >   if (!opt->message_given || opt->use_editor)
> >
> > So, it's not that we didn't take '--no-edit', it's that we didn't get a
> > _message_, so we'll open the editor to get one (even if '--no-edit' was
> > given).
>
> Yeah, I think the fundamental issue with --no-edit is that it is not a
> tristate, so we cannot tell the difference between --edit, --no-edit,
> and nothing.
>
> I think regardless of the "re-use message bits", we'd want something
> like:
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 02f6bd1279..260adcaa60 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -196,7 +196,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu
>
>  struct create_tag_options {
>         unsigned int message_given:1;
> -       unsigned int use_editor:1;
> +       int use_editor;
>         unsigned int sign;
>         enum {
>                 CLEANUP_NONE,
> @@ -227,7 +227,7 @@ static void create_tag(const struct object_id *object, const char *tag,
>                     tag,
>                     git_committer_info(IDENT_STRICT));
>
> -       if (!opt->message_given || opt->use_editor) {
> +       if ((!opt->message_given && opt->use_editor != 0) || opt->use_editor > 0) {
>                 int fd;
>
>                 /* write the template message before editing: */
> @@ -380,7 +380,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>         static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
>         struct ref_format format = REF_FORMAT_INIT;
>         int icase = 0;
> -       int edit_flag = 0;
> +       int edit_flag = -1;
>         struct option options[] = {
>                 OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
>                 { OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
>
> which even does the right thing with "git tag --no-edit -a foo" (it dies
> with "fatal: no tag message?"
>
> > This makes me think that we should do two things:
> >
> >   1. Make !opt->message_give && !opt->use_editor an invalid invocation.
> >      If I (1) didn't give a message but I did (2) give '--no-edit', I'd
> >      expect a complaint, not an editor window.
> >
> >   2. Then, do what Robert suggests, which is to "make opt->message_given
> >      true", by re-using the previous tag's message.
>
> I think I misunderstood Robert's proposal. I thought it was just about
> fixing --no-edit, but it's actually about _adding_ (2). Which I think
> we'd want to do differently. See Junio's reply elsewhere in the thread
> (and my reply there).
>
> > > I think it wouldn't be very hard to implement, either. Maybe a good
> > > starter project or #leftoverbits for somebody.
> >
> > Maybe. I think that it's made a little more complicated by the above,
> > but it's certainly doable. Maybe good for GSoC?
>
> I was thinking it was just the --no-edit fix. :) Even with the "--amend"
> thing, though, it's probably a little light for a 3-month-long GSoC
> project. :)

>
> On Thu, Apr 4, 2019 at 7:06 AM Jeff King <peff@peff.net> wrote:
> >
> > On Wed, Apr 03, 2019 at 08:26:06PM -0700, Taylor Blau wrote:
> >
> > > Agreed.
> > >
> > > I think that the implement is a little different than "add a --no-edit"
> > > flag, though. 'git tag' already has a OPT_BOOL for '--edit', which means
> > > that '--no-edit' exists, too.
> > >
> > > But, when we look and see how the edit option is passed around, we find
> > > that the check whether or not to launch the editor (again, in
> > > builtin/tag.c within 'create_tag()') is:
> > >
> > >   if (!opt->message_given || opt->use_editor)
> > >
> > > So, it's not that we didn't take '--no-edit', it's that we didn't get a
> > > _message_, so we'll open the editor to get one (even if '--no-edit' was
> > > given).
> >
> > Yeah, I think the fundamental issue with --no-edit is that it is not a
> > tristate, so we cannot tell the difference between --edit, --no-edit,
> > and nothing.
> >
> > I think regardless of the "re-use message bits", we'd want something
> > like:
> >
> > diff --git a/builtin/tag.c b/builtin/tag.c
> > index 02f6bd1279..260adcaa60 100644
> > --- a/builtin/tag.c
> > +++ b/builtin/tag.c
> > @@ -196,7 +196,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu
> >
> >  struct create_tag_options {
> >         unsigned int message_given:1;
> > -       unsigned int use_editor:1;
> > +       int use_editor;
> >         unsigned int sign;
> >         enum {
> >                 CLEANUP_NONE,
> > @@ -227,7 +227,7 @@ static void create_tag(const struct object_id *object, const char *tag,
> >                     tag,
> >                     git_committer_info(IDENT_STRICT));
> >
> > -       if (!opt->message_given || opt->use_editor) {
> > +       if ((!opt->message_given && opt->use_editor != 0) || opt->use_editor > 0) {
> >                 int fd;
> >
> >                 /* write the template message before editing: */
> > @@ -380,7 +380,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> >         static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
> >         struct ref_format format = REF_FORMAT_INIT;
> >         int icase = 0;
> > -       int edit_flag = 0;
> > +       int edit_flag = -1;
> >         struct option options[] = {
> >                 OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
> >                 { OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
> >
> > which even does the right thing with "git tag --no-edit -a foo" (it dies
> > with "fatal: no tag message?"
> >
> > > This makes me think that we should do two things:
> > >
> > >   1. Make !opt->message_give && !opt->use_editor an invalid invocation.
> > >      If I (1) didn't give a message but I did (2) give '--no-edit', I'd
> > >      expect a complaint, not an editor window.
> > >
> > >   2. Then, do what Robert suggests, which is to "make opt->message_given
> > >      true", by re-using the previous tag's message.
> >
> > I think I misunderstood Robert's proposal. I thought it was just about
> > fixing --no-edit, but it's actually about _adding_ (2). Which I think
> > we'd want to do differently. See Junio's reply elsewhere in the thread
> > (and my reply there).
> >
> > > > I think it wouldn't be very hard to implement, either. Maybe a good
> > > > starter project or #leftoverbits for somebody.
> > >
> > > Maybe. I think that it's made a little more complicated by the above,
> > > but it's certainly doable. Maybe good for GSoC?
> >
> > I was thinking it was just the --no-edit fix. :) Even with the "--amend"
> > thing, though, it's probably a little light for a 3-month-long GSoC
> > project. :)
>
> I apologize for the confusion. I'm not fully aware of any per-option
> philosophies in Git, so I may be unaware of the misunderstanding my
> request is causing. Let me attempt to clarify.
>
> My goal as a user is to correct a tag. If I point a tag at the wrong
> commit, I simply want to move that tag to point to another commit. At
> the moment, the only way I know to do this is the -f option, which I
> just treat as a "move" for the tag. I realize that may not be its
> intent in the implementation, but from a user perspective that's the
> end result I get.
>
> So if I treat -f as a "move this tag", I also want to say "reuse the
> existing commit message". So again, in my mind, that means -f
> --no-edit. Which means "I'm moving this tag and I want to keep the
> previous commit message".
>
> I hope this makes more sense. If getting this means not using -f or
> --no-edit at all, and is instead a whole different set of options, I'm
> OK with that as long as the end result is achievable. It's impossible
> to write a script to "move" (-f) a bunch of annotated tags without an
> editor prompting me on each one. So this "--no-edit" addition would
> assist in automation, and also making sure that we simply want to
> correct a tag, but not alter the message.

> > I was thinking it was just the --no-edit fix. :) Even with the "--amend"
> > thing, though, it's probably a little light for a 3-month-long GSoC
> > project. :)
> 
> I apologize for the confusion. I'm not fully aware of any per-option
> philosophies in Git, so I may be unaware of the misunderstanding my
> request is causing. Let me attempt to clarify.

> My goal as a user is to correct a tag. If I point a tag at the wrong
> commit, I simply want to move that tag to point to another commit. At
> the moment, the only way I know to do this is the -f option, which I
> just treat as a "move" for the tag. I realize that may not be its
> intent in the implementation, but from a user perspective that's the
> end result I get.
> 
> So if I treat -f as a "move this tag", I also want to say "reuse the
> existing commit message". So again, in my mind, that means -f
> --no-edit. Which means "I'm moving this tag and I want to keep the
> previous commit message".
> 
> I hope this makes more sense. If getting this means not using -f or
> --no-edit at all, and is instead a whole different set of options, I'm
> OK with that as long as the end result is achievable. It's impossible
> to write a script to "move" (-f) a bunch of annotated tags without an
> editor prompting me on each one. So this "--no-edit" addition would
> assist in automation, and also making sure that we simply want to
> correct a tag, but not alter the message.

>
> On Thu, Apr 04, 2019 at 08:56:16AM -0500, Robert Dailey wrote:
>
> > > I was thinking it was just the --no-edit fix. :) Even with the "--amend"
> > > thing, though, it's probably a little light for a 3-month-long GSoC
> > > project. :)
> >
> > I apologize for the confusion. I'm not fully aware of any per-option
> > philosophies in Git, so I may be unaware of the misunderstanding my
> > request is causing. Let me attempt to clarify.
>
> I think most of the confusion was just bad reading on my part. :)
>
> > My goal as a user is to correct a tag. If I point a tag at the wrong
> > commit, I simply want to move that tag to point to another commit. At
> > the moment, the only way I know to do this is the -f option, which I
> > just treat as a "move" for the tag. I realize that may not be its
> > intent in the implementation, but from a user perspective that's the
> > end result I get.
> >
> > So if I treat -f as a "move this tag", I also want to say "reuse the
> > existing commit message". So again, in my mind, that means -f
> > --no-edit. Which means "I'm moving this tag and I want to keep the
> > previous commit message".
> >
> > I hope this makes more sense. If getting this means not using -f or
> > --no-edit at all, and is instead a whole different set of options, I'm
> > OK with that as long as the end result is achievable. It's impossible
> > to write a script to "move" (-f) a bunch of annotated tags without an
> > editor prompting me on each one. So this "--no-edit" addition would
> > assist in automation, and also making sure that we simply want to
> > correct a tag, but not alter the message.
>
> Yeah, I think what you want to do is perfectly reasonable. The only
> reason not to use "-f" is because it already means other things, and we
> don't want to overload it.

> Calling it "--amend" would make perfect
> sense (and then fixing "--no-edit" so it lets you avoid opening the
> editor). So I think there are two bits of work:
>
>   1. Add an "--amend" option, which allows overwriting an existing tag
>      and pre-populates the message with the existing tag's message.
>
>   2. Make --no-edit work like the patch I showed earlier (assuming that
>      your --amend still opens an editor by default).
>
> -Peff

> This new proposed --amend option, although semantically different,
> would have a very similar functionality to the already existing -f
> option. So should we, perhaps, change -f's behavior to treat the tag
> as a new one, treating the old one as if it never existed (as I think
> Junio was saying)? By this I mean the command should fail if the user
> doesn't give a SHA-1 and the previous message wouldn't be preloaded.
> --amend, on the other hand, would give the user an opportunity to
> revise the tag by opening, by default, the editor with the
> pre-existing message unless given the '--no-edit' option, and if not
> given a SHA-1 it would keep on using the previous one.

> On Thu, Apr 11, 2019 at 03:20:52PM -0300, Bárbara de Castro Fernandes wrote:
>
>> This new proposed --amend option, although semantically different,
>> would have a very similar functionality to the already existing -f
>> option. So should we, perhaps, change -f's behavior to treat the tag
>> as a new one, treating the old one as if it never existed (as I think
>> Junio was saying)? By this I mean the command should fail if the user
>> doesn't give a SHA-1 and the previous message wouldn't be preloaded.
>> --amend, on the other hand, would give the user an opportunity to
>> revise the tag by opening, by default, the editor with the
>> pre-existing message unless given the '--no-edit' option, and if not
>> given a SHA-1 it would keep on using the previous one.
>
> Yes, that's what I'd expect it to do (so yes, it's also different from
> "-f" in that it defaults to the existing tag destination instead of
> HEAD).

> Jeff King <peff@peff.net> writes:
> 
> > On Thu, Apr 11, 2019 at 03:20:52PM -0300, Bárbara de Castro Fernandes wrote:
> >
> >> This new proposed --amend option, although semantically different,
> >> would have a very similar functionality to the already existing -f
> >> option. So should we, perhaps, change -f's behavior to treat the tag
> >> as a new one, treating the old one as if it never existed (as I think
> >> Junio was saying)? By this I mean the command should fail if the user
> >> doesn't give a SHA-1 and the previous message wouldn't be preloaded.
> >> --amend, on the other hand, would give the user an opportunity to
> >> revise the tag by opening, by default, the editor with the
> >> pre-existing message unless given the '--no-edit' option, and if not
> >> given a SHA-1 it would keep on using the previous one.
> >
> > Yes, that's what I'd expect it to do (so yes, it's also different from
> > "-f" in that it defaults to the existing tag destination instead of
> > HEAD).
> 
> Do you mean you'd expect "--amend" to do that, which is different
> from what "-f" does, so they should not be conflated into one?
> 
> If so, I think that makes sense and changing the behaviour of "-f"
> is too confusing.

> On Wed, Apr 03, 2019 at 09:38:02AM -0500, Robert Dailey wrote:
>
>> Similar to git commit, it would be nice to have a --no-edit option for
>> git tag. Use case is when I force-recreate a tag:
>> 
>> $ git tag -af 1.0 123abc
>> 
>> An editor will be prompted with the previous annotated tag message. I
>> would like to add --no-edit to instruct it to use any previously
>> provided message and without prompting the editor:
>> 
>> $ git tag --no-edit -af 1.0 123abc
>
> Yeah, that sounds like a good idea.

> >> Similar to git commit, it would be nice to have a --no-edit option for
> >> git tag. Use case is when I force-recreate a tag:
> >> 
> >> $ git tag -af 1.0 123abc
> >> 
> >> An editor will be prompted with the previous annotated tag message. I
> >> would like to add --no-edit to instruct it to use any previously
> >> provided message and without prompting the editor:
> >> 
> >> $ git tag --no-edit -af 1.0 123abc
> >
> > Yeah, that sounds like a good idea.
> 
> I am not so sure this is a good idea, especially if the plan is to
> do this alone without necessary associated change to make things
> consistent.

> The part that bothers me most is use of "-f".  The mentalitly behind
> "-f" is "I am creating a new and tag that is totally unrelated to
> any existing tag, but since the command refuses to reuse the ref to
> point at my new tag, I am giving an '-f' to force (1) unpointing the
> existing unrelated tag and (2) pointing the enwly created tag with
> that tagname".
> 
> The proposed change still uses "-f" but wants to somehow take the
> tag message from the unrelated tag that happens to sit at the same
> place as the new tag wants to go.  That breaks the mental model a
> big way.
> 
> If this were a new option that is spelled "--amend", I won't
> be complaining, though.