Re: [PATCH 3/8] Teach git var about GIT_EDITOR

Jonathan Nieder schrieb:

From: Johannes Sixt <redacted>

Expose the command used by launch_editor() for scripts to use.
This should allow one to avoid searching for a proper editor
separately in each command.

Signed-off-by: Johannes Sixt <redacted>
Signed-off-by: Jonathan Nieder <redacted>

Thanks for cleaning up behind me. I don't mind if you take authorship, but 
if you do keep my name, please make it:

From: Johannes Sixt <redacted>

-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+const char *git_editor(void)
 {
 	const char *editor, *terminal;
 
... 
 	terminal = getenv("TERM");
-	if (!editor && (!terminal || !strcmp(terminal, "dumb")))
+	if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
 		/* Terminal is dumb but no VISUAL nor EDITOR defined. */
-		return error(
+		error(
 		  "No editor specified in GIT_EDITOR, core.editor, VISUAL,\n"
 		  "or EDITOR. Tried to fall back to vi but terminal is dumb.\n"
 		  "Please set one of these variables to an appropriate\n"
 		  "editor or run again with options that will not cause an\n"
 		  "editor to be invoked (e.g., -m or -F for git commit).");
+		return NULL;
+	}

I somehow dislike that this huge error message is in git_editor(). The 
return value, NULL, should be indication enough for the callers to handle 
the situation suitable. In particular, launch_editor() wants to write this 
big warning, but 'git var -l' can avoid the error message and write only a 
short notice:

GIT_EDITOR=terminal is dumb, but VISUAL and EDITOR unset

+static const char *editor(int flag)
+{
+	const char *pgm = git_editor();
+
+	if (!pgm && (flag & IDENT_ERROR_ON_NO_NAME))
+		die("cannot find a suitable editor");
+	return pgm;

This should be

	return pgm ? pgm : "terminal is dumb, but VISUAL and EDITOR unset";

otherwise, 'git var -l' later trips over printf("%s", NULL).

-- Hannes