Johannes Sixt wrote: > Jonathan Nieder schrieb: >> From: Johannes Sixt <j.sixt@xxxxxxxxxxxxx> [...] > 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 <j6t@xxxxxxxx> Thanks for the catch. >>+ 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(). Makes sense. I am a bit uncomfortable with this error in general. It makes some sense to refuse to use $VISUAL and fall back to $EDITOR if TERM=dumb, since without this the distinction between VISUAL and EDITOR is not very useful. But wouldn’t that check be equally useful if GIT_EDITOR or core.editor is set to vi? Ideally, vi itself would check TERM and error out, and git would only need to report and handle the exit. Unfortunately, at least vim is happy to assume a terminal supports ANSI sequences even if TERM=dumb (e.g., when running from a text editor like Acme). Unless VISUAL, GIT_EDITOR, and core.editor are unset, nobody gets the benefit of this check. Should git error out rather than run $VISUAL when TERM=dumb? How about $GIT_EDITOR? The advice about options to avoid invoking an editor is not very helpful except with 'git commit', so probably only 'git commit' should print that message. > The return value, NULL, should be indication enough for the callers > to handle the situation suitable. Good idea. > 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 Maybe 'git var -l' should omit GIT_EDITOR in this situation. Thanks for the comments, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html