{"thread":{"id":"26135","subject":"[PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-tree","startedAt":"2010-12-25T01:20:47Z","lastAt":"2011-02-03T20:28:34Z","messageCount":20,"participants":["Zapped","Jens Lehmann","Johannes Schindelin","Junio C Hamano","Алексей Крезов","Алексей Шумкин","Casey Dahlin","Thomas Rast","Eric Wong","Alexey Shumkin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"158565","messageId":"1293240049-7744-1-git-send-email-zapped@mail.ru","threadId":"26135","inReplyTo":null,"subject":"[PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal","fromName":"Zapped","fromEmail":"zapped@mail.ru","sentAt":"2010-12-25T01:20:47Z","receivedAt":"2010-12-25T01:20:47Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"Signed-off-by: Zapped \n---\n userdiff.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/userdiff.c b/userdiff.c\nindex f9e05b5..259a382 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -52,7 +52,7 @@ PATTERNS(\"objc\",\n \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\"\n \t \"|[^[:space:]]|[\\x80-\\xff]+\"),\n PATTERNS(\"pascal\",\n-\t \"^((procedure|function|constructor|destructor|interface|\"\n+\t \"^(((class[ \\t]+)?(procedure|function)|constructor|destructor|interface|\"\n \t\t\"implementation|initialization|finalization)[ \\t]*.*)$\"\n \t \"\\n\"\n \t \"^(.*=[ \\t]*(class|record).*)$\",\n-- \n1.7.3.4.3.g3f811\n"},{"id":"158564","messageId":"1293240049-7744-2-git-send-email-zapped@mail.ru","threadId":"26135","inReplyTo":"1293240049-7744-1-git-send-email-zapped@mail.ru","subject":"[PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-tree","fromName":"Zapped","fromEmail":"zapped@mail.ru","sentAt":"2010-12-25T01:20:48Z","receivedAt":"2010-12-25T01:20:48Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"git-svn dcommit/set-tree fails when svn.pathnameencoding is set for native OS encoding (e.g. cp1251 for Windows) though git-svn fetch/clone works well\n\nSigned-off-by: Zapped \n---\n git-svn.perl | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 757de82..399bf4c 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4451,6 +4451,7 @@ sub new {\n \t$self->{path_prefix} = length $self->{svn_path} ?\n \t \"$self->{svn_path}/\" : '';\n \t$self->{config} = $opts->{config};\n+\t$self->{pathnameencoding} = Git::config('svn.pathnameencoding');\n \treturn $self;\n }\n \n-- \n1.7.3.4.3.g3f811\n"},{"id":"158566","messageId":"1293240049-7744-3-git-send-email-zapped@mail.ru","threadId":"26135","inReplyTo":"1293240049-7744-1-git-send-email-zapped@mail.ru","subject":"[PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Zapped","fromEmail":"zapped@mail.ru","sentAt":"2010-12-25T01:20:49Z","receivedAt":"2010-12-25T01:20:49Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"Signed-off-by: Zapped \n---\n contrib/completion/git-completion.bash | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex d3037fc..50fc385 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -280,7 +280,8 @@ __git_ps1 ()\n \t\telif [ \"true\" = \"$(git rev-parse --is-inside-work-tree 2>/dev/null)\" ]; then\n \t\t\tif [ -n \"${GIT_PS1_SHOWDIRTYSTATE-}\" ]; then\n \t\t\t\tif [ \"$(git config --bool bash.showDirtyState)\" != \"false\" ]; then\n-\t\t\t\t\tgit diff --no-ext-diff --quiet --exit-code || w=\"*\"\n+\t\t\t\t\tis=$(git config diff.ignoreSubmodules)\n+\t\t\t\t\tgit diff --no-ext-diff --quiet --exit-code --ignore-submodules=$is || w=\"*\"\n \t\t\t\t\tif git rev-parse --quiet --verify HEAD >/dev/null; then\n \t\t\t\t\t\tgit diff-index --cached --quiet HEAD -- || i=\"+\"\n \t\t\t\t\telse\n-- \n1.7.3.4.3.g3f811\n"},{"id":"158571","messageId":"4D15E48A.9050805@web.de","threadId":"26135","inReplyTo":"1293240049-7744-3-git-send-email-zapped@mail.ru","subject":"Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-25T12:33:14Z","receivedAt":"2010-12-25T12:33:14Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 25.12.2010 02:20, schrieb Zapped:\n> Signed-off-by: Zapped \n> ---\n> contrib/completion/git-completion.bash | 3 ++-\n> 1 files changed, 2 insertions(+), 1 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index d3037fc..50fc385 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -280,7 +280,8 @@ __git_ps1 ()\n> \t\telif [ \"true\" = \"$(git rev-parse --is-inside-work-tree 2>/dev/null)\" ]; then\n> \t\t\tif [ -n \"${GIT_PS1_SHOWDIRTYSTATE-}\" ]; then\n> \t\t\t\tif [ \"$(git config --bool bash.showDirtyState)\" != \"false\" ]; then\n> -\t\t\t\t\tgit diff --no-ext-diff --quiet --exit-code || w=\"*\"\n> +\t\t\t\t\tis=$(git config diff.ignoreSubmodules)\n> +\t\t\t\t\tgit diff --no-ext-diff --quiet --exit-code --ignore-submodules=$is || w=\"*\"\n> \t\t\t\t\tif git rev-parse --quiet --verify HEAD >/dev/null; then\n> \t\t\t\t\t\tgit diff-index --cached --quiet HEAD -- || i=\"+\"\n> \t\t\t\t\telse\n\nThanks for resubmitting this as an inline patch for review (although\nit would have been easier for me if the commit message would have\ndescribed the problem you tried to fix a bit more in detail ;-).\n\nAfter testing this patch it looks like it has a few issues:\n\n1) it will break any per-submodule configuration done via\n the 'submodule..ignore' setting in .git/config or\n .gitmodules, as using the --ignore-submodules option\n overrides those while only setting 'diff.ignoreSubmodules'\n should not do that.\n\n2) If diff.ignoreSubmodules is unset it leads to an error\n every time the prompt is displayed:\n 'fatal: bad --ignore-submodules argument:'\n\n3) And for me it didn't change the behavior at all:\n\n - The '*' in the prompt vanishes as I set diff.ignoreSubmodules\n as expected with or without your patch.\n Am I missing something here?\n\n - The real problem here is that the '+' never goes away even\n when 'diff.ignoreSubmodules' is set to 'all'. This is due\n to the fact that 'diff.ignoreSubmodules' is only honored by\n \"git diff\", but not by \"git diff-index\".\n\nSo the real issue here seems to be the \"git diff-index\" call, which\ndoesn't honor the 'diff.ignoreSubmodules' setting. In commit 37aea37\nDscho (CCed) introduced this configuration setting while explicitly\nstating that it only affects porcelain. As the other config options\nalways influence porcelain and plumbing, it looks like we would want\nto have this option honored by plumbing too, no?\n\nSo are there any reasons for the plumbing diff commands not to honor\nthe diff.ignoreSubmodules setting?\n"},{"id":"158575","messageId":"alpine.DEB.1.00.1012251407230.1822@bonsai2","threadId":"26135","inReplyTo":"4D15E48A.9050805@web.de","subject":"Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-12-25T13:08:03Z","receivedAt":"2010-12-25T13:08:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 25 Dec 2010, Jens Lehmann wrote:\n\n> In commit 37aea37 Dscho (CCed) introduced this configuration setting \n> while explicitly stating that it only affects porcelain. As the other \n> config options always influence porcelain and plumbing, it looks like we \n> would want to have this option honored by plumbing too, no?\n\nIIRC Junio was real happy that I did not honor plumbing.\n\nCiao,\nDscho\n"},{"id":"158595","messageId":"7vd3ooz6qd.fsf@alter.siamese.dyndns.org","threadId":"26135","inReplyTo":"4D15E48A.9050805@web.de","subject":"Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-26T19:14:50Z","receivedAt":"2010-12-26T19:14:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> So are there any reasons for the plumbing diff commands not to honor\n> the diff.ignoreSubmodules setting?\n\nThere are two kinds of users of the plumbing.\n\nOne class of plumbing users is scripts that is about automation and\nmechanization that want to control what they do precisely (think cron\njobs) without getting affected by the user preference stored in the\nrepository configuration. This class could either:\n\n (1) state what they want explicitly from the command line; or\n (2) rely on built-in defaults not changing underneath them.\n\nThe behaviour of diff recursively inspecting submodule dirtiness has an\nunfortunate history, in that the behaviour changed over time, and in each\nstep when we made a change, we thought we were making an unquestionable\nimprovement. Originally we only said \"submodule HEAD is different from\nwhat we have in the index/superproject HEAD\". Later we added different\nkind of dirtiness like untracked files or modified contents in submodules,\ndecided perhaps mistakenly that majority of users do want to see them as\ndirtiness and made that the default and allowed them to be ignored by an\nexplicit request. At that point, in order not to break existing scripts\n(mostly of the \"mechanization\" class, written back when there was no such\nextra dirtyness hence with no \"explicit refusal\" route available to them\nwithout rewriting), hence \"no configuration should affect plumbing\nrandomly\" policy.\n\nOn the other hand, you may write user facing Porcelain in scripts and run\nplumbing from there. This class of plumbing users could either:\n\n (1) inspect the config itself, interpret the customization and pass\n an explicit command line flag; or\n\n (2) allow the plumbing honor the end user configuration stored in the\n repository or user configuration files.\n\nIt is argurably more convenient for these users if the plumbing blindly\nhonored the configurations, as it would have allowed the latter\nimplementation. That way, we can be more lazy when writing our scripts,\nand ignore having to worry about new kinds of customization added to\nunderlying git after a script is written---but new kinds of customization\nmay break your script's expectation of what will and what will not be made\ncustomizable, and you would end up giving an explicit \"do not use that\nfeature\" in some cases, so the being able to be lazy is not necessarily\nalways a win.\n\nThings may have been a bit different if the original feature change to\ninspect submodules deeper, command line flags to control that behaviour\nand configuration to default the flags came at the same time, but\nunfortunately they happend over time. I think we have been slowly getting\nbetter at this, but in the case of this particular feature, the original\nintroduction of --ignore-submodules was in May 2008, deeper submodule\ninspection and the richer --ignore-submodules= option came much\nlater in June 2010, and the configuration was invented later in August\n2010, which would mean that allowing the plumbing to honor configuration\nwould have broken scripts written in the 2 years and 3 months period.\n\nAnd no, this does not call for a blanket \"do / do not honor configuration\"\noption to plumbing commands. A more selective \"do / do not honor these\nconfiguration variables\" option might be an option, though.\n\nBy the way, could we please have a real sign-off, not with a one with a\npseudonym, given to the series?\n"},{"id":"158599","messageId":"1824011293.20101227012557@mail.ru","threadId":"26135","inReplyTo":"4D15E48A.9050805@web.de","subject":"Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Алексей Крезов","fromEmail":"zapped@mail.ru","sentAt":"2010-12-26T22:25:57Z","receivedAt":"2010-12-26T22:25:57Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"Hello!\n\nI`m sorry, but I`m newbie in making and distributing of public patches.\nSo, don't beat me much ))\n\nJL> it would have been easier for me if the commit message would have\nJL> described the problem you tried to fix a bit more in detail ;-).\nMy problem was in the following.\nI use Git on Windows XP under Cygwin. So its perfomance is slower than\non *nix, I guess.\nMy project has 40 submodules (external libs) and some of them could\nhave untracked files (for some reasons). So when I run any command in Bash\nafter its execution Bash \"thought\" for 2-3 seconds. That was annoying.\nI do not remember the version of Git I used at that moment but I\nremember it was an update from 1.6.x to early 1.7.x. So I decided to roll back\nto 1.6.x ))\nThen there was some updates of Git. But after updating the problem\nstill happened.\nWhen I tried to discover the reason of such a behaviour I found that\nGit got status for all submodules including untracked content and so\nmarked them with *\nThen I read manual and found diff.ignoreSubmodules and tried to set up for each\nsubmodule in a .gitmodules but nothing changed (as it seemed to me at\nthat time).\nSo I've found the easiest way to solve my problem - this patch )\nMaybe after this patch there was some changes in Git solved this\nproblem but I did not investigate it, sorry.\n\nJL> 2) If diff.ignoreSubmodules is unset it leads to an error\nJL> every time the prompt is displayed:\nJL> 'fatal: bad --ignore-submodules argument:'\nYeah, you're right\n\nJL> Am 25.12.2010 02:20, schrieb Zapped:\n>> Signed-off-by: Zapped \n>> ---\n>> contrib/completion/git-completion.bash | 3 ++-\n>> 1 files changed, 2 insertions(+), 1 deletions(-)\n>> \n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index d3037fc..50fc385 100755\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -280,7 +280,8 @@ __git_ps1 ()\n>> elif [ \"true\" = \"$(git rev-parse --is-inside-work-tree 2>/dev/null)\" ]; then\n>> if [ -n \"${GIT_PS1_SHOWDIRTYSTATE-}\" ]; then\n>> if [ \"$(git config --bool bash.showDirtyState)\" != \"false\" ]; then\n>> - git diff --no-ext-diff --quiet --exit-code || w=\"*\"\n>> + is=$(git config diff.ignoreSubmodules)\n>> + git diff --no-ext-diff --quiet --exit-code --ignore-submodules=$is || w=\"*\"\n>> if git rev-parse --quiet --verify HEAD >/dev/null; then\n>> git diff-index --cached --quiet HEAD -- || i=\"+\"\n>> else\n\nJL> Thanks for resubmitting this as an inline patch for review (although\nJL> it would have been easier for me if the commit message would have\nJL> described the problem you tried to fix a bit more in detail ;-).\n\nJL> After testing this patch it looks like it has a few issues:\n\nJL> 1) it will break any per-submodule configuration done via\nJL> the 'submodule..ignore' setting in .git/config or\nJL> .gitmodules, as using the --ignore-submodules option\nJL> overrides those while only setting 'diff.ignoreSubmodules'\nJL> should not do that.\n\nJL> 2) If diff.ignoreSubmodules is unset it leads to an error\nJL> every time the prompt is displayed:\nJL> 'fatal: bad --ignore-submodules argument:'\n\nJL> 3) And for me it didn't change the behavior at all:\n\nJL> - The '*' in the prompt vanishes as I set diff.ignoreSubmodules\nJL> as expected with or without your patch.\nJL> Am I missing something here?\n\nJL> - The real problem here is that the '+' never goes away even\nJL> when 'diff.ignoreSubmodules' is set to 'all'. This is due\nJL> to the fact that 'diff.ignoreSubmodules' is only honored by\nJL> \"git diff\", but not by \"git diff-index\".\n\nJL> So the real issue here seems to be the \"git diff-index\" call, which\nJL> doesn't honor the 'diff.ignoreSubmodules' setting. In commit 37aea37\nJL> Dscho (CCed) introduced this configuration setting while explicitly\nJL> stating that it only affects porcelain. As the other config options\nJL> always influence porcelain and plumbing, it looks like we would want\nJL> to have this option honored by plumbing too, no?\n\nJL> So are there any reasons for the plumbing diff commands not to honor\nJL> the diff.ignoreSubmodules setting?\n"},{"id":"158601","messageId":"255471532.20101227014203@mail.ru","threadId":"26135","inReplyTo":"7vd3ooz6qd.fsf@alter.siamese.dyndns.org","subject":"Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Алексей Шумкин","fromEmail":"zapped@mail.ru","sentAt":"2010-12-26T22:42:03Z","receivedAt":"2010-12-26T22:42:03Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"JCH> By the way, could we please have a real sign-off, not with a one with a\nJCH> pseudonym, given to the series?\nOh, I`m sorry.\nOf course.\nShould I resend patches with real sign-off, at least first two ones?\n"},{"id":"158618","messageId":"4D187511.3090104@web.de","threadId":"26135","inReplyTo":"7vd3ooz6qd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-12-27T11:14:25Z","receivedAt":"2010-12-27T11:14:25Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 26.12.2010 20:14, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>> So are there any reasons for the plumbing diff commands not to honor\n>> the diff.ignoreSubmodules setting?\n\nThank you for explaining those reasons in detail.\n\n> One class of plumbing users is scripts that is about automation and\n> mechanization that want to control what they do precisely (think cron\n> jobs) without getting affected by the user preference stored in the\n> repository configuration. This class could either:\n> \n> (1) state what they want explicitly from the command line; or\n> (2) rely on built-in defaults not changing underneath them.\n> \n> The behaviour of diff recursively inspecting submodule dirtiness has an\n> unfortunate history, in that the behaviour changed over time, and in each\n> step when we made a change, we thought we were making an unquestionable\n> improvement. Originally we only said \"submodule HEAD is different from\n> what we have in the index/superproject HEAD\". Later we added different\n> kind of dirtiness like untracked files or modified contents in submodules,\n> decided perhaps mistakenly that majority of users do want to see them as\n> dirtiness and made that the default and allowed them to be ignored by an\n> explicit request. At that point, in order not to break existing scripts\n> (mostly of the \"mechanization\" class, written back when there was no such\n> extra dirtyness hence with no \"explicit refusal\" route available to them\n> without rewriting), hence \"no configuration should affect plumbing\n> randomly\" policy.\n\nGood point. But unfortunately the diff plumbing commands are affected by\nthe \"submodule..ignore\" ignore settings I introduced in August in\naee9c7d6 and 302ad7a9. Maybe we should revert the part of these patches\nthat changed the plumbing commands?\n\n> On the other hand, you may write user facing Porcelain in scripts and run\n> plumbing from there. This class of plumbing users could either:\n> \n> (1) inspect the config itself, interpret the customization and pass\n> an explicit command line flag; or\n> \n> (2) allow the plumbing honor the end user configuration stored in the\n> repository or user configuration files.\n> \n> It is argurably more convenient for these users if the plumbing blindly\n> honored the configurations, as it would have allowed the latter\n> implementation. That way, we can be more lazy when writing our scripts,\n> and ignore having to worry about new kinds of customization added to\n> underlying git after a script is written---but new kinds of customization\n> may break your script's expectation of what will and what will not be made\n> customizable, and you would end up giving an explicit \"do not use that\n> feature\" in some cases, so the being able to be lazy is not necessarily\n> always a win.\n\nAgreed.\n\n> Things may have been a bit different if the original feature change to\n> inspect submodules deeper, command line flags to control that behaviour\n> and configuration to default the flags came at the same time, but\n> unfortunately they happend over time. I think we have been slowly getting\n> better at this, but in the case of this particular feature, the original\n> introduction of --ignore-submodules was in May 2008, deeper submodule\n> inspection and the richer --ignore-submodules= option came much\n> later in June 2010, and the configuration was invented later in August\n> 2010, which would mean that allowing the plumbing to honor configuration\n> would have broken scripts written in the 2 years and 3 months period.\n> \n> And no, this does not call for a blanket \"do / do not honor configuration\"\n> option to plumbing commands. A more selective \"do / do not honor these\n> configuration variables\" option might be an option, though.\n\nWhat about a new \"--ignore-submodules=config\" option to tell the plumbing\nthat it should honor the config?\n\n\nAnd it looks like the PS1 problem that started this discussion is a\nvalid example for mixed usage of porcelain and plumbing commands. In a\nfirst attempt to fix the problem by using \"git diff --cached\" instead\nof \"git diff-index --cached\" I noticed that those two commands give\ndifferent results when new submodules were created and had been added\nto the index. \"git diff --cached\" ignores them while \"git diff-index\n--cached\" shows them. Anything I am missing here?\n"},{"id":"158625","messageId":"alpine.DEB.1.00.1012272256560.1794@bonsai2","threadId":"26135","inReplyTo":"4D187511.3090104@web.de","subject":"Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-12-27T22:06:38Z","receivedAt":"2010-12-27T22:06:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jens,\n\nOn Mon, 27 Dec 2010, Jens Lehmann wrote:\n\n> [...]\n>\n> And it looks like the PS1 problem that started this discussion is a\n> valid example for mixed usage of porcelain and plumbing commands.\n\nThe distinction between porcelain and plumbing commands is not as \nclear-cut as some people would like it to be (just call \"git grep 'git \nlog'\" in a git.git checkout). IMHO the reason is that a distinction \nbetween porcelain and plumbing makes sense in the world of sanitary \nengineering, but not necessarily in the world of software (a distinction \nbetween assembler vs source code, or GUI vs library makes sense, but not \nbetween \"programs to be called by humans\" and \"programs to be called by \nother programs\").\n\nNote: I do not think that the \"plumbing\" concept was not well-intended, \nbut I doubt that the concept holds up in the face of reality.\n\nI fear, though, that we cannot simply abolish the notion \"plumbing vs \nporcelain\" from git.git...\n\nCiao,\nDscho\n"},{"id":"158627","messageId":"20101227224357.GA9947@foucault.redhat.com","threadId":"26135","inReplyTo":"alpine.DEB.1.00.1012272256560.1794@bonsai2","subject":"Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Casey Dahlin","fromEmail":"cdahlin@redhat.com","sentAt":"2010-12-27T22:43:57Z","receivedAt":"2010-12-27T22:43:57Z","isPatch":true,"sender":{"key":"cdahlin@redhat.com","avatar":null},"body":"On Mon, Dec 27, 2010 at 11:06:38PM +0100, Johannes Schindelin wrote:\n> Note: I do not think that the \"plumbing\" concept was not well-intended, \n> but I doubt that the concept holds up in the face of reality.\n> \n\nI've always felt that plumbing commands existed to expose the C portion of git\nto the bash portion of git. As the latter shrinks plumbing commands make less\nsense (and offering those commands as a library makes more sense, which is\nunfortunately close to impossible in the current git source).\n\n--CJD\n"},{"id":"158638","messageId":"1393005584.20101228151409@mail.ru","threadId":"26135","inReplyTo":"4D15E48A.9050805@web.de","subject":"Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable","fromName":"Алексей Шумкин","fromEmail":"zapped@mail.ru","sentAt":"2010-12-28T12:14:09Z","receivedAt":"2010-12-28T12:14:09Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"Hi, all.\n\nWell, I rolled back my so-called patch which rose up such a\ndiscussion, and what I did see? Annoying behaviuor is vanished and perfomance of $PS1 is\nacceptable (as fast as with the \"patch\").\nI guess, as I said, something changed in Git since I discovered\nand solved the problem such a weird way.\n\nSo, it seems to me, this patch might be considered unnecessary?\n"},{"id":"158898","messageId":"201101041813.45053.trast@student.ethz.ch","threadId":"26135","inReplyTo":"1293240049-7744-1-git-send-email-zapped@mail.ru","subject":"Re: [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-01-04T17:13:44Z","receivedAt":"2011-01-04T17:13:44Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Zapped wrote:\n> Signed-off-by: Zapped \n\nAs Junio already said, please provide a real name for the sign-off.\nBut I also found the commit message and content confusing, probably\nbecause I haven't programmed Pascal for 15 years.\n\nYou said\n\n Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal\n\n> PATTERNS(\"pascal\",\n> -\t \"^((procedure|function|constructor|destructor|interface|\"\n> +\t \"^(((class[ \\t]+)?(procedure|function)|constructor|destructor|interface|\"\n> \t\t\"implementation|initialization|finalization)[ \\t]*.*)$\"\n> \t \"\\n\"\n> \t \"^(.*=[ \\t]*(class|record).*)$\",\n\nBut the last line very conspicuously already mentions 'class', so why\ndoes it fail?\n\nI had to look up a bit of Pascal syntax. Google helped with\n\n http://www.freepascal.org/docs-html/ref/ref.html\n\nwhich answers this. Also, as stated in SubmittingPatches, we\ngenerally word the messages as stating the behaviour of the changed\nversion in the present tense. So a better commit message would be\n\n userdiff: match Pascal class methods\n\n Class declarations were already covered by the second pattern, but\n class methods have the 'class' keyword in front too. Account for\n it.\n\n Signed-off-by: Алексей Крезов \n\nOk, now I feel silly for only having a two-liner despite my\ncomplaints.\n\nThat being said, I have now verified that the patch is good, and, you\ncan include my\n\n Acked-by: Thomas Rast \n\nin a reroll if you fix the above.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"158899","messageId":"201101041818.09365.trast@student.ethz.ch","threadId":"26135","inReplyTo":"1293240049-7744-2-git-send-email-zapped@mail.ru","subject":"Re: [PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-tree","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-01-04T17:18:09Z","receivedAt":"2011-01-04T17:18:09Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Zapped wrote:\n> git-svn dcommit/set-tree fails when svn.pathnameencoding is set for native OS encoding (e.g. cp1251 for Windows) though git-svn fetch/clone works well\n\nI'll let Eric judge whether loading the encoding here is the right\nfix, but here too the commit message states only what is broken, not\nwhy you fixed it that way. Can you elaborate a bit?\n\nAlso, this should be easy to cover with a test case, can you make one?\n\n> git-svn.perl | 1 +\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index 757de82..399bf4c 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -4451,6 +4451,7 @@ sub new {\n> \t$self->{path_prefix} = length $self->{svn_path} ?\n> \t \"$self->{svn_path}/\" : '';\n> \t$self->{config} = $opts->{config};\n> +\t$self->{pathnameencoding} = Git::config('svn.pathnameencoding');\n> \treturn $self;\n> }\n> \n> \n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"158920","messageId":"20110104232029.GA15889@dcvr.yhbt.net","threadId":"26135","inReplyTo":"201101041818.09365.trast@student.ethz.ch","subject":"Re: [PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-tree","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2011-01-04T23:20:29Z","receivedAt":"2011-01-04T23:20:29Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Thomas Rast wrote:\n> Zapped wrote:\n> > git-svn dcommit/set-tree fails when svn.pathnameencoding is set for native OS encoding (e.g. cp1251 for Windows) though git-svn fetch/clone works well\n> \n> I'll let Eric judge whether loading the encoding here is the right\n> fix, but here too the commit message states only what is broken, not\n> why you fixed it that way. Can you elaborate a bit?\n> \n> Also, this should be easy to cover with a test case, can you make one?\n\nI second Thomas's requests. I'm also a bit disappointed the original\noption is missing tests, especially since not many people are likely to\nuse it.\n\n-- \nEric Wong\n"},{"id":"158955","messageId":"707440142.20110105144417@mail.ru","threadId":"26135","inReplyTo":"201101041818.09365.trast@student.ethz.ch","subject":"Re[2]: [PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-tree","fromName":"Алексей Шумкин","fromEmail":"zapped@mail.ru","sentAt":"2011-01-05T11:44:17Z","receivedAt":"2011-01-05T11:44:17Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"As I already said I'm newbie in submitting/distributing patches\nbut I'll try :)\n\nTuesday, January 4, 2011, 8:18:09 PM, you wrote:\n\nTR> Zapped wrote:\n>> git-svn dcommit/set-tree fails when svn.pathnameencoding is set for native OS encoding (e.g. cp1251 for Windows) though git-svn fetch/clone works well\n\nTR> I'll let Eric judge whether loading the encoding here is the right\nTR> fix, but here too the commit message states only what is broken, not\nTR> why you fixed it that way. Can you elaborate a bit?\n\nTR> Also, this should be easy to cover with a test case, can you make one?\n\n>> git-svn.perl | 1 +\n>> 1 files changed, 1 insertions(+), 0 deletions(-)\n>> \n>> diff --git a/git-svn.perl b/git-svn.perl\n>> index 757de82..399bf4c 100755\n>> --- a/git-svn.perl\n>> +++ b/git-svn.perl\n>> @@ -4451,6 +4451,7 @@ sub new {\n>> $self->{path_prefix} = length $self->{svn_path} ?\n>> \"$self->{svn_path}/\" : '';\n>> $self->{config} = $opts->{config};\n>> + $self->{pathnameencoding} = Git::config('svn.pathnameencoding');\n>> return $self;\n>> }\n>> \n>> \n\n\n\n-- \nС уважением,\n Алексей Шумкин mailto:zapped@mail.ru\n"},{"id":"158957","messageId":"4510264083.20110105145302@mail.ru","threadId":"26135","inReplyTo":"201101041813.45053.trast@student.ethz.ch","subject":"Re[2]: [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal","fromName":"Алексей Шумкин","fromEmail":"zapped@mail.ru","sentAt":"2011-01-05T11:53:02Z","receivedAt":"2011-01-05T11:53:02Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"Hello, Thomas\n\nTR> But the last line very conspicuously already mentions 'class', so why\nTR> does it fail?\nYes, As you already discovered that last line match for class/record\ndefinition but not for class methods.\n\nI did as you said I changed commit message (also included\n\"Acked-by:\"). So should I re-submit patch to the maillist as a new one\nor as an answer to this thread?\n\nTR> Zapped wrote:\n>> Signed-off-by: Zapped \n\nTR> As Junio already said, please provide a real name for the sign-off.\nTR> But I also found the commit message and content confusing, probably\nTR> because I haven't programmed Pascal for 15 years.\n\nTR> You said\n\nTR> Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal\n\n>> PATTERNS(\"pascal\",\n>> - \"^((procedure|function|constructor|destructor|interface|\"\n>> + \"^(((class[ \\t]+)?(procedure|function)|constructor|destructor|interface|\"\n>> \"implementation|initialization|finalization)[ \\t]*.*)$\"\n>> \"\\n\"\n>> \"^(.*=[ \\t]*(class|record).*)$\",\n\nTR> But the last line very conspicuously already mentions 'class', so why\nTR> does it fail?\n\nTR> I had to look up a bit of Pascal syntax. Google helped with\n\nTR> http://www.freepascal.org/docs-html/ref/ref.html\n\nTR> which answers this. Also, as stated in SubmittingPatches, we\nTR> generally word the messages as stating the behaviour of the changed\nTR> version in the present tense. So a better commit message would be\n\nTR> userdiff: match Pascal class methods\n\nTR> Class declarations were already covered by the second pattern, but\nTR> class methods have the 'class' keyword in front too. Account for\nTR> it.\n\nTR> Signed-off-by: Алексей Крезов \n\nTR> Ok, now I feel silly for only having a two-liner despite my\nTR> complaints.\n\nTR> That being said, I have now verified that the patch is good, and, you\nTR> can include my\n\nTR> Acked-by: Thomas Rast \n\nTR> in a reroll if you fix the above.\n"},{"id":"158959","messageId":"201101051523.55749.trast@student.ethz.ch","threadId":"26135","inReplyTo":"4510264083.20110105145302@mail.ru","subject":"Re: [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-01-05T14:23:55Z","receivedAt":"2011-01-05T14:23:55Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Алексей Шумкин wrote:\n> I did as you said I changed commit message (also included\n> \"Acked-by:\"). So should I re-submit patch to the maillist as a new one\n> or as an answer to this thread?\n\nIt doesn't matter that much; most people resend as a reply to some\nemail in the thread. Just don't distribute the reroll all across the\nthread :-)\n\nWhat you can do now that you have an Ack, or if you feel otherwise\nconfident that this patch is ready for inclusion, is Cc it to Junio.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"158961","messageId":"201101051531.49142.trast@student.ethz.ch","threadId":"26135","inReplyTo":"201101051523.55749.trast@student.ethz.ch","subject":"Re: [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-01-05T14:31:48Z","receivedAt":"2011-01-05T14:31:48Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> Алексей Шумкин wrote:\n> > I did as you said I changed commit message (also included\n> > \"Acked-by:\"). So should I re-submit patch to the maillist as a new one\n> > or as an answer to this thread?\n> \n> It doesn't matter that much; most people resend as a reply to some\n> email in the thread. Just don't distribute the reroll all across the\n> thread :-)\n\nBTW speaking of series (and sorry for not pointing this out earlier):\n\nSince these patches are not related to each other at all, it's better\nif you treat them as separate, not as a series. Making a series is\nsort of an \"all or nothing\" hint, meaning that if something holds up\none patch in the series, Junio will treat the whole series as stalled,\netc.\n\nSo probably it's better if you resend the acked patch alone, to show\nthat it can stand on its own. Then later repeat the same for the\nother two patches when you have them ready.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"160349","messageId":"loom.20110203T212349-287@post.gmane.org","threadId":"26135","inReplyTo":"20110104232029.GA15889@dcvr.yhbt.net","subject":"Re: [PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-tree","fromName":"Alexey Shumkin","fromEmail":"zapped@mail.ru","sentAt":"2011-02-03T20:28:34Z","receivedAt":"2011-02-03T20:28:34Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"Eric Wong yhbt.net> writes:\n\n> \n> Thomas Rast student.ethz.ch> wrote:\n> > Zapped wrote:\n> > > git-svn dcommit/set-tree fails when svn.pathnameencoding is set for native \nOS encoding (e.g. cp1251\n> for Windows) though git-svn fetch/clone works well\n> > \n> > I'll let Eric judge whether loading the encoding here is the right\n> > fix, but here too the commit message states only what is broken, not\n> > why you fixed it that way. Can you elaborate a bit?\n> > \n> > Also, this should be easy to cover with a test case, can you make one?\n> \n> I second Thomas's requests. I'm also a bit disappointed the original\n> option is missing tests, especially since not many people are likely to\n> use it.\n> \n\nYes, I remember that, sorry, but I just have no enough time to get\ninto git tests and make my own ones.\n"}]}