{"thread":{"id":"9410","subject":"core.pager handling broken","startedAt":"2007-08-06T15:56:43Z","lastAt":"2007-08-06T23:26:16Z","messageCount":5,"participants":["Brian Gernhardt","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"50027","messageId":"2BA49414-54E0-4353-B237-7799B675FAE8@silverinsanity.com","threadId":"9410","inReplyTo":null,"subject":"core.pager handling broken","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-08-06T15:56:43Z","receivedAt":"2007-08-06T15:56:43Z","isPatch":false,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"I don't have time to look into it, but something broke the core.pager  \nvariable.  I've bisected it down to  \n\"e90fdc39b6903502192b2dd11e5503cea721a1ad: Clean up work-tree  \nhandling\".  With this commit:\n\n$ git config core.pager\ntig\n$ git log\n# Uses less\n$ GIT_PAGER=tig git log\n# Uses tig\n\nI don't have time to track it down right now (I'm off visiting  \nfamily), but will look into it tomorrow unless someone thinks it's  \nobvious and points it out before then.\n\n~~ Brian\n"},{"id":"50032","messageId":"Pine.LNX.4.64.0708061831070.14781@racer.site","threadId":"9410","inReplyTo":"2BA49414-54E0-4353-B237-7799B675FAE8@silverinsanity.com","subject":"[PATCH] check_repository_format_version(): run git_default_config() again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-08-06T17:32:16Z","receivedAt":"2007-08-06T17:32:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nI broke this in \"e90fdc39: Clean up work-tree handling\".  It is\nneeded, for example for interpreting core.pager before starting the\npager.\n\nNoticed by Brian Gernhardt.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tMy first commit with git-gui!  Yeah!\n\n\tOn Mon, 6 Aug 2007, Brian Gernhardt wrote:\n\n\t> I don't have time to look into it, but something broke the \n\t> core.pager variable.  I've bisected it down to \n\t> \"e90fdc39b6903502192b2dd11e5503cea721a1ad: Clean up work-tree \n\t> handling\".\n\n\tThanks.\n\n setup.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 4945eb3..35fa031 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -355,7 +355,8 @@ int check_repository_format_version(const char *var, const char *value)\n \t\t\tfree(git_work_tree_cfg);\n \t\tgit_work_tree_cfg = xstrdup(value);\n \t\tinside_work_tree = -1;\n-\t}\n+\t} else\n+\t\treturn git_default_config(var, value);\n \treturn 0;\n }\n \n-- \n1.5.3.rc4.17.gb980\n"},{"id":"50041","messageId":"7vsl6wbe9e.fsf@assigned-by-dhcp.cox.net","threadId":"9410","inReplyTo":"7vbqdkcxy3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] check_repository_format_version(): run git_default_config() again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-06T20:26:53Z","receivedAt":"2007-08-06T20:26:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sorry, I do not understand this patch from the commit log\n> description.\n>\n> In the old days in 1.5.2 before either implementation of\n> work-tree stuff, this function never called git_default_config()\n> and presumably \"git log\" worked as expected for Brian.  Why is\n> this call needed _here_, not in some of the callers?\n\nSorry scratch that.  v1.5.2 did not even have core.pager.\n\nI see git_config(git_default_config) is called from\nsetup_git_directory_gently() in the version that introduced\ncore.pager.  Curiously enough it does the config twice, once\nwhere no GIT_DIR is set and it sets inside_work_tree, and\nanother by calling git_config(git_setup_config) -- big thanks\nfor sorting this mess out.\n\nI however have a mild suspicion that this has to be done much\nearlier.\n\nFor example, \"git -p log\" would first call handle_options(),\nwhich calls the setup_pager() logic, and then we call\nhandle_internal_command() which calls setup_git_directory() that\neventually leads to the callchain of finding the .git directory\nand .git/config file to be used.  In that case, when we make the\ncall to setup_pager() from handle_options(), we haven't even\nfigured out if there is a configuration file, let alone reading\nfrom it.\n\nWe could work this around by having the \"we need config -- where\nis it\" logic in setup_pager(), but if we later have new options\nlike -p that affects the way how git wrapper itself behaves\nbased on the configuration, we would need the same \"early config\nread\" logic to support it.\n"},{"id":"50056","messageId":"7vy7go9s84.fsf@assigned-by-dhcp.cox.net","threadId":"9410","inReplyTo":"7vsl6wbe9e.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] check_repository_format_version(): run git_default_config() again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-06T23:08:11Z","receivedAt":"2007-08-06T23:08:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I see git_config(git_default_config) is called ... big thanks\n> for sorting this mess out.\n>\n> I however have a mild suspicion that this has to be done much\n> earlier.\n> ...\n> We could work this around by having the \"we need config -- where\n> is it\" logic in setup_pager(), but if we later have new options\n> like -p that affects the way how git wrapper itself behaves\n> based on the configuration, we would need the same \"early config\n> read\" logic to support it.\n\nOk, how about this as an interim solution?\n\nThis is a minimum change in the sense that it restores the old\nbehaviour of not even reading config in setup_git_directory(),\nbut have the core.pager honored when we know it matters.\n\n---\n\n pager.c |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 3bfed02..8bac9d9 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -31,8 +31,11 @@ void setup_pager(void)\n \n \tif (!isatty(1))\n \t\treturn;\n-\tif (!pager)\n+\tif (!pager) {\n+\t\tif (!pager_program)\n+\t\t\tgit_config(git_default_config);\n \t\tpager = pager_program;\n+\t}\n \tif (!pager)\n \t\tpager = getenv(\"PAGER\");\n \tif (!pager)\n"},{"id":"50058","messageId":"Pine.LNX.4.64.0708070025320.14781@racer.site","threadId":"9410","inReplyTo":"7vy7go9s84.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] check_repository_format_version(): run git_default_config() again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-08-06T23:26:16Z","receivedAt":"2007-08-06T23:26:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 6 Aug 2007, Junio C Hamano wrote:\n\n> This is a minimum change in the sense that it restores the old\n> behaviour of not even reading config in setup_git_directory(),\n> but have the core.pager honored when we know it matters.\n\nLooks obviously correct to me, and much better than relying on other sites \nreading the config.\n\nACK.\n\nCiao,\nDscho\n"}]}