{"thread":{"id":"51346","subject":"[2.22.0] difftool no longer passes through to git diff if diff.tool is unset","startedAt":"2019-06-19T23:54:27Z","lastAt":"2019-06-26T18:08:24Z","messageCount":8,"participants":["Pugh, Logan","Denton Liu","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"377602","messageId":"SN6PR09MB3248331E45EED2DD26CA55528AE50@SN6PR09MB3248.namprd09.prod.outlook.com","threadId":"51346","inReplyTo":null,"subject":"[2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Pugh, Logan","fromEmail":"logan.pugh@austintexas.gov","sentAt":"2019-06-19T23:54:22Z","receivedAt":"2019-06-19T23:54:27Z","isPatch":false,"sender":{"key":"logan.pugh@austintexas.gov","avatar":null},"body":"Note: This issue was originally discussed on this StackOverflow thread: https://stackoverflow.com/q/56675863\n\nPrior to Git version 2.22.0 I was able to use git difftool without configuring diff.tool or merge.tool and it would show the diff using git diff.\n\nE.g. with Git 2.21.0:\n\n~/gits/src/git$ git difftool --no-index color.c color.h\ndiff --git a/color.c b/color.h\nindex ebb222ec33..98894d6a17 100644\n\nIn Git version 2.22.0 an error message about diff.tool not being configured is displayed:\n\n~/gits/src/git$ git difftool --no-index color.c color.h\n\nThis message is displayed because 'diff.tool' is not configured.\nSee 'git difftool --tool-help' or 'git help config' for more details.\n'git difftool' will now attempt to use one of the following tools:\nkompare emerge vimdiff\n\nThe following commit is thought to have caused the regression: https://github.com/git/git/commit/05fb8726cccc74908853c166248ff9b6abdafae5\n\nPlease let me know if I can provide any more info.\n\nThanks,\n\nLogan\n"},{"id":"377603","messageId":"20190620011744.GA3696@archbookpro.localdomain","threadId":"51346","inReplyTo":"SN6PR09MB3248331E45EED2DD26CA55528AE50@SN6PR09MB3248.namprd09.prod.outlook.com","subject":"Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-06-20T01:17:44Z","receivedAt":"2019-06-20T01:17:50Z","isPatch":false,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Logan,\n\nOn Wed, Jun 19, 2019 at 11:54:22PM +0000, Pugh, Logan wrote:\n> Note: This issue was originally discussed on this StackOverflow thread: https://stackoverflow.com/q/56675863\n> \n> Prior to Git version 2.22.0 I was able to use git difftool without configuring diff.tool or merge.tool and it would show the diff using git diff.\n> \n> E.g. with Git 2.21.0:\n> \n> ~/gits/src/git$ git difftool --no-index color.c color.h\n> diff --git a/color.c b/color.h\n> index ebb222ec33..98894d6a17 100644\n> \n> In Git version 2.22.0 an error message about diff.tool not being configured is displayed:\n> \n> ~/gits/src/git$ git difftool --no-index color.c color.h\n> \n> This message is displayed because 'diff.tool' is not configured.\n> See 'git difftool --tool-help' or 'git help config' for more details.\n> 'git difftool' will now attempt to use one of the following tools:\n> kompare emerge vimdiff\n> \n> The following commit is thought to have caused the regression: https://github.com/git/git/commit/05fb8726cccc74908853c166248ff9b6abdafae5\n> \n> Please let me know if I can provide any more info.\n> \n> Thanks,\n> \n> Logan\n\nThanks for the report. I'll take a look at this later today.\n"},{"id":"377605","messageId":"20190620024517.GA24321@archbookpro.localdomain","threadId":"51346","inReplyTo":"20190620011744.GA3696@archbookpro.localdomain","subject":"Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-06-20T02:45:17Z","receivedAt":"2019-06-20T02:45:22Z","isPatch":false,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Wed, Jun 19, 2019 at 09:17:44PM -0400, Denton Liu wrote:\n> Hi Logan,\n> \n> On Wed, Jun 19, 2019 at 11:54:22PM +0000, Pugh, Logan wrote:\n> > Note: This issue was originally discussed on this StackOverflow thread: https://stackoverflow.com/q/56675863\n> > \n> > Prior to Git version 2.22.0 I was able to use git difftool without configuring diff.tool or merge.tool and it would show the diff using git diff.\n> > \n> > E.g. with Git 2.21.0:\n> > \n> > ~/gits/src/git$ git difftool --no-index color.c color.h\n> > diff --git a/color.c b/color.h\n> > index ebb222ec33..98894d6a17 100644\n> > \n> > In Git version 2.22.0 an error message about diff.tool not being configured is displayed:\n> > \n> > ~/gits/src/git$ git difftool --no-index color.c color.h\n> > \n> > This message is displayed because 'diff.tool' is not configured.\n> > See 'git difftool --tool-help' or 'git help config' for more details.\n> > 'git difftool' will now attempt to use one of the following tools:\n> > kompare emerge vimdiff\n> > \n> > The following commit is thought to have caused the regression: https://github.com/git/git/commit/05fb8726cccc74908853c166248ff9b6abdafae5\n> > \n> > Please let me know if I can provide any more info.\n> > \n> > Thanks,\n> > \n> > Logan\n> \n> Thanks for the report. I'll take a look at this later today.\n\nUsing the following command on git.git,\n\n\t$ HOME=/dev/null ./git --exec-path=. difftool --no-index color.c color.h\n\nI did a quick bisect on the issue and it seems like the cause of this\nbug is actually 287ab28bfa (diff: reuse diff setup for --no-index case,\n2019-02-16). I'll do a bit more digging tomorrow.\n\n(CCing Peff into this)\n"},{"id":"377609","messageId":"20190620052125.GA12434@sigill.intra.peff.net","threadId":"51346","inReplyTo":"20190620024517.GA24321@archbookpro.localdomain","subject":"Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-20T05:21:25Z","receivedAt":"2019-06-20T05:25:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 19, 2019 at 10:45:17PM -0400, Denton Liu wrote:\n\n> Using the following command on git.git,\n> \n> \t$ HOME=/dev/null ./git --exec-path=. difftool --no-index color.c color.h\n> \n> I did a quick bisect on the issue and it seems like the cause of this\n> bug is actually 287ab28bfa (diff: reuse diff setup for --no-index case,\n> 2019-02-16). I'll do a bit more digging tomorrow.\n\nI don't know much about how git-difftool works, but it looks like it\nsets GIT_EXTERNAL_DIFF=git-difftool--helper.\n\nPrior to 287ab28bfa, we would not have respected any external diff\ncommand when running git-diff. But after it, we do.\n\nIn the case that he user has not provided --no-index, then this all\nworks as I guess difftool is meant to: it runs the helper and says \"hey,\nyou have not configured this\".\n\nIt seems like the behavior of the above command prior to 287ab28bfa was\nnot intentional. It would run git-diff, expecting it to trigger the\nhelper, but it never did (and instead just did a normal no-index diff).\n\nSo it seems like the new behavior is actually the right thing, as it\nmakes the --no-index case consistent with the regular one? I'm not\nat all clear why you would run \"difftool\" here if you it is not\nconfigured and you just want the straight diff output.\n\n-Peff\n"},{"id":"377691","messageId":"SN6PR09MB3248412C9E5AE197B65A8D878AE40@SN6PR09MB3248.namprd09.prod.outlook.com","threadId":"51346","inReplyTo":"20190620052125.GA12434@sigill.intra.peff.net","subject":"Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Pugh, Logan","fromEmail":"logan.pugh@austintexas.gov","sentAt":"2019-06-20T19:29:36Z","receivedAt":"2019-06-20T19:29:40Z","isPatch":false,"sender":{"key":"logan.pugh@austintexas.gov","avatar":null},"body":"> On Wed, Jun 19, 2019 at 10:45:17PM -0400, Denton Liu wrote:\n> \n>> Using the following command on git.git,\n>> \n>> \t$ HOME=/dev/null ./git --exec-path=. difftool --no-index color.c color.h\n>> \n>> I did a quick bisect on the issue and it seems like the cause of this\n>> bug is actually 287ab28bfa (diff: reuse diff setup for --no-index case,\n>> 2019-02-16). I'll do a bit more digging tomorrow.\n> \n> I don't know much about how git-difftool works, but it looks like it\n> sets GIT_EXTERNAL_DIFF=git-difftool--helper.\n> \n> Prior to 287ab28bfa, we would not have respected any external diff\n> command when running git-diff. But after it, we do.\n> \n> In the case that he user has not provided --no-index, then this all\n> works as I guess difftool is meant to: it runs the helper and says \"hey,\n> you have not configured this\".\n> \n> It seems like the behavior of the above command prior to 287ab28bfa was\n> not intentional. It would run git-diff, expecting it to trigger the\n> helper, but it never did (and instead just did a normal no-index diff).\n> \n> So it seems like the new behavior is actually the right thing, as it\n> makes the --no-index case consistent with the regular one? I'm not\n> at all clear why you would run \"difftool\" here if you it is not\n> configured and you just want the straight diff output.\n> \n> -Peff\n\n\nHi Peff,\n\nThanks for the explanation. It sounds like I was under the incorrect \nassumption that I could use the difftool command the same way as the \ndiff command. Part of my confusion could be blamed on the git-difftool \ndocumentation (https://git-scm.com/docs/git-difftool) which near the top \nstates:\n\n > git difftool is a frontend to git diff and accepts the same options \nand arguments. See git-diff[1].\n\nMy use case is a CLI program I've written that processes and then \ncompares two arbitrary files using the git difftool apparatus as \nconfigured by the end user, leaving the choice to them whether to use \nthe internal diff tool or an external tool.\n\nNow, if I'm understanding correctly, I should not rely on the behavior \nof git difftool --no-index passing through to git diff. I could add \nanother CLI switch and code path to my program that calls git diff \ndirectly instead of git difftool but the passthrough behavior seemed \nmore elegant at the time.\n\nIdeally, in my mind, git difftool should work as it says on the tin, as \na straight up passthrough to git diff *unless* explicitly configured to \nuse external tools (e.g. diff.tool and diff.guitool).\n\nHopefully that makes sense, please let me know if I am off-base.\n\nThanks,\n\nLogan\n"},{"id":"378042","messageId":"20190625213545.GA23411@sigill.intra.peff.net","threadId":"51346","inReplyTo":"SN6PR09MB3248412C9E5AE197B65A8D878AE40@SN6PR09MB3248.namprd09.prod.outlook.com","subject":"Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-25T21:35:45Z","receivedAt":"2019-06-25T21:35:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 20, 2019 at 07:29:36PM +0000, Pugh, Logan wrote:\n\n> Thanks for the explanation. It sounds like I was under the incorrect \n> assumption that I could use the difftool command the same way as the \n> diff command. Part of my confusion could be blamed on the git-difftool \n> documentation (https://git-scm.com/docs/git-difftool) which near the top \n> states:\n\nWell, it _is_ true that you can use it the same way. It's just that you\nneed to configure it to use whatever 3rd-party tool you want (and if you\ndo not want to configure a tool, then you are better off just using\ngit-diff directly). It was only due to a bug/historical accident that it\nbehaved just like git-diff in the no-index case (but not in the regular\ncase -- AFAICT, that would have been broken for your script always).\n\n> My use case is a CLI program I've written that processes and then \n> compares two arbitrary files using the git difftool apparatus as \n> configured by the end user, leaving the choice to them whether to use \n> the internal diff tool or an external tool.\n> \n> Now, if I'm understanding correctly, I should not rely on the behavior \n> of git difftool --no-index passing through to git diff. I could add \n> another CLI switch and code path to my program that calls git diff \n> directly instead of git difftool but the passthrough behavior seemed \n> more elegant at the time.\n> \n> Ideally, in my mind, git difftool should work as it says on the tin, as \n> a straight up passthrough to git diff *unless* explicitly configured to \n> use external tools (e.g. diff.tool and diff.guitool).\n\nThat does make some sense to me for your use case. But I'm worried it\nwould be a worse experience for people new to difftool (they run it and\nscratch their heads why it does not do anything different, whereas now\nthey get walked through an interactive configuration).\n\nI dunno. I do not use difftool myself, so I don't have strong opinions.\n\nIn the meantime, I think you can probably switch behavior in your script\nby checking if the diff.tool config is set. It might be nice if difftool\nhad a better way to query that without you having to know if it's\nconfigured. Or in your case I suppose even better would just be an\noption like \"--if-not-configured-just-use-regular-diff\". Then it would\ndo what you want, without impacting users who do want the interactive\nsetup.\n\n-Peff\n"},{"id":"378044","messageId":"BL0PR0901MB4466FF7338C085ADBBF594898AE30@BL0PR0901MB4466.namprd09.prod.outlook.com","threadId":"51346","inReplyTo":"20190625213545.GA23411@sigill.intra.peff.net","subject":"Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Pugh, Logan","fromEmail":"logan.pugh@austintexas.gov","sentAt":"2019-06-25T23:09:08Z","receivedAt":"2019-06-25T23:09:12Z","isPatch":false,"sender":{"key":"logan.pugh@austintexas.gov","avatar":null},"body":"> Well, it _is_ true that you can use it the same way. It's just that you\n> need to configure it to use whatever 3rd-party tool you want (and if you\n> do not want to configure a tool, then you are better off just using\n> git-diff directly). It was only due to a bug/historical accident that it\n> behaved just like git-diff in the no-index case (but not in the regular\n> case -- AFAICT, that would have been broken for your script always).\n\nYes, it would seem that I had only stumbled upon the broken behavior \nbecause of my --no-index use case, but at least the inconsistency is fixed.\n\n> That does make some sense to me for your use case. But I'm worried it\n> would be a worse experience for people new to difftool (they run it and\n> scratch their heads why it does not do anything different, whereas now\n> they get walked through an interactive configuration).\n\nThat is a fair point. UX matters even for CLI programs. Prior to it \nbeing fixed, I myself was confused as to why I was getting a git-diff \noutput when trying to use an external tool with git-difftool but hadn't \nconfigured it correctly. At least now there is some feedback when the \nconfiguration is invalid.\n\n> In the meantime, I think you can probably switch behavior in your script\n> by checking if the diff.tool config is set. It might be nice if difftool\n> had a better way to query that without you having to know if it's\n> configured.\n\nWhat I ended up doing in my app was just requiring the user to be \nexplicit (via separate arguments) about whether they wanted to use \ngit-diff or git-difftool. My app also accepts additional arguments that \nget passed through to git-diff/difftool and I didn't want to have to \ncheck those in addition to git-config. I think that would have been \nsignificantly more complex to implement.\n\n> Or in your case I suppose even better would just be an\n> option like \"--if-not-configured-just-use-regular-diff\". Then it would\n> do what you want, without impacting users who do want the interactive\n> setup.\n\nIf such an option was considered I would be in favor of it. Maybe call \nit \"--no-tutorial\" or perhaps \"--diff-fallback\".\n\nBut having fixed my app, I'm content with the status quo too, now.\n\nThanks,\n\nLogan\n"},{"id":"378073","messageId":"20190626180821.GA23857@sigill.intra.peff.net","threadId":"51346","inReplyTo":"BL0PR0901MB4466FF7338C085ADBBF594898AE30@BL0PR0901MB4466.namprd09.prod.outlook.com","subject":"Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-26T18:08:21Z","receivedAt":"2019-06-26T18:08:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 25, 2019 at 11:09:08PM +0000, Pugh, Logan wrote:\n\n> > Or in your case I suppose even better would just be an\n> > option like \"--if-not-configured-just-use-regular-diff\". Then it would\n> > do what you want, without impacting users who do want the interactive\n> > setup.\n> \n> If such an option was considered I would be in favor of it. Maybe call \n> it \"--no-tutorial\" or perhaps \"--diff-fallback\".\n> \n> But having fixed my app, I'm content with the status quo too, now.\n\nYeah, those are definitely better names. :)\n\nI think we're on the same page about a good path forward, then. I don't\nplan to work on this myself, but maybe it would be a good #leftoverbits\ncandidate for somebody wanting to get started on modifying Git.\n\n-Peff\n"}]}