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.
