{"thread":{"id":"58410","subject":"git mergetool, merge.tool, merge.guitool and DISPLAY","startedAt":"2022-09-09T11:56:43Z","lastAt":"2022-10-12T16:04:02Z","messageCount":5,"participants":["Tao Klerks","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"462847","messageId":"CAPMMpogZd_em4_Fdk0sNFqAXqH19VOVyw3WsNT2LHsQNOb0_rw@mail.gmail.com","threadId":"58410","inReplyTo":null,"subject":"git mergetool, merge.tool, merge.guitool and DISPLAY","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-09-09T11:56:24Z","receivedAt":"2022-09-09T11:56:43Z","isPatch":false,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"Hi folks,\n\nI've just become aware today of what *seems* like a very strange\ndiscrepancy or limitation in \"git mergetool\":\n\nWhen you use it without having configured a \"merge.tool\", it\nauto-selects the first available tool from a predefined list, which\nappears to be hardcoded in \"git-mergetool--lib.sh\", with some\nconditions around \"$DISPLAY\", \"$GNOME_DESKTOP_SESSION_ID\" and\n\"${VISUAL:-$EDITOR}\".\n\nIn this \"auto-selection\" situation, no GUI-based merge tool will be\noffered/selected if you're not in a GUI session.\n\nWhen you configure your tools, you can configure \"merge.tool\" for the\ndefault, and \"merge.guitool\" for GUI contexts - so far so good, sounds\nconsistent.\n\nHowever, once you've configured these two settings, \"git mergetool\"\nwill never select the GUI tool you've configured unless you very\n*explicitly* tell it to, by specifying the --gui argument. The\nsensible auto-selection based on \"DISPLAY\" disappears.\n\nThe upshot, as I understand it, is that the only way to get a GUI when\nI'm connected with an X session, and get a terminal-based mergetool\nwhen I'm not, without having to be aware and issue different commands,\nis to accept whatever tooling default order is hardcoded in\n\"git-mergetool--lib.sh\"\n\nIs this intentional / is there any logic here, or is this just\nunfortunate, a result of the auto-selection evolving more\nintelligently than the built-in explicit \"--gui\" selection, over\ntime??\n\nIf I wanted to improve this, what would be a more sensible approach?\n* Assume \"--gui\" if there is a DISPLAY and \"--no-gui\" has not been\nspecified? (behavior change for some existing users)\n* Add a new configuration to enable such an \"auto-gui respecting\nconfig\" mode (no behavior change)\n\n(I have not looked into \"git difftool\", but I assume the same\narguments and issues apply - if so an equivalent improvement would\nneed to be made there of course)\n\nThanks for any thoughts,\nTao\n"},{"id":"462873","messageId":"xmqqpmg4lbnd.fsf@gitster.g","threadId":"58410","inReplyTo":"CAPMMpogZd_em4_Fdk0sNFqAXqH19VOVyw3WsNT2LHsQNOb0_rw@mail.gmail.com","subject":"Re: git mergetool, merge.tool, merge.guitool and DISPLAY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-09T17:07:18Z","receivedAt":"2022-09-09T17:07:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tao Klerks <tao@klerks.biz> writes:\n\n> When you configure your tools, you can configure \"merge.tool\" for the\n> default, and \"merge.guitool\" for GUI contexts - so far so good, sounds\n> consistent.\n>\n> However, once you've configured these two settings, \"git mergetool\"\n> will never select the GUI tool you've configured unless you very\n> *explicitly* tell it to, by specifying the --gui argument. The\n> sensible auto-selection based on \"DISPLAY\" disappears.\n\nWe've had merge.tool almost forever but merge.guitool is a more\nrecent invention in late 2018.\n\n> The upshot, as I understand it, is that the only way to get a GUI when\n> I'm connected with an X session, and get a terminal-based mergetool\n> when I'm not, without having to be aware and issue different commands,\n> is to accept whatever tooling default order is hardcoded in\n> \"git-mergetool--lib.sh\"\n\n60aced3d (mergetool: fallback to tool when guitool unavailable,\n2019-04-29) says something interesting:\n\n    The behavior for when difftool or mergetool are called without `--gui`\n    should be identical with or without this patch.\n\nSo, either we broke that promise since then, or the above commit was\nalready broken, or the tool was already broken before that?  In any\ncase, I do not think of a good reason why configured .guitool is not\nautomatically honored and .tool ignored when we know we are in an GUI\nenvironment.  In other words, the choice of the tool should probably\ngo like:\n\n    are we in GUI? (determined by an explicit --gui, --no-gui, or env)\n    if so\n\tpick one from configured .guitool (or from the fallback default\n\tlist of tools)\n    else\n        pick one from configured .tool (or from the fallback default\n        list of non-GUI tools)\n\nI would think.\n\n> Is this intentional / is there any logic here, or is this just\n> unfortunate, a result of the auto-selection evolving more\n> intelligently than the built-in explicit \"--gui\" selection, over\n> time??\n"},{"id":"462879","messageId":"CAPMMpoj9ww18j4gUKhXsLFUGrYsbuq6Aww4z9VDboA3h78eHsw@mail.gmail.com","threadId":"58410","inReplyTo":"xmqqpmg4lbnd.fsf@gitster.g","subject":"Re: git mergetool, merge.tool, merge.guitool and DISPLAY","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-09-09T18:09:03Z","receivedAt":"2022-09-09T18:09:19Z","isPatch":false,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Fri, Sep 9, 2022 at 7:07 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Tao Klerks <tao@klerks.biz> writes:\n>\n> > The upshot, as I understand it, is that the only way to get a GUI when\n> > I'm connected with an X session, and get a terminal-based mergetool\n> > when I'm not, without having to be aware and issue different commands,\n> > is to accept whatever tooling default order is hardcoded in\n> > \"git-mergetool--lib.sh\"\n>\n> 60aced3d (mergetool: fallback to tool when guitool unavailable,\n> 2019-04-29) says something interesting:\n>\n>     The behavior for when difftool or mergetool are called without `--gui`\n>     should be identical with or without this patch.\n>\n> So, either we broke that promise since then, or the above commit was\n> already broken, or the tool was already broken before that?\n\nThanks for the quick feedback!\n\nI don't think that promise was then, or is now, broken. The behavior\nwithout `--gui` is to always use the merge.tool (and never the\nmerge.guitool). That is in fact my complaint - that I believe there\nshould be an auto-selection of merge.tool or merge.guitool, depending\non the DISPLAY variable, when neither `--gui` nor `--no-gui` have been\nspecified.\n\nMy proposed behavior would be different to the current and original\nbehavior, for users that have added a merge.guitool to their config in\nthe meantime. If DISPLAY-sensitivity had been added at the time\n(before any users had a merge.guitool) it would have been a safe\nchange without change in behavior for existing users, but now that\nsome users do already have a merge.guitool, that is no longer the\ncase.\n\n> In any\n> case, I do not think of a good reason why configured .guitool is not\n> automatically honored and .tool ignored when we know we are in an GUI\n> environment.  In other words, the choice of the tool should probably\n> go like:\n>\n>     are we in GUI? (determined by an explicit --gui, --no-gui, or env)\n>     if so\n>         pick one from configured .guitool (or from the fallback default\n>         list of tools)\n>     else\n>         pick one from configured .tool (or from the fallback default\n>         list of non-GUI tools)\n>\n> I would think.\n\nExcellent, I agree. My concern then is whether this behavior should be\nplaced behind a new config switch, to avoid surprising users who might\nhave come to expect the current (in my opinion suboptimal) behavior,\nor whether invoking the merge.guitool instead of the merge.tool, when\nthere is a merge.guitool of course and the DISPLAY is set, would be a\nnet improvement even for those users and should just be implemented.\n"},{"id":"462883","messageId":"xmqqmtb8jsej.fsf@gitster.g","threadId":"58410","inReplyTo":"CAPMMpoj9ww18j4gUKhXsLFUGrYsbuq6Aww4z9VDboA3h78eHsw@mail.gmail.com","subject":"Re: git mergetool, merge.tool, merge.guitool and DISPLAY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-09T18:48:20Z","receivedAt":"2022-09-09T18:48:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tao Klerks <tao@klerks.biz> writes:\n\n> Excellent, I agree. My concern then is whether this behavior should be\n> placed behind a new config switch, to avoid surprising users who might\n> have come to expect the current (in my opinion suboptimal) behavior,\n> or whether invoking the merge.guitool instead of the merge.tool, when\n> there is a merge.guitool of course and the DISPLAY is set, would be a\n> net improvement even for those users and should just be implemented.\n\nI wonder if we can think of a new config knob as giving the default\nvalue for the --gui option that is a tristate <false, auto, true>.\nWhen the knob is not set, it default to 'false', which is the\ncurrent behaviour.  The default most likely to be the most sensible\nwould be 'auto' which is \"see the environment to guess if use of GUI\nis appropriate\".\n\n"},{"id":"464723","messageId":"CAPMMpojtZDiJcTgQDrnXuOvDrR2RB30BdyhNZB4MmryO0RxJ=g@mail.gmail.com","threadId":"58410","inReplyTo":"xmqqmtb8jsej.fsf@gitster.g","subject":"Re: git mergetool, merge.tool, merge.guitool and DISPLAY","fromName":"Tao Klerks","fromEmail":"tao@klerks.biz","sentAt":"2022-10-12T16:03:47Z","receivedAt":"2022-10-12T16:04:02Z","isPatch":false,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"On Fri, Sep 9, 2022 at 8:48 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> I wonder if we can think of a new config knob as giving the default\n> value for the --gui option that is a tristate <false, auto, true>.\n> When the knob is not set, it default to 'false', which is the\n> current behaviour.  The default most likely to be the most sensible\n> would be 'auto' which is \"see the environment to guess if use of GUI\n> is appropriate\".\n>\n\nI just submitted an RFC patch along those lines. I have a few\n\"niggles\" I'd like feedback on, but in general I'm happy with it.\n"}]}