{"thread":{"id":"20894","subject":"bash_completion outside repo","startedAt":"2009-09-10T15:13:13Z","lastAt":"2009-09-13T20:40:14Z","messageCount":20,"participants":["james bardin","Michael J Gruber","Todd Zullinger","Jeff King","Shawn O. Pearce","Junio C Hamano","Bert Wesarg"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"122813","messageId":"a3b675320909100813i3e70ab3at66408aebb9952c7d@mail.gmail.com","threadId":"20894","inReplyTo":null,"subject":"bash_completion outside repo","fromName":"james bardin","fromEmail":"jbardin@bu.edu","sentAt":"2009-09-10T15:13:13Z","receivedAt":"2009-09-10T15:13:13Z","isPatch":false,"sender":{"key":"jbardin@bu.edu","avatar":null},"body":"Hi,\n\nI'm making a git rpm for our site, and I'm getting an error with\nbash_completion on RHEL/CentOS 5.\n\nWhen outside a repo, any completion causes git to spit out\n fatal: error processing config file(s)\n\nThis is 1.6.4.2 using the contrib bash completion file\n\nThanks\n-jim\n"},{"id":"122885","messageId":"4AAA4C87.6010900@drmicha.warpmail.net","threadId":"20894","inReplyTo":"a3b675320909100813i3e70ab3at66408aebb9952c7d@mail.gmail.com","subject":"Re: bash_completion outside repo","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-09-11T13:11:35Z","receivedAt":"2009-09-11T13:11:35Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"james bardin venit, vidit, dixit 10.09.2009 17:13:\n> Hi,\n> \n> I'm making a git rpm for our site, and I'm getting an error with\n> bash_completion on RHEL/CentOS 5.\n> \n> When outside a repo, any completion causes git to spit out\n> fatal: error processing config file(s)\n> \n> This is 1.6.4.2 using the contrib bash completion file\n> \n> Thanks\n> -jim\n\nI can't reproduce this with git version 1.6.5.rc0.164.g5f6b0 on Fedora 11.\n\nWhich exact steps reproduce it for you, and which relevant settings (PS1\nand GIT_PS1_...) do you use? Do you have a system or global .gitconfig?\n\nMichael\n"},{"id":"122886","messageId":"20090911133313.GF2582@inocybe.localdomain","threadId":"20894","inReplyTo":"a3b675320909100813i3e70ab3at66408aebb9952c7d@mail.gmail.com","subject":"Re: bash_completion outside repo","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2009-09-11T13:33:13Z","receivedAt":"2009-09-11T13:33:13Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"james bardin wrote:\n> I'm making a git rpm for our site, and I'm getting an error with\n> bash_completion on RHEL/CentOS 5.\n\nOut of curiosity, have you tried rebuilding the Fedora packages from\nrawhide? They should work on RHEL/CentOS 5 just fine (I use them\nmyself).\n\n> When outside a repo, any completion causes git to spit out\n> fatal: error processing config file(s)\n>\n> This is 1.6.4.2 using the contrib bash completion file\n\nI can't reproduce this on either Fedora or CentOS.\n\n-- \nTodd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nA great many people think they are thinking when they are merely\nrearranging their prejudices.\n -- William James\n\n"},{"id":"122889","messageId":"a3b675320909110700k7eb7286qc8cb5691aae214c7@mail.gmail.com","threadId":"20894","inReplyTo":"20090911133313.GF2582@inocybe.localdomain","subject":"Re: bash_completion outside repo","fromName":"james bardin","fromEmail":"jbardin@bu.edu","sentAt":"2009-09-11T14:00:33Z","receivedAt":"2009-09-11T14:00:33Z","isPatch":false,"sender":{"key":"jbardin@bu.edu","avatar":null},"body":"On Fri, Sep 11, 2009 at 9:33 AM, Todd Zullinger wrote:\n> james bardin wrote:\n>> I'm making a git rpm for our site, and I'm getting an error with\n>> bash_completion on RHEL/CentOS 5.\n>\n> Out of curiosity, have you tried rebuilding the Fedora packages from\n> rawhide?  They should work on RHEL/CentOS 5 just fine (I use them\n> myself).\n>\n\nI was trying to prune out some of the dependencies to make internal\ndeployment easier.\nI bundled the doc tarballs instead of requiring asciidoc, and git\nincludes its own perl(Error) if it's not available.\n\n\n\nOn Fri, Sep 11, 2009 at 9:11 AM, Michael J Gruber\n wrote:\n\n> I can't reproduce this with git version 1.6.5.rc0.164.g5f6b0 on Fedora 11.\n>\n> Which exact steps reproduce it for you, and which relevant settings (PS1\n> and GIT_PS1_...) do you use? Do you have a system or global .gitconfig?\n>\n\nI did a make install, and dropped the completion file in\n/etc/bash_completion.d/. No other settings changed. I did a quick\ncheck, and it happens with the current 1.6.5 snapshot too, and on a\nfedora 10 box I found.\n\nIt seems I only get this error if I don't have a global config.\nTouching ~/.gitconfig stops the error.\n"},{"id":"122890","messageId":"20090911141730.GA384@coredump.intra.peff.net","threadId":"20894","inReplyTo":"a3b675320909110700k7eb7286qc8cb5691aae214c7@mail.gmail.com","subject":"Re: bash_completion outside repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-11T14:17:30Z","receivedAt":"2009-09-11T14:17:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 11, 2009 at 10:00:33AM -0400, james bardin wrote:\n\n> I did a make install, and dropped the completion file in\n> /etc/bash_completion.d/. No other settings changed. I did a quick\n> check, and it happens with the current 1.6.5 snapshot too, and on a\n> fedora 10 box I found.\n> \n> It seems I only get this error if I don't have a global config.\n> Touching ~/.gitconfig stops the error.\n\nAh, I see. It looks like we use \"git config --list\" to view several bits\nof configuration. However, it is not happy if there is no config file to\nlist.\n\nHowever, I'm not sure that \"config --list\" isn't broken. Inside a repo,\ndoing \"git config --list\" shows the repo config and my global config,\nand exits with no error. Outside a repo, it shows my global config, and\nexits with no error. But if I _don't_ have global config, it produces an\nerror. Shouldn't it treat that as simply \"no config is available\"?\n\nI also question why it is using \"git config --list\" at all in snippets\nlike this:\n\n for i in $(git --git-dir=\"$d\" config --list); do\n case \"$i\" in\n remote.*.url=*)\n i=\"${i#remote.}\"\n echo \"${i/.url=*/}\"\n ;;\n esac\n done\n\ninstead of just using \"git config --get-regexp 'remote\\..*\\.url'\", which\nwould be slightly more efficient, and also doesn't have this bug. ;)\n\n-Peff\n"},{"id":"122891","messageId":"20090911143651.GE1033@spearce.org","threadId":"20894","inReplyTo":"20090911141730.GA384@coredump.intra.peff.net","subject":"Re: bash_completion outside repo","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-09-11T14:36:51Z","receivedAt":"2009-09-11T14:36:51Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jeff King wrote:\n> I also question why it is using \"git config --list\" at all in snippets\n> like this:\n> \n> for i in $(git --git-dir=\"$d\" config --list); do\n> case \"$i\" in\n> remote.*.url=*)\n> i=\"${i#remote.}\"\n> echo \"${i/.url=*/}\"\n> ;;\n> esac\n> done\n> \n> instead of just using \"git config --get-regexp 'remote\\..*\\.url'\", which\n> would be slightly more efficient, and also doesn't have this bug. ;)\n\nF'king oversight. You are right, this should be --get-regexp.\nThere isn't a reason here, probably other than \"I forgot about\n--get-regexp when I wrote the original code\".\n\n-- \nShawn.\n"},{"id":"122894","messageId":"20090911150934.GB977@coredump.intra.peff.net","threadId":"20894","inReplyTo":"20090911143651.GE1033@spearce.org","subject":"Re: bash_completion outside repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-11T15:09:34Z","receivedAt":"2009-09-11T15:09:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 11, 2009 at 07:36:51AM -0700, Shawn O. Pearce wrote:\n\n> > instead of just using \"git config --get-regexp 'remote\\..*\\.url'\", which\n> > would be slightly more efficient, and also doesn't have this bug. ;)\n> \n> F'king oversight. You are right, this should be --get-regexp.\n> There isn't a reason here, probably other than \"I forgot about\n> --get-regexp when I wrote the original code\".\n\nOK. I will leave a bash-completion patch for you (or somebody else\ninterested) as I don't use it myself.\n\nAssuming you make such a patch, that will clear up the original issue. I\nwonder if we should fix \"git config --list\". The current semantics seem\na little crazy to me, but it is a scriptable interface. I'm inclined to\ncall this a bug, though.\n\n-Peff\n"},{"id":"122901","messageId":"20090911164730.GA21536@coredump.intra.peff.net","threadId":"20894","inReplyTo":"20090911150934.GB977@coredump.intra.peff.net","subject":"Re: bash_completion outside repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-11T16:47:30Z","receivedAt":"2009-09-11T16:47:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 11, 2009 at 11:09:34AM -0400, Jeff King wrote:\n\n> Assuming you make such a patch, that will clear up the original issue. I\n> wonder if we should fix \"git config --list\". The current semantics seem\n> a little crazy to me, but it is a scriptable interface. I'm inclined to\n> call this a bug, though.\n\nAnd here is a patch to fix it.\n\n-- >8 --\nSubject: [PATCH] config: treat non-existent config files as empty\n\nThe git_config() function signals error by returning -1 in\ntwo instances:\n\n 1. An actual error occurs in opening a config file (parse\n errors cause an immediate die).\n\n 2. Of the three possible config files, none was found.\n\nHowever, this second case is often not an error at all; it\nsimply means that the user has no configuration (they are\noutside a repo, and they have no ~/.gitconfig file). This\ncan lead to confusing errors, such as when the bash\ncompletion calls \"git config --list\" outside of a repo. If\nthe user has a ~/.gitconfig, the command completes\nsuccesfully; if they do not, it complains to stderr.\n\nThis patch allows callers of git_config to distinguish\nbetween the two cases. Error is signaled by -1, and\notherwise the return value is the number of files parsed.\nThis means that the traditional \"git_config(...) < 0\" check\nfor error should work, but callers who want to know whether\nwe parsed any files or not can still do so.\n\nWe need to tweak one use of git_config in builtin-remote\nthat previously assumed the return value was either '0' or\n'-1'.\n\nSigned-off-by: Jeff King \n---\n\nThis is actually a bit overengineered. Of the hundreds of calls to\ngit_config, there are exactly _two_ which check the return value. And\nneither of them cares whether we parsed files or not; they really only\ncare if there was an error. So we could simply return 0 as long as there\nis no error.\n\nThis also makes me wonder, though. Git can do wildly different things\n(including hard-to-reverse things) based on config (e.g., just consider\ngc.pruneExpire). Yet we call git_config() without ever checking for\nerrors. In the actual parsing routines, we die() if there is an error.\nBut if we fail to open the file due to some transient error, we will\nsilently ignore the situation.\n\nGranted, such transient errors are unlikely. The biggest reasons for\nfailing to open a file are that it doesn't exist, or that we have no\npermission to read it, both of which are treated explicitly in\ngit_config as \"silently ok\". But I wonder if we should simply be dying\non such an error, and git_config() should just have a void return.\n\n builtin-remote.c | 3 ++-\n config.c | 4 +---\n t/t1300-repo-config.sh | 8 ++++++++\n 3 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 0777dd7..3bf1fe8 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -1245,7 +1245,8 @@ static int update(int argc, const char **argv)\n \tfor (i = 1; i < argc; i++) {\n \t\tint groups_found = 0;\n \t\tremote_group.name = argv[i];\n-\t\tresult = git_config(get_remote_group, &groups_found);\n+\t\tif (git_config(get_remote_group, &groups_found) < 0)\n+\t\t\tresult = -1;\n \t\tif (!groups_found && (i != 1 || strcmp(argv[1], \"default\"))) {\n \t\t\tstruct remote *remote;\n \t\t\tif (!remote_is_configured(argv[i]))\ndiff --git a/config.c b/config.c\nindex e87edea..e429674 100644\n--- a/config.c\n+++ b/config.c\n@@ -709,9 +709,7 @@ int git_config(config_fn_t fn, void *data)\n \t\tfound += 1;\n \t}\n \tfree(repo_config);\n-\tif (found == 0)\n-\t\treturn -1;\n-\treturn ret;\n+\treturn ret == 0 ? found : ret;\n }\n \n /*\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 83b7294..db987b7 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -289,6 +289,14 @@ test_expect_success 'working --list' \\\n \t'git config --list > output && cmp output expect'\n \n cat > expect << EOF\n+EOF\n+\n+test_expect_success '--list without repo produces empty output' '\n+\tgit --git-dir=nonexistent config --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect << EOF\n beta.noindent sillyValue\n nextsection.nonewline wow2 for me\n EOF\n-- \n1.6.5.rc0.174.g29a6d.dirty\n"},{"id":"122912","messageId":"7viqfpw6tv.fsf@alter.siamese.dyndns.org","threadId":"20894","inReplyTo":"20090911164730.GA21536@coredump.intra.peff.net","subject":"Re: bash_completion outside repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-11T20:43:24Z","receivedAt":"2009-09-11T20:43:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This is actually a bit overengineered. Of the hundreds of calls to\n> git_config, there are exactly _two_ which check the return value. And\n> neither of them cares whether we parsed files or not; they really only\n> care if there was an error. So we could simply return 0 as long as there\n> is no error.\n>\n> This also makes me wonder, though. Git can do wildly different things\n> (including hard-to-reverse things) based on config (e.g., just consider\n> gc.pruneExpire). Yet we call git_config() without ever checking for\n> errors. In the actual parsing routines, we die() if there is an error.\n> But if we fail to open the file due to some transient error, we will\n> silently ignore the situation.\n>\n> Granted, such transient errors are unlikely. The biggest reasons for\n> failing to open a file are that it doesn't exist, or that we have no\n> permission to read it, both of which are treated explicitly in\n> git_config as \"silently ok\". But I wonder if we should simply be dying\n> on such an error, and git_config() should just have a void return.\n\nThanks.\n\nENOENT should be the same as having an empty file, which is the main point\nof the patch and at the same time why you feel this is overengineered. I\nagree with you on both counts.\n\nWhile I also agree that EACCESS and other failures should be treated as\nfatal in principle for safety, e.g. when running prune without being able\nto read gc.pruneExpire as you mentioned, we would need a tradeoff between\nsafety and convenience. When asked to help looking at a complex merge in\na colleages' repository, do you want your \"git diff\" to refuse to run only\nbecause her .git/config cannot be read by you? Of course, you _can_ work\nthis particular one around by various means (e.g. prefixing GIT_CONFIG=...\nto force ignoring the file; telling the colleage that she'd better make\nher .git/config readable to you if she wants your help), if either one of\nthe owner of the .git/config file or the party that wants to access the\nrepository is a non-person such a workaround would be harder to arrange.\n\nAlso there was a move going in the opposite direction to allow a config\nfile that is syntactically broken to be handled without the parser dying,\nprimarily to help \"git config -e\". In the longer term, our direction\nshouldn't be adding more die() in the git_config() callchain, but instead\nallowing it to report different kind of failures to let the caller act\nbased on what it does and what the nature of failure is.\n\nFor example, \"gc\" may say \"I won't prune because I had to skip some of the\nlines in your .git/config because you have syntax errors in them, and I\nmay have missed the definition of gc.pruneExpire you may wanted to place\non them\", while \"diff\" may ignore that kind of errors.\n\nHaving said all that, my preference *for now* would be to ignore \"there is\nno $HOME/.gitconfig (or /etc/gitconfig) file\", but catch all other errors\nand die().\n\nThere are some other glitches in the current git_config() callchain.\n\n - No config file anywhere gives an error. I agree with you that this is\n a bug.\n\n - Having GIT_CONFIG=/no/such/file in the environment gives an error,\n which is good.\n\n - config.c::git_parse_file() [*1*] dies when it detects certain file\n format errors itself. This is not good for \"git config -e\", as it\n needs to learn core.editor before it can be used to fix such an error.\n\n This function then calls config.c::get_value() and it dies when\n config.c::get_value() reports any error.\n\n - config.c::get_value() is called by config.c::git_parse_file() to finish\n parsing out the pair, and stores the \"value\" in a form\n usable in the code, e.g. a variable defined in environment.c. The\n function returns an error on some file format errors (e.g. a variable\n name is too long, string quoting unterminated) that signals the calling\n config.c::git_parse_file() to die(). These error returns are good (the\n caller however may need to be fixed for \"config -e\" issue not to die).\n\n It then calls the parse callbacck routines. They return error when\n they detect semantic errors (e.g. \"branch.autosetupmerge = alwys\" is\n not one of the valid values).\n\nThe last one is not the topic of this patch, but it is quite problematic.\nWhen you are interested in finding out what value gc.pruneExpire is set,\nyou do not care (as long as the configuration file was syntactically\ncorrect and you did not have to skip any file you were supposed to read\ndue to EACCESS) if \"branch.autosetupmerge\" has an invalid value.\n\nA possible longer term solution would be to:\n\n - Change the signature of callbacks (e.g. git_default_branch_config()) so\n that they return void. They are not allowed to report any semantic\n errors while parsing.\n\n - Instead, they use special \"INVALID\" value and store that when they see\n a semantically incorrect value. They may also want to store what the\n actual string the config file gave them for later reporting, together\n with the name of and the line number in the config file for diagnostic\n purposes.\n\n - The user of the internalized value (i.e. \"git grep git_branch_track\"\n shows there are only two, cmd_branch() and cmd_checkout()) must check\n for the above INVALID value before they use the variable, and die at\n the point of the use.\n\nI'll send an illustration patch separately.\n\n[Footnote]\n\n*1* What a horrible name for this function! It is static so git_ prefix\nis unneeded, and if it anticipates it might get someday exported, parse_file\nis too generic and should be named git_parse_config_file().\n"},{"id":"122915","messageId":"20090911211209.GA26527@coredump.intra.peff.net","threadId":"20894","inReplyTo":"7viqfpw6tv.fsf@alter.siamese.dyndns.org","subject":"Re: bash_completion outside repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-11T21:12:09Z","receivedAt":"2009-09-11T21:12:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 11, 2009 at 01:43:24PM -0700, Junio C Hamano wrote:\n\n> While I also agree that EACCESS and other failures should be treated as\n> fatal in principle for safety, e.g. when running prune without being able\n> to read gc.pruneExpire as you mentioned, we would need a tradeoff between\n> safety and convenience. When asked to help looking at a complex merge in\n> a colleages' repository, do you want your \"git diff\" to refuse to run only\n> because her .git/config cannot be read by you? Of course, you _can_ work\n\nSorry, I think I was a bit unclear in the original message. There are\ntwo classes of errors right now:\n\n 1. access(fn, R_OK) != 0 (i.e., ENOENT and EACCESS)\n\n 2. fopen(fn, \"r\") != 0\n\nIn the case of (1), we treat it as an empty file (except in the case\nthat _all_ files fail (1), in which case we reported an error, which is\nreally stupid and is what this patch fixes).\n\nIn the case of (2), we treat it as an error, but because nobody actually\nbothers to check the error code, it is effectively ignored. What I was\nthinking is that (2) should be promoted to die(), and leave (1) as-is.\n\nSo I think in your example it would fall under (1), and we both agree\nthat should be allowed.\n\nBut:\n\n> Also there was a move going in the opposite direction to allow a config\n> file that is syntactically broken to be handled without the parser dying,\n> primarily to help \"git config -e\". In the longer term, our direction\n> shouldn't be adding more die() in the git_config() callchain, but instead\n> allowing it to report different kind of failures to let the caller act\n> based on what it does and what the nature of failure is.\n\nYeah, that is the opposite of what I proposed above. But it is a step\ntowards lib-ifying the config code, which is probably a good thing. The\nconfig code is an utter mess. I am a little hesitant to touch it just\nbecause I don't think there is anything _broken_ in it exactly, but\nevery time I look at it, I am always shocked by how unnecessarily\ncomplex it is.\n\nI think the \"right\" thing to do would probably be to have a lib-ified\nfunction to read the config, and then have a wrapper that 99% of the\nprograms use that just checks the error return and calls die() if there\nis a problem. But such a cleanup is likely to introduce new bugs, so I\nhave let it be (also, because my time is not infinite and there are\nother more interesting things to work on ;) ).\n\n> For example, \"gc\" may say \"I won't prune because I had to skip some of the\n> lines in your .git/config because you have syntax errors in them, and I\n> may have missed the definition of gc.pruneExpire you may wanted to place\n> on them\", while \"diff\" may ignore that kind of errors.\n\nYeah, that makes sense to me, and should be possible with a decent\nlib-ified interface.\n\n> Having said all that, my preference *for now* would be to ignore \"there is\n> no $HOME/.gitconfig (or /etc/gitconfig) file\", but catch all other errors\n> and die().\n\nOK, then I think we are on the same page.\n\n> There are some other glitches in the current git_config() callchain.\n> \n> - No config file anywhere gives an error. I agree with you that this is\n> a bug.\n\nYep, and this patch fixes that.\n\n> - Having GIT_CONFIG=/no/such/file in the environment gives an error,\n> which is good.\n\nYep, and and this patch should leave that untouched (I didn't test that\nspecifically, but I checked that \"git config --global\" does, and I\nassume they use the same code path. Of course, one never knows...).\n\n> A possible longer term solution would be to:\n> \n> - Change the signature of callbacks (e.g. git_default_branch_config()) so\n> that they return void. They are not allowed to report any semantic\n> errors while parsing.\n> \n> - Instead, they use special \"INVALID\" value and store that when they see\n> a semantically incorrect value. They may also want to store what the\n> actual string the config file gave them for later reporting, together\n> with the name of and the line number in the config file for diagnostic\n> purposes.\n> \n> - The user of the internalized value (i.e. \"git grep git_branch_track\"\n> shows there are only two, cmd_branch() and cmd_checkout()) must check\n> for the above INVALID value before they use the variable, and die at\n> the point of the use.\n\nThat all makes sense to me. My biggest worry is that we will need to be\nchecking for \"INVALID\" values in lots of places before actually using\nthe value from a variable. IOW, it is nice for code to be able to call\ninto some library call that respects a config variable without having to\ncare about doing some magic setup to say \"Oh, by the way, I am\ninteresting in the value of diff.foo\". At the same time, it is nice for\nthe library code to not have to say \"We're using diff.foo. Let's make\nsure somebody has checked the value before using it.\"\n\nIn other words, I would like not-too-syntactically-painful lazy values.\nWith no runtime overhead. In C. ;)\n\n-Peff\n"},{"id":"122916","messageId":"7vvdjpuqgx.fsf@alter.siamese.dyndns.org","threadId":"20894","inReplyTo":"7viqfpw6tv.fsf@alter.siamese.dyndns.org","subject":"Re: bash_completion outside repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-11T21:22:06Z","receivedAt":"2009-09-11T21:22:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> The last one is not the topic of this patch, but it is quite problematic.\n> When you are interested in finding out what value gc.pruneExpire is set,\n> you do not care (as long as the configuration file was syntactically\n> correct and you did not have to skip any file you were supposed to read\n> due to EACCESS) if \"branch.autosetupmerge\" has an invalid value.\n>\n> A possible longer term solution would be to:\n>\n> - Change the signature of callbacks (e.g. git_default_branch_config()) so\n> that they return void. They are not allowed to report any semantic\n> errors while parsing.\n>\n> - Instead, they use special \"INVALID\" value and store that when they see\n> a semantically incorrect value. They may also want to store what the\n> actual string the config file gave them for later reporting, together\n> with the name of and the line number in the config file for diagnostic\n> purposes.\n>\n> - The user of the internalized value (i.e. \"git grep git_branch_track\"\n> shows there are only two, cmd_branch() and cmd_checkout()) must check\n> for the above INVALID value before they use the variable, and die at\n> the point of the use.\n>\n> I'll send an illustration patch separately.\n\nSo here is an illustration to handle _only_ a misspelled\nbranch.autosetupmerge.\n\nIf you have this in your .git/config file:\n\n\t[branch]\n \tautosetupmerge = nevver\n\nyou cannot run \"git diff\" without this patch. But with this patch, only\nthe commands that _care_ about this misconfiguration would notice and\nreport.\n\nThe new world order is:\n\n - The config parsing callback should detect semantic errors, but should\n not return non-zero, to let the parser continue.\n\n In the _real_ patch, their signatures should be changed to return void.\n I wanted to illustrate more important points in this patch so I didn't\n do that.\n\n - Instead, when they detect an error in configured values, they make a\n note that the value is bad (e.g. I introduced BRANCH_TRACK_CONFIG_ERROR\n for this), and also can ask record_bad_config() to record this fact for\n possible later reporting purposes. You also _could_ issue error() to\n make it easier to detect unrelated but bad configuration for users, but\n that is secondary and I didn't do that in this illustration.\n\n - Then, when the values are actually _used_, the users should take notice\n of the situation (e.g. both cmd_checkout() and cmd_branch() codepaths\n are modified to do this) and ask die_with_bad_config() to report the\n error the parser detected earlier.\n\nThis way, commands that do not use the value of branch.autosetupmerge,\ne.g. \"git diff\", won't have to abort.\n\n builtin-branch.c | 5 +++-\n builtin-checkout.c | 5 +++-\n cache.h | 7 ++++++\n config.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++---\n 4 files changed, 68 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 9f57992..10010a7 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -635,9 +635,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\trename_branch(head, argv[0], rename > 1);\n \telse if (rename && (argc == 2))\n \t\trename_branch(argv[0], argv[1], rename > 1);\n-\telse if (argc <= 2)\n+\telse if (argc <= 2) {\n+\t\tif (track == BRANCH_TRACK_CONFIG_ERROR)\n+\t\t\tdie_with_bad_config(\"branch.autosetupmerge\");\n \t\tcreate_branch(head, argv[0], (argc == 2) ? argv[1] : head,\n \t\t\t force_create, reflog, track);\n+\t}\n \telse\n \t\tusage_with_options(builtin_branch_usage, options);\n \ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex d050c37..3bf57a5 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -630,8 +630,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\topts.new_branch = argv0 + 1;\n \t}\n \n-\tif (opts.track == BRANCH_TRACK_UNSPECIFIED)\n+\tif (opts.track == BRANCH_TRACK_UNSPECIFIED) {\n+\t\tif (git_branch_track == BRANCH_TRACK_CONFIG_ERROR)\n+\t\t\tdie_with_bad_config(\"branch.autosetupmerge\");\n \t\topts.track = git_branch_track;\n+\t}\n \tif (conflict_style) {\n \t\topts.merge = 1; /* implied */\n \t\tgit_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\ndiff --git a/cache.h b/cache.h\nindex 5fad24c..f7ff502 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -533,6 +533,7 @@ enum safe_crlf {\n extern enum safe_crlf safe_crlf;\n \n enum branch_track {\n+\tBRANCH_TRACK_CONFIG_ERROR = -2,\n \tBRANCH_TRACK_UNSPECIFIED = -1,\n \tBRANCH_TRACK_NEVER = 0,\n \tBRANCH_TRACK_REMOTE,\n@@ -890,6 +891,9 @@ extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigne\n /* Dumb servers support */\n extern int update_server_info(int);\n \n+/*\n+ * configuration file management\n+ */\n typedef int (*config_fn_t)(const char *, const char *, void *);\n extern int git_default_config(const char *, const char *, void *);\n extern int git_config_from_file(config_fn_t fn, const char *, void *);\n@@ -910,6 +914,9 @@ extern int git_config_global(void);\n extern int config_error_nonbool(const char *);\n extern const char *config_exclusive_filename;\n \n+extern void record_bad_config(const char *name, const char *value);\n+extern void die_with_bad_config(const char *name);\n+\n #define MAX_GITNAME (1000)\n extern char git_default_email[MAX_GITNAME];\n extern char git_default_name[MAX_GITNAME];\ndiff --git a/config.c b/config.c\nindex e87edea..5b6a929 100644\n--- a/config.c\n+++ b/config.c\n@@ -306,6 +306,41 @@ static void die_bad_config(const char *name)\n \tdie(\"bad config value for '%s'\", name);\n }\n \n+static struct bad_config_note {\n+\tstruct bad_config_note *next;\n+\tconst char *name;\n+\tconst char *value;\n+\tint linenr;\n+\tconst char *filename;\n+} *bad_config_note;\n+\n+void record_bad_config(const char *name, const char *value)\n+{\n+\tstruct bad_config_note *note = xcalloc(1, sizeof(*note));\n+\tnote->next = bad_config_note;\n+\tbad_config_note = note;\n+\tnote->name = xstrdup(name);\n+\tnote->value = xstrdup(value);\n+\tnote->linenr = config_linenr;\n+\tnote->filename = config_file_name ? xstrdup(config_file_name) : NULL;\n+}\n+\n+void die_with_bad_config(const char *name)\n+{\n+\tstruct bad_config_note *note;\n+\tfor (note = bad_config_note; note; note = note->next)\n+\t\tif (!strcmp(name, note->name)) {\n+\t\t\tchar whence[PATH_MAX + 200];\n+\t\t\tif (note->filename)\n+\t\t\t\tsprintf(whence, \" in %s\", note->filename);\n+\t\t\telse\n+\t\t\t\twhence[0] = '\\0';\n+\t\t\tdie(\"bad config value '%s' for '%s'%s, line %d\",\n+\t\t\t note->value, note->name, whence, note->linenr);\n+\t\t}\n+\tdie(\"program error: bad config value for %s not recorded\", name);\n+}\n+\n int git_config_int(const char *name, const char *value)\n {\n \tlong ret = 0;\n@@ -324,6 +359,8 @@ unsigned long git_config_ulong(const char *name, const char *value)\n \n int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n {\n+\tlong ret;\n+\n \t*is_bool = 1;\n \tif (!value)\n \t\treturn 1;\n@@ -333,14 +370,24 @@ int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n \t\treturn 1;\n \tif (!strcasecmp(value, \"false\") || !strcasecmp(value, \"no\") || !strcasecmp(value, \"off\"))\n \t\treturn 0;\n+\n \t*is_bool = 0;\n-\treturn git_config_int(name, value);\n+\n+\tret = 0;\n+\tif (!git_parse_long(value, &ret)) {\n+\t\trecord_bad_config(name, value);\n+\t\t*is_bool = -1;\n+\t}\n+\treturn ret;\n }\n \n int git_config_bool(const char *name, const char *value)\n {\n-\tint discard;\n-\treturn !!git_config_bool_or_int(name, value, &discard);\n+\tint is_bool, val;\n+\tval = git_config_bool_or_int(name, value, &is_bool);\n+\tif (is_bool < 0)\n+\t\treturn is_bool;\n+\treturn !!val;\n }\n \n int git_config_string(const char **dest, const char *var, const char *value)\n@@ -546,11 +593,13 @@ static int git_default_i18n_config(const char *var, const char *value)\n static int git_default_branch_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"branch.autosetupmerge\")) {\n+\t\tint val;\n \t\tif (value && !strcasecmp(value, \"always\")) {\n \t\t\tgit_branch_track = BRANCH_TRACK_ALWAYS;\n \t\t\treturn 0;\n \t\t}\n-\t\tgit_branch_track = git_config_bool(var, value);\n+\t\tval = git_config_bool(var, value);\n+\t\tgit_branch_track = (val < 0) ? BRANCH_TRACK_CONFIG_ERROR : val;\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"branch.autosetuprebase\")) {\n"},{"id":"122917","messageId":"20090911212938.GA26589@coredump.intra.peff.net","threadId":"20894","inReplyTo":"7vvdjpuqgx.fsf@alter.siamese.dyndns.org","subject":"Re: bash_completion outside repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-11T21:29:38Z","receivedAt":"2009-09-11T21:29:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 11, 2009 at 02:22:06PM -0700, Junio C Hamano wrote:\n\n> So here is an illustration to handle _only_ a misspelled\n> branch.autosetupmerge.\n> \n> If you have this in your .git/config file:\n> \n> \t[branch]\n> \tautosetupmerge = nevver\n> \n> you cannot run \"git diff\" without this patch. But with this patch, only\n> the commands that _care_ about this misconfiguration would notice and\n> report.\n\nOK, that example makes sense. But I'm a little dubious of how this\nscales to something like color.diff.plain. Who is responsible for\nchecking? Do we do it at the beginning of every program which cares\nabout diff values? If so, what is the failure mode when we forget (and I\nsuspect we will, because it is easy for programs to call into unexpected\ncode that is three layers deep)?\n\nI think something like that needs to \"belong\" to the diff code itself. I\nguess in the case of \"diff\", we could check all diff-related config at\ndiff setup time. But what about something used in several places, like\ncore.quotepath?\n\n-Peff\n"},{"id":"122918","messageId":"7vr5uduotz.fsf@alter.siamese.dyndns.org","threadId":"20894","inReplyTo":"20090911212938.GA26589@coredump.intra.peff.net","subject":"Re: bash_completion outside repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-11T21:57:28Z","receivedAt":"2009-09-11T21:57:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think something like that needs to \"belong\" to the diff code itself. I\n> guess in the case of \"diff\", we could check all diff-related config at\n> diff setup time.\n\nNot necessarily. You do not want to care about color configuration if you are\ndoing diff --raw for example. The one that first uses the color variable\nshould be able to notice the breakage, no?\n\n> But what about something used in several places, like\n> core.quotepath?\n\nExactly the same way I checked what codepaths needed to fix for the\nautosetupmerge stuff. core.quotepath internally sets quote_path_fully,\nand the sole user of quote_path_fully is sq_must_quote() which is only\nused by next_quote_pos(). So you can have your check very isolated.\n"},{"id":"122920","messageId":"7vmy51uoi9.fsf@alter.siamese.dyndns.org","threadId":"20894","inReplyTo":"7vr5uduotz.fsf@alter.siamese.dyndns.org","subject":"Re: bash_completion outside repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-11T22:04:30Z","receivedAt":"2009-09-11T22:04:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> I think something like that needs to \"belong\" to the diff code itself. I\n>> guess in the case of \"diff\", we could check all diff-related config at\n>> diff setup time.\n>\n> Not necessarily. You do not want to care about color configuration if you are\n> doing diff --raw for example. The one that first uses the color variable\n> should be able to notice the breakage, no?\n>\n>> But what about something used in several places, like\n>> core.quotepath?\n>\n> Exactly the same way I checked what codepaths needed to fix for the\n> autosetupmerge stuff. core.quotepath internally sets quote_path_fully,\n> and the sole user of quote_path_fully is sq_must_quote() which is only\n> used by next_quote_pos(). So you can have your check very isolated.\n\nNote that I was _not_ defending the approach my illustration took. I\nmerely was pointing out that core.quotepath and diff.color are _not_\nvalid counterexamples to it.\n\nA better counter-proposal would be made something along this line:\n\n\tCurrently, core.frotz and nitfol.rezrov both map internally to a\n\tvariable xyzzy, so config parser could set xyzzy = INVALID_INFOCOM\n but the user of xyzzy cannot report which configuration variable\n\tcaused the error with your illustration scheme. Here is my\n\tattempt to solve this issue.\n\n\t...patch follows...\n\nAnd I am sure we will be able to find such examples that my illustration\npatch is not _exactly_ the best approach to solve pretty easily around\nremote.*.* and branch.*.* variables.\n"},{"id":"122921","messageId":"20090911220506.GA27098@coredump.intra.peff.net","threadId":"20894","inReplyTo":"7vr5uduotz.fsf@alter.siamese.dyndns.org","subject":"Re: bash_completion outside repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-11T22:05:07Z","receivedAt":"2009-09-11T22:05:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 11, 2009 at 02:57:28PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I think something like that needs to \"belong\" to the diff code itself. I\n> > guess in the case of \"diff\", we could check all diff-related config at\n> > diff setup time.\n> \n> Not necessarily. You do not want to care about color configuration if you are\n> doing diff --raw for example. The one that first uses the color variable\n> should be able to notice the breakage, no?\n> \n> > But what about something used in several places, like\n> > core.quotepath?\n> \n> Exactly the same way I checked what codepaths needed to fix for the\n> autosetupmerge stuff. core.quotepath internally sets quote_path_fully,\n> and the sole user of quote_path_fully is sq_must_quote() which is only\n> used by next_quote_pos(). So you can have your check very isolated.\n\nI guess I'm just worried that in doing this for _every_ variable we are\ngoing to run across cases where variables are used in several different\ncodepaths, and we are going to end up adding a large number of tests for\n\"is this thing valid\". And if we forget one, it's going to cause us to\naccess some sentinel value that may cause a segfault.\n\nBut that is just my gut feeling. I haven't actually looked at doing a\nfull-scale conversion.\n\n-Peff\n"},{"id":"122924","messageId":"20090911232344.GH2582@inocybe.localdomain","threadId":"20894","inReplyTo":"20090911150934.GB977@coredump.intra.peff.net","subject":"[PATCH] completion: Replace config --list with --get-regexp","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2009-09-11T23:23:45Z","receivedAt":"2009-09-11T23:23:45Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"James Bardin noted that the completion spewed warnings when no git\nconfig file is present. This is likely a bug to be fixed in git config,\nbut it's also a good excuse to simplify the completion code by using the\n--get-regexp option as Jeff King pointed out.\n\nSigned-off-by: Todd Zullinger \n---\n\nJeff King wrote:\n> On Fri, Sep 11, 2009 at 07:36:51AM -0700, Shawn O. Pearce wrote:\n>\n>>> instead of just using \"git config --get-regexp 'remote\\..*\\.url'\", which\n>>> would be slightly more efficient, and also doesn't have this bug. ;)\n>>\n>> F'king oversight. You are right, this should be --get-regexp.\n>> There isn't a reason here, probably other than \"I forgot about\n>> --get-regexp when I wrote the original code\".\n>\n> OK. I will leave a bash-completion patch for you (or somebody else\n> interested) as I don't use it myself.\n\nSomething like this perhaps? Perhaps the commit message could use\nadjustment to reference your fix for 'config --list'?\n\n contrib/completion/git-completion.bash | 30 +++++++++---------------------\n 1 files changed, 9 insertions(+), 21 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex bf688e1..d668fc9 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -318,13 +318,9 @@ __git_remotes ()\n \t\techo ${i#$d/remotes/}\n \tdone\n \t[ \"$ngoff\" ] && shopt -u nullglob\n-\tfor i in $(git --git-dir=\"$d\" config --list); do\n-\t\tcase \"$i\" in\n-\t\tremote.*.url=*)\n-\t\t\ti=\"${i#remote.}\"\n-\t\t\techo \"${i/.url=*/}\"\n-\t\t\t;;\n-\t\tesac\n+\tfor i in $(git --git-dir=\"$d\" config --get-regexp 'remote\\..*\\.url' 2>/dev/null); do\n+\t\ti=\"${i#remote.}\"\n+\t\techo \"${i/.url*/}\"\n \tdone\n }\n \n@@ -605,13 +601,9 @@ __git_porcelain_commandlist=\"$(__git_porcelain_commands 2>/dev/null)\"\n __git_aliases ()\n {\n \tlocal i IFS=$'\\n'\n-\tfor i in $(git --git-dir=\"$(__gitdir)\" config --list); do\n-\t\tcase \"$i\" in\n-\t\talias.*)\n-\t\t\ti=\"${i#alias.}\"\n-\t\t\techo \"${i/=*/}\"\n-\t\t\t;;\n-\t\tesac\n+\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"alias\\..*\" 2>/dev/null); do\n+\t\ti=\"${i#alias.}\"\n+\t\techo \"${i/ */}\"\n \tdone\n }\n \n@@ -1769,13 +1761,9 @@ _git_remote ()\n \t\t;;\n \tupdate)\n \t\tlocal i c='' IFS=$'\\n'\n-\t\tfor i in $(git --git-dir=\"$(__gitdir)\" config --list); do\n-\t\t\tcase \"$i\" in\n-\t\t\tremotes.*)\n-\t\t\t\ti=\"${i#remotes.}\"\n-\t\t\t\tc=\"$c ${i/=*/}\"\n-\t\t\t\t;;\n-\t\t\tesac\n+\t\tfor i in $(git --git-dir=\"$(__gitdir)\" config --get-regexp \"remotes\\..*\" 2>/dev/null); do\n+\t\t\ti=\"${i#remotes.}\"\n+\t\t\tc=\"$c ${i/ */}\"\n \t\tdone\n \t\t__gitcomp \"$c\"\n \t\t;;\n-- \n1.6.4.2\n\n-- \nTodd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nThat men do not learn very much from the lessons of history is the\nmost important of all the lessons of history.\n -- Aldous Huxley Collected Essays, 1959\n"},{"id":"122972","messageId":"20090912183139.GO1033@spearce.org","threadId":"20894","inReplyTo":"20090911232344.GH2582@inocybe.localdomain","subject":"Re: [PATCH] completion: Replace config --list with --get-regexp","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-09-12T18:31:39Z","receivedAt":"2009-09-12T18:31:39Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Todd Zullinger wrote:\n> James Bardin noted that the completion spewed warnings when no git\n> config file is present. This is likely a bug to be fixed in git config,\n> but it's also a good excuse to simplify the completion code by using the\n> --get-regexp option as Jeff King pointed out.\n> \n> Signed-off-by: Todd Zullinger \n\nThanks, looks good.\n\nTrivially-acked-by: Shawn O. Pearce \n\n> contrib/completion/git-completion.bash | 30 +++++++++---------------------\n> 1 files changed, 9 insertions(+), 21 deletions(-)\n\n-- \nShawn.\n"},{"id":"123049","messageId":"36ca99e90909130351m7ad5a641t8adb3ade3d2ec5ca@mail.gmail.com","threadId":"20894","inReplyTo":"20090912183139.GO1033@spearce.org","subject":"Re: [PATCH] completion: Replace config --list with --get-regexp","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-09-13T10:51:05Z","receivedAt":"2009-09-13T10:51:05Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Sat, Sep 12, 2009 at 20:31, Shawn O. Pearce wrote:\n> Todd Zullinger wrote:\n>> James Bardin noted that the completion spewed warnings when no git\n>> config file is present.  This is likely a bug to be fixed in git config,\n>> but it's also a good excuse to simplify the completion code by using the\n>> --get-regexp option as Jeff King pointed out.\n>>\n>> Signed-off-by: Todd Zullinger \n>\n> Thanks, looks good.\nI have not looked into this, but what about pushurl?\n\nBert\n"},{"id":"123075","messageId":"20090913182937.GR2582@inocybe.localdomain","threadId":"20894","inReplyTo":"36ca99e90909130351m7ad5a641t8adb3ade3d2ec5ca@mail.gmail.com","subject":"Re: [PATCH] completion: Replace config --list with --get-regexp","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2009-09-13T18:29:37Z","receivedAt":"2009-09-13T18:29:37Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Bert Wesarg wrote:\n> I have not looked into this, but what about pushurl?\n\nIs it reasonable to expect someone to have remote..pushurl and\nnot have remote..url set? If not, then we should be fine, as\nall the old code and this new code do is extract .\n\nOTOH, if there are some cases where setting pushurl and not url make\nsense, extending the regex to catch pushurl as well is a simple matter\nof changing 'remote\\..*\\.url' to 'remote\\..*\\.(push)?url' and, I\nbelieve, using 'echo \"${i/.*url*/}\"' to strip off everything after the\nremote .\n\n-- \nTodd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nFleas can be taught nearly anything that a Congressman can.\n -- Mark Twain\n\n"},{"id":"123104","messageId":"7vws42poi9.fsf@alter.siamese.dyndns.org","threadId":"20894","inReplyTo":"20090912183139.GO1033@spearce.org","subject":"Re: [PATCH] completion: Replace config --list with --get-regexp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-13T20:40:14Z","receivedAt":"2009-09-13T20:40:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, everybody. Will apply.\n"}]}