{"thread":{"id":"10375","subject":"gitk patch collection pull request","startedAt":"2007-10-19T05:28:23Z","lastAt":"2007-10-24T00:17:46Z","messageCount":25,"participants":["Shawn O. Pearce","Johannes Sixt","Paul Mackerras","Michele Ballabio","Linus Torvalds","Jonathan del Strother","Jan Hudec","Rocco Rutte"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"56521","messageId":"20071019052823.GI14735@spearce.org","threadId":"10375","inReplyTo":null,"subject":"gitk patch collection pull request","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-19T05:28:23Z","receivedAt":"2007-10-19T05:28:23Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"The following changes since commit 719c2b9d926bf2be4879015e3620d27d32f007b6:\n Paul Mackerras (1):\n gitk: Fix bug causing undefined variable error when cherry-picking\n\nare available in the git repository at:\n\n git://repo.or.cz:/git/spearce.git gitk\n\nJonathan del Strother (2):\n gitk: Added support for OS X mouse wheel\n Fixing gitk indentation\n\nSam Vilain (1):\n gitk: disable colours when calling git log\n\n gitk | 16 +++++++++++-----\n 1 files changed, 11 insertions(+), 5 deletions(-)\n\n\nI'm carrying these in my pu branch but would like to move them up\ninto master. My gitk branch is actually forked off your own gitk\nrepository and doesn't contain the git.git history so you should\nbe able to do a direct pull.\n\n-- \nShawn.\n"},{"id":"56530","messageId":"47185BCC.9010307@viscovery.net","threadId":"10375","inReplyTo":"20071019052823.GI14735@spearce.org","subject":"[PATCH resend again] gitk: Do not pick up file names of \"copy from\" lines","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-19T07:25:00Z","receivedAt":"2007-10-19T07:25:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nA file copy would be detected only if the original file was modified in the\nsame commit. This implies that there will be a patch listed under the\noriginal file name, and we would expect that clicking the original file\nname in the file list warps the patch window to that file's patch. (If the\noriginal file was not modified, the copy would not be detected in the first\nplace, the copied file would be listed as \"new file\", and this whole matter\nwould not apply.)\n\nHowever, if the name of the copy is sorted after the original file's patch,\nthen the logic introduced by commit d1cb298b0b (which picks up the link\ninformation from the \"copy from\" line) would overwrite the link\ninformation that is already present for the original file name, which was\nparsed earlier. Hence, this patch reverts part of said commit.\n\nSigned-off-by: Johannes Sixt \n---\n Shawn O. Pearce schrieb:\n > I'm carrying these in my pu branch but would like to move them up\n > into master.\n\n Would you mind putting this one into your queue, too? I haven't seen it\n appear in Paul's repo.\n\n -- Hannes\n\n gitk | 3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex b3ca704..1306382 100755\n--- a/gitk\n+++ b/gitk\n@@ -5216,8 +5216,7 @@ proc getblobdiffline {bdf ids} {\n \t set diffinhdr 0\n\n \t} elseif {$diffinhdr} {\n-\t if {![string compare -length 12 \"rename from \" $line] ||\n-\t\t![string compare -length 10 \"copy from \" $line]} {\n+\t if {![string compare -length 12 \"rename from \" $line]} {\n \t\tset fname [string range $line [expr 6 + [string first \" from \" $line] ] end]\n \t\tif {[string index $fname 0] eq \"\\\"\"} {\n \t\t set fname [lindex $fname 0]\n-- \n1.5.3.722.gccbb1\n"},{"id":"56532","messageId":"20071019073253.GM14735@spearce.org","threadId":"10375","inReplyTo":"47185BCC.9010307@viscovery.net","subject":"Re: [PATCH resend again] gitk: Do not pick up file names of \"copy from\" lines","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-19T07:32:53Z","receivedAt":"2007-10-19T07:32:53Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt wrote:\n> Would you mind putting this one into your queue, too? I haven't seen it\n> appear in Paul's repo.\n\nI think it is already in Paul's repo:\n\n Applying gitk: Do not pick up file names of \"copy from\" lines\n error: patch failed: gitk:5216\n error: gitk: patch does not apply\n fatal: sha1 information is lacking or useless (gitk).\n Repository lacks necessary blobs to fall back on 3-way merge.\n Cannot fall back to three-way merge.\n Patch failed at 0001.\n When you have resolved this problem run \"git-am -3 --resolved\".\n If you would prefer to skip this patch, instead run \"git-am -3 --skip\".\n \n> diff --git a/gitk b/gitk\n> index b3ca704..1306382 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -5216,8 +5216,7 @@ proc getblobdiffline {bdf ids} {\n> \t set diffinhdr 0\n> \n> \t} elseif {$diffinhdr} {\n> -\t if {![string compare -length 12 \"rename from \" $line] ||\n> -\t\t![string compare -length 10 \"copy from \" $line]} {\n> +\t if {![string compare -length 12 \"rename from \" $line]} {\n> \t\tset fname [string range $line [expr 6 + [string first \" from \n> \t\t\" $line] ] end]\n> \t\tif {[string index $fname 0] eq \"\\\"\"} {\n> \t\t set fname [lindex $fname 0]\n\n-- \nShawn.\n"},{"id":"56537","messageId":"47186563.3070607@viscovery.net","threadId":"10375","inReplyTo":"20071019073253.GM14735@spearce.org","subject":"Re: [PATCH resend again] gitk: Do not pick up file names of \"copy from\" lines","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-19T08:05:55Z","receivedAt":"2007-10-19T08:05:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Shawn O. Pearce schrieb:\n> Johannes Sixt wrote:\n>> Would you mind putting this one into your queue, too? I haven't seen it\n>> appear in Paul's repo.\n> \n> I think it is already in Paul's repo:\n\nNo, it's not. I checked both Paul's master and dev, and also your own\ngitk branch. Would you mind cherry-picking from the tip of\n\ngit://repo.or.cz/git/mingw.git mob\n\nThanks,\n-- Hannes\n"},{"id":"56540","messageId":"20071019081428.GP14735@spearce.org","threadId":"10375","inReplyTo":"47186563.3070607@viscovery.net","subject":"Re: [PATCH resend again] gitk: Do not pick up file names of \"copy from\" lines","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-19T08:14:28Z","receivedAt":"2007-10-19T08:14:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt wrote:\n> Shawn O. Pearce schrieb:\n> >I think it is already in Paul's repo:\n> \n> No, it's not. I checked both Paul's master and dev, and also your own\n> gitk branch. Would you mind cherry-picking from the tip of\n> \n> git://repo.or.cz/git/mingw.git mob\n\nPicked. Its now in spearce/gitk.\n\n-- \nShawn.\n"},{"id":"56557","messageId":"18200.36704.936554.220173@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"20071019052823.GI14735@spearce.org","subject":"Re: gitk patch collection pull request","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-19T11:05:04Z","receivedAt":"2007-10-19T11:05:04Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Shawn O. Pearce writes:\n\n> The following changes since commit 719c2b9d926bf2be4879015e3620d27d32f007b6:\n> Paul Mackerras (1):\n> gitk: Fix bug causing undefined variable error when cherry-picking\n> \n> are available in the git repository at:\n> \n> git://repo.or.cz:/git/spearce.git gitk\n\nOK, but ...\n\n> Jonathan del Strother (2):\n> gitk: Added support for OS X mouse wheel\n> Fixing gitk indentation\n\nThis one is bogus. Firstly, it doesn't have \"gitk:\" at the start of\nthe headline (and \"Fixing\" should be \"Fix\"). Secondly, the actual\nchange itself is bogus. It changes an initial tab to 8 spaces on each\nof 4 lines. I like it the way it is - and if he wanted to change it,\nhe should have changed it throughout the file, not just on 4 lines.\nSo that change is rejected.\n\nThe other changes are OK. If you could re-do your tree without\n0d6df4de (and possible change \"Added\" to \"Add\" in e1b5683c while\nyou're at it), I'll do the pull.\n\nPaul.\n"},{"id":"56577","messageId":"200710191544.22228.barra_cuda@katamail.com","threadId":"10375","inReplyTo":"20071019052823.GI14735@spearce.org","subject":"[PATCH-resent] gitk: fix in procedure drawcommits","fromName":"Michele Ballabio","fromEmail":"barra_cuda@katamail.com","sentAt":"2007-10-19T13:44:22Z","receivedAt":"2007-10-19T13:44:22Z","isPatch":true,"sender":{"key":"barra_cuda@katamail.com","avatar":"https://avatars.githubusercontent.com/u/16371673?v=4"},"body":"This patch indroduces a check before unsetting an array element.\n\nWithout this, gitk may complain with\n\n\tcan't unset \"prevlines(...)\": no such element in array\n\nwhen scrolling happens.\n\nSigned-off-by: Michele Ballabio \n---\n\nThere's an error that seems to occur in gitk only on\nmutt's imported repo, but I don't know why. This is\nhopefully the right fix.\n\nAn example of this error:\n\ncan't unset \"prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)\": no such element in array\ncan't unset \"prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)\": no such element in array\n    while executing\n\"unset prevlines($lid)\"\n    (procedure \"drawcommits\" line 39)\n    invoked from within\n\"drawcommits $row $endrow\"\n    (procedure \"drawfrac\" line 10)\n    invoked from within\n\"drawfrac $f0 $f1\"\n    (procedure \"scrollcanv\" line 3)\n    invoked from within\n\"scrollcanv .tf.histframe.csb 0.00672513 0.0087015\"\n\n gitk | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 300fdce..527b716 100755\n--- a/gitk\n+++ b/gitk\n@@ -3697,7 +3697,9 @@ proc drawcommits {row {endrow {}}} {\n \n \tif {[info exists lineends($r)]} {\n \t foreach lid $lineends($r) {\n-\t\tunset prevlines($lid)\n+\t\tif {[info exists prevlines($lid)]} {\n+\t\t unset prevlines($lid)\n+\t\t}\n \t }\n \t}\n \tset rowids [lindex $rowidlist $r]\n-- \n1.5.3\n"},{"id":"56612","messageId":"alpine.LFD.0.999.0710191227340.26902@woody.linux-foundation.org","threadId":"10375","inReplyTo":"20071019052823.GI14735@spearce.org","subject":"Re: gitk patch collection pull request","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-19T19:31:47Z","receivedAt":"2007-10-19T19:31:47Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 19 Oct 2007, Shawn O. Pearce wrote:\n>\n> The following changes since commit 719c2b9d926bf2be4879015e3620d27d32f007b6:\n> Paul Mackerras (1):\n> gitk: Fix bug causing undefined variable error when cherry-picking\n\nThe biggest problem I have with gitk is that it is almost totally useless \nfor when you have a file limit.\n\nI do \"gitk --merge\" (which has an implicit file limit of the list \nof unmerged files) or just \"gitk ORIG_HEAD.. Makefile\" and the *history* \nof gitk looks fine.\n\nBut the diffs are crap and useless, because they contain all the stuff I \ndid *not* want, and hides all the relevant information.\n\nSo then I might use the nice gitk history window to see the commits, but I \nwill fall back on \"git log -p --merge\" and \"git log -p ORIG_HEAD.. Makefile\"\nfor the real work. \n\nI had that happen unusually many times lately, since we had a fair number \nof (mostly trivial) conflicts during this merge window. And it's sad. Gitk \nis so good in so many other ways, and then it is so *totally* useless when \nit comes to something as fundamental as just looking at the history of a \nfew files.\n\n\t\t\tLinus\n"},{"id":"56647","messageId":"20071020031048.GQ14735@spearce.org","threadId":"10375","inReplyTo":"18200.36704.936554.220173@cargo.ozlabs.ibm.com","subject":"Re: gitk patch collection pull request","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-20T03:10:48Z","receivedAt":"2007-10-20T03:10:48Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Paul Mackerras wrote:\n> Shawn O. Pearce writes:\n> > The following changes since commit 719c2b9d926bf2be4879015e3620d27d32f007b6:\n> > Paul Mackerras (1):\n> > gitk: Fix bug causing undefined variable error when cherry-picking\n> > \n> > are available in the git repository at:\n> > \n> > git://repo.or.cz:/git/spearce.git gitk\n...\n> > Jonathan del Strother (2):\n> > gitk: Added support for OS X mouse wheel\n> > Fixing gitk indentation\n> \n> This one is bogus. Firstly, it doesn't have \"gitk:\" at the start of\n> the headline (and \"Fixing\" should be \"Fix\"). Secondly, the actual\n> change itself is bogus. It changes an initial tab to 8 spaces on each\n> of 4 lines. I like it the way it is - and if he wanted to change it,\n> he should have changed it throughout the file, not just on 4 lines.\n> So that change is rejected.\n> \n> The other changes are OK. If you could re-do your tree without\n> 0d6df4de (and possible change \"Added\" to \"Add\" in e1b5683c while\n> you're at it), I'll do the pull.\n\nDone. I also added this one from Michele:\n\n From: Michele Ballabio \n Subject: [PATCH-resent] gitk: fix in procedure drawcommits\n\n-- \nShawn.\n"},{"id":"56650","messageId":"18201.34779.27836.531742@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"alpine.LFD.0.999.0710191227340.26902@woody.linux-foundation.org","subject":"Re: gitk patch collection pull request","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-20T04:45:15Z","receivedAt":"2007-10-20T04:45:15Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> The biggest problem I have with gitk is that it is almost totally useless \n> for when you have a file limit.\n> \n> I do \"gitk --merge\" (which has an implicit file limit of the list \n> of unmerged files) or just \"gitk ORIG_HEAD.. Makefile\" and the *history* \n> of gitk looks fine.\n> \n> But the diffs are crap and useless, because they contain all the stuff I \n> did *not* want, and hides all the relevant information.\n\nDo you mean that when you have a file limit, the diff window should\njust show the diffs for those files, not any other files the commit\nmight have modified? That would be easy enough to implement in gitk.\n\nPaul.\n"},{"id":"56651","messageId":"alpine.LFD.0.999.0710192149020.3794@woody.linux-foundation.org","threadId":"10375","inReplyTo":"18201.34779.27836.531742@cargo.ozlabs.ibm.com","subject":"Re: gitk patch collection pull request","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-20T04:51:27Z","receivedAt":"2007-10-20T04:51:27Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 20 Oct 2007, Paul Mackerras wrote:\n> \n> Do you mean that when you have a file limit, the diff window should\n> just show the diffs for those files, not any other files the commit\n> might have modified?\n\nYes. The same way \"git log -p\" works by default.\n\nWith perhaps a checkbox to toggle the \"--full-diff\" behaviour.\n\n> That would be easy enough to implement in gitk.\n\nWell, the \"--merged\" case is slightly trickier, since git will figure out \nthe pathnames on its own (it limits pathnames to the intersection of the \nnames you give one the command line *and* the list of unmerged files, ie \nthe \"filter\" becomes \"git ls-files -u [pathspec]\".\n\nBut goodie. I look forward to it ;)\n\n\t\tLinus\n"},{"id":"56685","messageId":"18201.54648.707559.480169@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"200710191544.22228.barra_cuda@katamail.com","subject":"Re: [PATCH-resent] gitk: fix in procedure drawcommits","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-20T10:16:24Z","receivedAt":"2007-10-20T10:16:24Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Michele Ballabio writes:\n\n> This patch indroduces a check before unsetting an array element.\n\nintroduces\n\n> There's an error that seems to occur in gitk only on\n> mutt's imported repo, but I don't know why. This is\n> hopefully the right fix.\n\nWell no. I'd rather understand why this is happening, in case the\nerror indicates that I'm not handling a corner case correctly. Can\nyou make a copy of the repo that triggers the bug available to me?\n\nPaul.\n"},{"id":"56690","messageId":"531A500E-667F-413C-BD20-D23DC817EB72@steelskies.com","threadId":"10375","inReplyTo":"18200.36704.936554.220173@cargo.ozlabs.ibm.com","subject":"Re: gitk patch collection pull request","fromName":"Jonathan del Strother","fromEmail":"maillist@steelskies.com","sentAt":"2007-10-20T11:12:37Z","receivedAt":"2007-10-20T11:12:37Z","isPatch":false,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"\nOn 19 Oct 2007, at 12:05, Paul Mackerras wrote:\n\n> Shawn O. Pearce writes:\n>\n>> The following changes since commit \n>> 719c2b9d926bf2be4879015e3620d27d32f007b6:\n>> Paul Mackerras (1):\n>> gitk: Fix bug causing undefined variable error when cherry- \n>> picking\n>>\n>> are available in the git repository at:\n>>\n>> git://repo.or.cz:/git/spearce.git gitk\n>\n> OK, but ...\n>\n>> Jonathan del Strother (2):\n>> gitk: Added support for OS X mouse wheel\n>> Fixing gitk indentation\n>\n> This one is bogus. Firstly, it doesn't have \"gitk:\" at the start of\n> the headline (and \"Fixing\" should be \"Fix\"). Secondly, the actual\n> change itself is bogus. It changes an initial tab to 8 spaces on each\n> of 4 lines. I like it the way it is - and if he wanted to change it,\n> he should have changed it throughout the file, not just on 4 lines.\n> So that change is rejected.\n\nIn my defense, most of that file is space indented, and the places \nthat are tab indented are generally totally broken unless you have an \n8 char tab width. It seems to have the whole 'tabs for code \nindentation, with space for alignment' rule back-to-front. I can't \nfollow the logic of that, so didn't feel comfortable changing the \nwhole file. I probably shouldn't have submitted the second patch - I \ninitially fixed the weird indentation in my first patch, just so my if- \nblock didn't look totally weird, but then was told that ought to be 2 \nseparate patches.\n"},{"id":"56695","messageId":"18201.60047.898077.579869@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"531A500E-667F-413C-BD20-D23DC817EB72@steelskies.com","subject":"Re: gitk patch collection pull request","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-20T11:46:23Z","receivedAt":"2007-10-20T11:46:23Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Jonathan del Strother writes:\n\n> In my defense, most of that file is space indented, and the places \n\nOnly the lines that are indented 1 level start with spaces. Any line\nthat is indented 2 or more levels should start with a tab.\n\n> that are tab indented are generally totally broken unless you have an \n> 8 char tab width.\n\nSo set your tabs to 8 spaces when looking at it. :)\n\n> It seems to have the whole 'tabs for code \n> indentation, with space for alignment' rule back-to-front.\n\nI don't recall signing up to that rule. :) I use 4-column indentation\nand 8-column tabs, and my editor (emacs) handles it all automatically\nfor me.\n\nPaul.\n"},{"id":"56698","messageId":"B3349B61-995B-42D0-B777-CEA618943848@steelskies.com","threadId":"10375","inReplyTo":"18201.60047.898077.579869@cargo.ozlabs.ibm.com","subject":"Re: gitk patch collection pull request","fromName":"Jonathan del Strother","fromEmail":"maillist@steelskies.com","sentAt":"2007-10-20T13:00:20Z","receivedAt":"2007-10-20T13:00:20Z","isPatch":false,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"\nOn 20 Oct 2007, at 12:46, Paul Mackerras wrote:\n\n> Jonathan del Strother writes:\n>\n>> In my defense, most of that file is space indented, and the places\n>\n> Only the lines that are indented 1 level start with spaces. Any line\n> that is indented 2 or more levels should start with a tab.\n\n>> It seems to have the whole 'tabs for code\n>> indentation, with space for alignment' rule back-to-front.\n>\n> I don't recall signing up to that rule. :) I use 4-column indentation\n> and 8-column tabs, and my editor (emacs) handles it all automatically\n> for me.\n\n\nUgh... I don't usually get involved in tab/space wars, but I'm \ncurious... why on earth would you choose this style?\nWith space indentation you can make sure that everyone sees the \nindentation as it was intended. With tab indentation, you save space, \nadd semantic meaning, and let people control how wide they want their \nindents to appear. This approach seems to take the worst parts of \neach and combine them. What's the benefit?\n\nI appreciate I'm not going to convert you - this is an honest question.\n"},{"id":"56710","messageId":"20071020153216.GD19521@efreet.light.src","threadId":"10375","inReplyTo":"B3349B61-995B-42D0-B777-CEA618943848@steelskies.com","subject":"Re: gitk patch collection pull request","fromName":"Jan Hudec","fromEmail":"bulb@ucw.cz","sentAt":"2007-10-20T15:32:16Z","receivedAt":"2007-10-20T15:32:16Z","isPatch":false,"sender":{"key":"bulb@ucw.cz","avatar":null},"body":"On Sat, Oct 20, 2007 at 14:00:20 +0100, Jonathan del Strother wrote:\n>\n> On 20 Oct 2007, at 12:46, Paul Mackerras wrote:\n>\n>> Jonathan del Strother writes:\n>>\n>>> In my defense, most of that file is space indented, and the places\n>>\n>> Only the lines that are indented 1 level start with spaces. Any line\n>> that is indented 2 or more levels should start with a tab.\n>\n>>> It seems to have the whole 'tabs for code\n>>> indentation, with space for alignment' rule back-to-front.\n>>\n>> I don't recall signing up to that rule. :) I use 4-column indentation\n>> and 8-column tabs, and my editor (emacs) handles it all automatically\n>> for me.\n>\n>\n> Ugh... I don't usually get involved in tab/space wars, but I'm curious... \n> why on earth would you choose this style?\n\nBecause that's default behaviour of both emacs and vi when you set\nindentation different from tabstop. Actually most of GNU software, whether it\nuses the GNU standard indent of 2, or more, uses tabs for any indents\nover 8. Probably even most unix software uses this.\n\nActually, even if the indent is 8, function arguments are often aligned under\nthe open parenthesis and a tabs + spaces combination is normally used for\nthat as well (because, again, that's what most editors will by default do!).\n\n> With space indentation you can make sure that everyone sees the indentation \n> as it was intended. With tab indentation, you save space, add semantic \n> meaning, and let people control how wide they want their indents to appear. \n> This approach seems to take the worst parts of each and combine them. \n> What's the benefit?\n\nTab stops are every 8 characters. No more, no less. Ever. This makes the text\nwith whatever formating you want the shortest.\n\n> I appreciate I'm not going to convert you - this is an honest question.\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n-- \n\t\t\t\t\t\t Jan 'Bulb' Hudec \n"},{"id":"56711","messageId":"200710201802.48111.barra_cuda@katamail.com","threadId":"10375","inReplyTo":"18201.54648.707559.480169@cargo.ozlabs.ibm.com","subject":"Re: [PATCH-resent] gitk: fix in procedure drawcommits","fromName":"Michele Ballabio","fromEmail":"barra_cuda@katamail.com","sentAt":"2007-10-20T16:02:47Z","receivedAt":"2007-10-20T16:02:47Z","isPatch":true,"sender":{"key":"barra_cuda@katamail.com","avatar":"https://avatars.githubusercontent.com/u/16371673?v=4"},"body":"[Rocco Rutte added on CC:, since he wrote the hg-fast-export scripts]\n\nOn Saturday 20 October 2007, Paul Mackerras wrote:\n> Well no.  I'd rather understand why this is happening, in case the \n> error indicates that I'm not handling a corner case correctly.  Can\n> you make a copy of the repo that triggers the bug available to me?\n\nIIRC, I just cloned mutt's hg repo:\n\n hg clone http://dev.mutt.org/hg/mutt\n\nthen imported it in git with the scripts at\n\n http://repo.or.cz/w/fast-export.git\n\nwith\n\n hg-fast-export.sh -r ../mutt\n\n\nAfter that, I've done the usual maintenance repack.\n\nThen, running gitk and keeping pressed pgdown triggers that\n\n\t\"Error: can't unset...\"\n\nwindow.\n\nUh-oh. I think I just found the issue. That's probably a bug\nsomewhere in the import (either fast-export or fast-import or\nthe original repo, I don't know), so I'm not sure if gitk\nshould be patched, but since the resulting repo seems correct\nas far as git is concerned (i.e. git fsck --full --strict\ndoesn't complain), I guess something should be done.\n\nHere is the culprit (or so I think). One of the guilty commits is:\n\n\tcommit a3b4383d69e0754346578c85ba8ff7c05bd88705\n\ttree 1bf99cd22abe97c59f8c0b7ad6b8244f0854b8af\n\tparent 6d919fccf603aba995035fa0fb507aa2bd3bf0ae\n\tparent 6d919fccf603aba995035fa0fb507aa2bd3bf0ae\n\tauthor Brendan Cully 1179646159 -0700\n\tcommitter Brendan Cully 1179646159 -0700\n\t\n\t Forget SMTP password if authentication fails.\n\t Thanks to Gregory Shapiro for the initial patch (I've moved the reset\n\t from smtp_auth_sasl up to smtp_auth, and used the account API\n\t instead of twiddling account bits by hand). Closes #2872.\n\nThis commit (and many others) has two parents, but the two parents\nhave the same hash. So gitk tries to unset the same variable twice,\nhence the error. At this point, the fix for gitk should be either to\ncheck if the parents have the same hash when reading the commit or\navoiding to unset two times the same variable.\n\nThis explanation makes sense to me, now the problem is: have I messed\nup the import myself, the scripts/commands used are to blame, or is\nit entirely the original repo's fault?\n\nSince I've redone the import and the error remains, I guess\nthat's not my fault :)\n"},{"id":"56717","messageId":"20071020183533.GC8887@efreet.light.src","threadId":"10375","inReplyTo":"200710201802.48111.barra_cuda@katamail.com","subject":"Re: [PATCH-resent] gitk: fix in procedure drawcommits","fromName":"Jan Hudec","fromEmail":"bulb@ucw.cz","sentAt":"2007-10-20T18:35:33Z","receivedAt":"2007-10-20T18:35:33Z","isPatch":true,"sender":{"key":"bulb@ucw.cz","avatar":null},"body":"On Sat, Oct 20, 2007 at 18:02:47 +0200, Michele Ballabio wrote:\n> IIRC, I just cloned mutt's hg repo:\n> hg clone http://dev.mutt.org/hg/mutt\n> then imported it in git with the scripts at\n> http://repo.or.cz/w/fast-export.git\n> with\n> hg-fast-export.sh -r ../mutt\n> [...]\n> \n> Here is the culprit (or so I think). One of the guilty commits is:\n> \n> \tcommit a3b4383d69e0754346578c85ba8ff7c05bd88705\n> \ttree 1bf99cd22abe97c59f8c0b7ad6b8244f0854b8af\n> \tparent 6d919fccf603aba995035fa0fb507aa2bd3bf0ae\n> \tparent 6d919fccf603aba995035fa0fb507aa2bd3bf0ae\n> \tauthor Brendan Cully 1179646159 -0700\n> \tcommitter Brendan Cully 1179646159 -0700\n> \t\n> \t Forget SMTP password if authentication fails.\n> \t Thanks to Gregory Shapiro for the initial patch (I've moved the reset\n> \t from smtp_auth_sasl up to smtp_auth, and used the account API\n> \t instead of twiddling account bits by hand). Closes #2872.\n\nJudging from the symptoms, I would suspect hg-fast-export. Either mercurial\nsometimes stores two same hashes instead of the hash and 0 (in which case\nhg-fast-import should probably be ready to deal with it), or hg-fast-import\ndoes something wrong when it sees the 0 parent.\n\n> This commit (and many others) has two parents, but the two parents\n> have the same hash. So gitk tries to unset the same variable twice,\n> hence the error. At this point, the fix for gitk should be either to\n> check if the parents have the same hash when reading the commit or\n> avoiding to unset two times the same variable.\n> \n> This explanation makes sense to me, now the problem is: have I messed\n> up the import myself, the scripts/commands used are to blame, or is\n> it entirely the original repo's fault?\n> \n> Since I've redone the import and the error remains, I guess\n> that's not my fault :)\n\n-- \n\t\t\t\t\t\t Jan 'Bulb' Hudec \n"},{"id":"56738","messageId":"18202.49408.264600.839673@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"200710201802.48111.barra_cuda@katamail.com","subject":"Re: [PATCH-resent] gitk: fix in procedure drawcommits","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-21T03:01:20Z","receivedAt":"2007-10-21T03:01:20Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Michele Ballabio writes:\n\n> This commit (and many others) has two parents, but the two parents\n> have the same hash. So gitk tries to unset the same variable twice,\n> hence the error. At this point, the fix for gitk should be either to\n> check if the parents have the same hash when reading the commit or\n> avoiding to unset two times the same variable.\n\nActually, there is a commit like that in the kernel tree, and with\nthis clue, I have managed to reproduce the problem on the kernel tree\nwith the command\n\n\tgitk v2.6.12-rc2..13e652800d1644dfedcd0d59ac95ef0beb7f3165\n\nI have just pushed out a fix to my gitk.git tree.\n\nPaul.\n"},{"id":"56768","messageId":"20071021121236.GA290@localhost.daprodeges.fqdn.th-h.de","threadId":"10375","inReplyTo":"200710201802.48111.barra_cuda@katamail.com","subject":"Re: [PATCH-resent] gitk: fix in procedure drawcommits","fromName":"Rocco Rutte","fromEmail":"pdmef@gmx.net","sentAt":"2007-10-21T12:12:36Z","receivedAt":"2007-10-21T12:12:36Z","isPatch":true,"sender":{"key":"pdmef@gmx.net","avatar":null},"body":"[ No need to Cc: me as I'm on the git list, too ]\n\nHi,\n\n* Michele Ballabio [07-10-20 18:02:47 +0200] wrote:\n\n>Uh-oh. I think I just found the issue. That's probably a bug\n>somewhere in the import (either fast-export or fast-import or\n>the original repo, I don't know), so I'm not sure if gitk\n>should be patched, but since the resulting repo seems correct\n>as far as git is concerned (i.e. git fsck --full --strict\n>doesn't complain), I guess something should be done.\n\n>Here is the culprit (or so I think). One of the guilty commits is:\n\n>\tcommit a3b4383d69e0754346578c85ba8ff7c05bd88705\n>\ttree 1bf99cd22abe97c59f8c0b7ad6b8244f0854b8af\n>\tparent 6d919fccf603aba995035fa0fb507aa2bd3bf0ae\n>\tparent 6d919fccf603aba995035fa0fb507aa2bd3bf0ae\n>\tauthor Brendan Cully 1179646159 -0700\n>\tcommitter Brendan Cully 1179646159 -0700\n\n>\t Forget SMTP password if authentication fails.\n>\t Thanks to Gregory Shapiro for the initial patch (I've moved the reset\n>\t from smtp_auth_sasl up to smtp_auth, and used the account API\n>\t instead of twiddling account bits by hand). Closes #2872.\n\nOh. Yes, this is a bug in the python scripts that get merges quite \nwrong. I didn't notice that earlier as git doesn't complain and the \ncontents of the repo turns out as identical.\n\nI'll push fixes (e.g. packed refs support) to the fast-export repo in \nMonday. With these changes, the mutt repo as well hg-crew (which has far \nmore merges than the mutt repo) seem to work correctly (no identical \nparents and identical contents).\n\nThanks for tracking it down.\n\nStill I think fast-import could warn or error out if its gets such \ncontent which doesn't really make sense...\n\nRocco\n"},{"id":"56932","messageId":"18205.15967.792413.775786@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"alpine.LFD.0.999.0710192149020.3794@woody.linux-foundation.org","subject":"Re: gitk patch collection pull request","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-23T00:20:47Z","receivedAt":"2007-10-23T00:20:47Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> On Sat, 20 Oct 2007, Paul Mackerras wrote:\n> > \n> > Do you mean that when you have a file limit, the diff window should\n> > just show the diffs for those files, not any other files the commit\n> > might have modified?\n> \n> Yes. The same way \"git log -p\" works by default.\n> \n> With perhaps a checkbox to toggle the \"--full-diff\" behaviour.\n\nOK, done. The checkbox is in the Edit/Preferences window. It's\ncalled \"Limit diffs to listed paths\" and it's on by default.\n\n> > That would be easy enough to implement in gitk.\n> \n> Well, the \"--merged\" case is slightly trickier, since git will figure out \n> the pathnames on its own (it limits pathnames to the intersection of the \n> names you give one the command line *and* the list of unmerged files, ie \n> the \"filter\" becomes \"git ls-files -u [pathspec]\".\n\nIf you use the --merge flag, gitk will do a git ls-files -u at\nstartup, and use the result as the list of paths (after intersecting\nit with the paths on the command line, if you specify paths there).\n\nI pondered whether I needed to re-do the git ls-files -u when you\nupdate the view with File->Update. I decided not to for now, but it\nwould be possible to add it.\n\n> But goodie. I look forward to it ;)\n\nI just pushed it out to my gitk.git repo (master branch). Enjoy. :)\n\nPaul.\n"},{"id":"57017","messageId":"alpine.LFD.0.999.0710231214150.30120@woody.linux-foundation.org","threadId":"10375","inReplyTo":"18205.15967.792413.775786@cargo.ozlabs.ibm.com","subject":"Re: gitk patch collection pull request","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-23T19:17:36Z","receivedAt":"2007-10-23T19:17:36Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 23 Oct 2007, Paul Mackerras wrote:\n> \n> OK, done. The checkbox is in the Edit/Preferences window. It's\n> called \"Limit diffs to listed paths\" and it's on by default.\n\nOk, the diff looks fine, but now the \"list of files\" pane on the right is \nempty. \n\nEven when you limit the diff output, you often have lots of files. At \nleast I do, because I often limit by subdirectory. So I'd still like to \nsee the file list on the right, so that I can jump to a particular part of \nthe diff.\n\n(It would also be nice if it showed the *size* of the changes a'la \ndiffstat or something, but that's another and independent issue).\n\n\t\tLinus\n"},{"id":"57036","messageId":"18206.34254.741787.255299@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"alpine.LFD.0.999.0710231214150.30120@woody.linux-foundation.org","subject":"Re: gitk patch collection pull request","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-23T23:37:50Z","receivedAt":"2007-10-23T23:37:50Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> Ok, the diff looks fine, but now the \"list of files\" pane on the right is \n> empty. \n\nReally? It looks OK here - that is, it lists the names of the files\nwhose diffs are shown on the left, i.e. the files modified by the\ncommit that are within the path limit.\n\nIs it completely empty, or does it have just the \"Comments\" entry at\nthe top?\n\nCan you give me an example of a gitk command line that shows the\nproblem on the kernel tree?\n\nPaul.\n"},{"id":"57038","messageId":"alpine.LFD.0.999.0710231647350.30120@woody.linux-foundation.org","threadId":"10375","inReplyTo":"18206.34254.741787.255299@cargo.ozlabs.ibm.com","subject":"Re: gitk patch collection pull request","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-23T23:51:33Z","receivedAt":"2007-10-23T23:51:33Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 24 Oct 2007, Paul Mackerras wrote:\n> \n> Is it completely empty, or does it have just the \"Comments\" entry at\n> the top?\n\nJust the comment.\n\n> Can you give me an example of a gitk command line that shows the\n> problem on the kernel tree?\n\nHappened for just a random directory I tested. According to my bash \nhistory, it seems to have been\n\n\tgitk v2.6.23.. drivers/char/\n\nwhich is pretty basic..\n\n\t\tLinus\n"},{"id":"57041","messageId":"18206.36650.787899.517514@cargo.ozlabs.ibm.com","threadId":"10375","inReplyTo":"alpine.LFD.0.999.0710231647350.30120@woody.linux-foundation.org","subject":"Re: gitk patch collection pull request","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-10-24T00:17:46Z","receivedAt":"2007-10-24T00:17:46Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> Happened for just a random directory I tested. According to my bash \n> history, it seems to have been\n> \n> \tgitk v2.6.23.. drivers/char/\n\nAhhhh... It's the slash on the end that does it (it works properly\nwithout the slash). I just pushed out a fix for that (and a couple of\nother bugs I just found).\n\nPaul.\n"}]}