On Tue, Oct 18, 2022 at 8:54 AM Tao Klerks via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: Tao Klerks <tao@xxxxxxxxxx> > > When no merge.tool or diff.tool is configured or manually selected, the > selection of a default tool is sensitive to the DISPLAY variable; in a > GUI session a gui-specific tool will be proposed if found, and > otherwise a terminal-based one. This "GUI-optimizing" behavior is > important because a GUI can make a huge difference to a user's ability > to understand and correctly complete a non-trivial conflicting merge. > > Some time ago the merge.guitool and diff.guitool config options were > introduced to enable users to configure both a GUI tool, and a non-GUI > tool (with fallback if no GUI tool configured), in the same environment. > > Unfortunately, the --gui argument introduced to support the selection of > the guitool is still explicit. When using configured tools, there is no > equivalent of the no-tool-configured "propose a GUI tool if we are in a GUI > environment" behavior. > > As proposed in <xmqqmtb8jsej.fsf@gitster.g>, introduce new configuration > options, difftool.guiDefault and mergetool.guiDefault, supporting a special > value "auto" which causes the corresponding tool or guitool to be selected > depending on the presence of a non-empty DISPLAY value. Also support "true" > to say "default to the guitool (unless --no-gui is passed on the > commandline)", and "false" as the previous default behavior when these new > configuration options are not specified. > > Signed-off-by: Tao Klerks <tao@xxxxxxxxxx> > --- > RFC: mergetool: new config guiDefault supports auto-toggling gui by > DISPLAY > > I'm reasonably comfortable that with this patch we do the right thing, > but I'm not sure about yet another remaining implementation detail: > > * After implementing Junio's recommended "fail if defaulting config is > consulted and is invalid" flow, there now needs to be a distinction > between subshell exit code 1, which was used before and indicates > "tool not found or broken; falling back to default" and other > (higher) exit codes, which newly mean "something went wrong, stop!". > The resulting code looks awkward, I can't tell whether I'm missing a > code or even commenting pattern that would make it clearer. > > V3: > > * Simplify C code to use OPT_BOOL with an int rather than a custom > option-parsing function with an enum > * Fix doc to more extensively use backticks for config keys / values / > args > * Fix more shell script formatting issues > * Change error-handling in mergetool and difftool helpers to exit if > defaulting config is invalid > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1381%2FTaoK%2Ftao-mergetool-autogui-v3 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1381/TaoK/tao-mergetool-autogui-v3 > Pull-Request: https://github.com/gitgitgadget/git/pull/1381 Hi folks, this v3 never got any feedback - the only reason I had left it as an RFC was that the error-handling looked a bit awkward, as I noted above. Should I resubmit this without the RFC prefix? Are there any concerns about the change here to better support mixed GUI/console-only environments? Thanks, Tao
