Web lists-archives.com

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




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. :)

-Peff