On Thu, Apr 4, 2019 at 8:56 AM Robert Dailey <rcdailey.lists@xxxxxxxxx> wrote:
>
> On Thu, Apr 4, 2019 at 7:06 AM Jeff King <peff@xxxxxxxx> 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.
Sorry I said "commit message" but I meant "annotated tag message".
