{"thread":{"id":"8532","subject":"That improved git-gui blame viewer..","startedAt":"2007-06-09T18:26:45Z","lastAt":"2007-06-14T06:24:40Z","messageCount":29,"participants":["Linus Torvalds","Junio C Hamano","Shawn O. Pearce","Marco Costalba","Johannes Schindelin","Josef Weidendorfer","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"44475","messageId":"alpine.LFD.0.98.0706091117510.20321@woody.linux-foundation.org","threadId":"8532","inReplyTo":null,"subject":"That improved git-gui blame viewer..","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-06-09T18:26:45Z","receivedAt":"2007-06-09T18:26:45Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nShawn, could you please push it out somewhere else than just \"pu\", and \nconvince Junio to merge it? Right now, \"git gui blame\" is almost useless \nin the standard git tree, and maybe the new one has some bugs, but it's at \nleast _useful_ (and about a million times prettier than it used to be), so \nI don't think you can possibly regress in this area.\n\nThat said, I do have one comment about the state of git-gui \"pu\".. I think \nit's fairly pretty, and definitely useful, but one feature I end up really \nwishing for is a \"search\" button (or Ctrl-F). I might not know what \nline-number I'm looking for, I'm more likely to know which function I want \nto look at, and the most natural way to find it is with a simple ctrl-F.\n\nMaybe it exists, but if so, I didn't find it.\n\nBut the old git-gui blame viewer didn't have that either, so this \nshouldn't hold up getting a useable git-gui into the standard git distro..\n\n\t\tLinus\n"},{"id":"44477","messageId":"7v3b11f0bf.fsf@assigned-by-dhcp.cox.net","threadId":"8532","inReplyTo":"alpine.LFD.0.98.0706091117510.20321@woody.linux-foundation.org","subject":"Re: That improved git-gui blame viewer..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-09T18:29:56Z","receivedAt":"2007-06-09T18:29:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Shawn, could you please push it out somewhere else than just \"pu\", and \n> convince Junio to merge it? Right now, \"git gui blame\" is almost useless \n\nI do not need convincing but \"Hey, pull it\" needs to be given,\nespecially as I do not use git-gui myself. The last time I\nlooked at it, I think I said I liked it much better than the\nprevious one.\n"},{"id":"44697","messageId":"20070611064203.GG6073@spearce.org","threadId":"8532","inReplyTo":"alpine.LFD.0.98.0706091117510.20321@woody.linux-foundation.org","subject":"Re: That improved git-gui blame viewer..","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-11T06:42:03Z","receivedAt":"2007-06-11T06:42:03Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Linus Torvalds wrote:\n> That said, I do have one comment about the state of git-gui \"pu\".. I think \n> it's fairly pretty, and definitely useful,\n\nThanks! I'm glad at least one other person on this planet finds\nmy work on the blame viewer at least partially useful.\n\n> but one feature I end up really \n> wishing for is a \"search\" button (or Ctrl-F). I might not know what \n> line-number I'm looking for, I'm more likely to know which function I want \n> to look at, and the most natural way to find it is with a simple ctrl-F.\n\nNo, there isn't a search. Tk internally offers one, I just haven't\nbound UI to it. Ctrl-F is actually bound to page down, much as it is\nin vi. Actually vi keys work everywhere in git-gui for scrolling.\n\nI pushed out 0.7.3 tonight without search. I'll work on search and\njump-to-line this week, and try to get an 0.7.4 early next week\nwith those and maybe some other improvements in the blame viewer\nthat I want to get done.\n\nI may actually wind up just doing 0.8.0 sooner than I had expected.\nThere's a lot of new features queued up in the master branch that I\nslated for 0.8.0 that I'm using on a daily basis and is production\nstable.\n\nI wanted to implement some slick inotify() based features for the\nmain window during 0.8.0, but I haven't had time to work on them.\nIt would really improve performance of git-gui on Windows NT, and\nI do have a lot of git-gui users there.\n\n-- \nShawn.\n"},{"id":"44732","messageId":"alpine.LFD.0.98.0706110826590.14121@woody.linux-foundation.org","threadId":"8532","inReplyTo":"20070611064203.GG6073@spearce.org","subject":"Re: That improved git-gui blame viewer..","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-06-11T15:46:25Z","receivedAt":"2007-06-11T15:46:25Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 11 Jun 2007, Shawn O. Pearce wrote:\n> \n> Thanks! I'm glad at least one other person on this planet finds\n> my work on the blame viewer at least partially useful.\n\nI think a lot of people are going to enjoy the git-gui blame viewer..\n\nI don't generally do \"git blame\" very much at all, and I've found that any \ntime I do it (even historically - when I've used bk annotate and even \nCVS), I have invariably _always_ wanted to go back in history to see the \nblame of the *previous* version (because the commit that gets blamed \ninitially is simply not interesting, and is just whitespace cleanups or \nwhatever!).\n\nNow, this was something I knew very well on an intellectual level, and I \ntried to explain in one of my early discourses on why \"annotate\" is such a \nhorribly interface, and why git could do so much better, but it's \nsomething that I think \"git-gui blame\" is showing that could really be \ndone.\n\nNow, admittedly, git-gui seems buggy in this regard (I *think* you meant \nthat the two commits that show up listed to the left of each line are the \n\"before and after\" commit, but they end both being the \"after\" commit for \nme, but I can almost *taste* git-gui doing the right thing here. And once \nit does that, I think a lot of people will start to realize how useful \nthis kind of interactive \"drill down into the history of a line\" thing can \nbe.\n\nSo I think git-gui isn't quite there yet, but I think it's getting pretty \nclose. It would need\n\n - the afore-mentioned bug fixed (I _think_ it is a bug, but maybe you had \n some other reason for having two separate columns of commits per line), \n so that you can literally click on the *previous* version of a line and \n say \"I don't care about this commit, I want to see the previous one\".\n\n - I'd love to see a \"go to most recent change\" thing (the way _I_ work, I \n tend to peel history, so I often look at recent changes, but if that \n one isn't interesting, I'd go to the \"previous version\" and then again \n ask for \"go to the most recent change\".\n\n - I like the floating tooltip showing what the commit was, but I think it \n could be extended a bit more to perhaps literally show what the \n previous contents of that chunk were (again, that's useful for the \"am \n I interested in _this_ commit, or should I just dig down to the \n previous one\")\n\nBut yes, the current git-gui blame is already quite interesting. It's not \n\"all that it could be\", but it has gone from \"Whee, that looks so ugly \nthat I need to find me a couple of spoons to dig out my eyeballs with\" to \n\"Hey, I can see this being _really_ good\".\n\n\t\tLinus\n"},{"id":"44734","messageId":"7vveduqxxr.fsf@assigned-by-dhcp.pobox.com","threadId":"8532","inReplyTo":"alpine.LFD.0.98.0706110826590.14121@woody.linux-foundation.org","subject":"Re: That improved git-gui blame viewer..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-11T16:05:04Z","receivedAt":"2007-06-11T16:05:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> I don't generally do \"git blame\" very much at all, and I've found that any \n> time I do it (even historically - when I've used bk annotate and even \n> CVS), I have invariably _always_ wanted to go back in history to see the \n> blame of the *previous* version (because the commit that gets blamed \n> initially is simply not interesting, and is just whitespace cleanups or \n> whatever!).\n\nIncidentally I added a small patch to underlying git-blame to\nlet you ignore whitespace changes so that you can blame through\nthem. I do not think I merged it to 'master' yet, but it is\ntrivial and look obviously safe and correct.\n\n> So I think git-gui isn't quite there yet, but I think it's getting pretty \n> close. It would need\n>\n> - the afore-mentioned bug fixed (I _think_ it is a bug, but maybe you had \n> some other reason for having two separate columns of commits per line), \n> so that you can literally click on the *previous* version of a line and \n> say \"I don't care about this commit, I want to see the previous one\".\n\nI think the two columns are for \"who _placed_ these lines in the\nfinal image\" vs \"where these lines originally came into the\nhistory\". The former is with -C, and the latter is without.\n\nAn option to re-blame starting from the parent commit of what is\ncurrently blamed (i.e. \"peel\" one level) would certainly be\ninteresting but I do not think git-gui has it (yet). Of course\nit has a not-so-interesting corner case of deciding which parent\nto follow for a merge, but a merge commit is blamed only for\nlines that are either evil or conflict fixups, so in practice\nthere is not much to be gained from peeling a merge.\n"},{"id":"44797","messageId":"e5bfff550706112316j42c7c8e8uf8383de990ca9707@mail.gmail.com","threadId":"8532","inReplyTo":"7vveduqxxr.fsf@assigned-by-dhcp.pobox.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-12T06:16:01Z","receivedAt":"2007-06-12T06:16:01Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/11/07, Junio C Hamano wrote:\n>\n> An option to re-blame starting from the parent commit of what is\n> currently blamed (i.e. \"peel\" one level) would certainly be\n> interesting but I do not think git-gui has it (yet).\n\nNot to advertise, but qgit has already that from ages.\n\nAnnotate algorithm of qgit is little different in that it starts from\nthe oldest revision that modified a file and goes to the latest. In\nthis way we can have the whole file history annotated in one pass and\nvery fast.\n\nCurrently the slowest steps are, from fastest to slowest:\n\n1 - annotating the files\n2 - getting the file history\n3 - getting the corrisponding patches (in case of long histories)\n\n\nThanks for your patience.\nMarco\n"},{"id":"44800","messageId":"7vbqflll55.fsf@assigned-by-dhcp.pobox.com","threadId":"8532","inReplyTo":"e5bfff550706112316j42c7c8e8uf8383de990ca9707@mail.gmail.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-12T06:52:38Z","receivedAt":"2007-06-12T06:52:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marco Costalba\" writes:\n\n> On 6/11/07, Junio C Hamano wrote:\n>>\n>> An option to re-blame starting from the parent commit of what is\n>> currently blamed (i.e. \"peel\" one level) would certainly be\n>> interesting but I do not think git-gui has it (yet).\n>\n> Not to advertise, but qgit has already that from ages.\n\nAdvertising is good.\n\n> Annotate algorithm of qgit is little different in that it starts from\n> the oldest revision that modified a file and goes to the latest. In\n> this way we can have the whole file history annotated in one pass and\n> very fast.\n\nI am not sure about two things in this description.\n\n (1) Are you emulating CVS-like \"a file has an identity, and we\n follow its changes\" model? How does it handle file split,\n merge, and code movement in general?\n\n (2) It is unclear why going from old to new has the advantage\n of being \"one pass\", implication of which is that the\n opposite direction needs to be done as more than one pass.\n Care to enlighten?\n"},{"id":"44822","messageId":"e5bfff550706120427g7ad9d38bpc34d9ea284ace693@mail.gmail.com","threadId":"8532","inReplyTo":"7vbqflll55.fsf@assigned-by-dhcp.pobox.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-12T11:27:19Z","receivedAt":"2007-06-12T11:27:19Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/12/07, Junio C Hamano wrote:\n>\n> > Annotate algorithm of qgit is little different in that it starts from\n> > the oldest revision that modified a file and goes to the latest. In\n> > this way we can have the whole file history annotated in one pass and\n> > very fast.\n>\n> I am not sure about two things in this description.\n>\n> (1) Are you emulating CVS-like \"a file has an identity, and we\n> follow its changes\" model? How does it handle file split,\n> merge, and code movement in general?\n>\n\nIt uses 'git rev-list HEAD -- ' to get the list of revisions\nthat modified a path,\nI really would like to keep it like that because it is the way 'git'\nworks, and I would feel uncomfortable in filtering out git results, it\nseems quite fragile to me.\n\nThis means that file splits, merges, renames etc.. are handled as much\nas they are handled in git. IOW *if* 'git rev-list HEAD -- '\nreturns a list of revisions taking in account all of the above, so it\nwill, automatically, do qgit.\n\nBTW _currentlly_ git-rev-list does not do that.\n\n\n> (2) It is unclear why going from old to new has the advantage\n> of being \"one pass\", implication of which is that the\n> opposite direction needs to be done as more than one pass.\n> Care to enlighten?\n>\n\nGoing from oldest to newest has this advantage:\n\n1 - start from a known good first (empty) annotation, i.e. the first\nrevision in history has an empty annotation (this is a choice to get\nconsistent results when dealing with git repository started after the\nbegining of the project, Linux tree started from 2.6.12 is an\nexample).\n\n2 - Given a good annotation (ann1) at a given time in history you can\ncalculate the next annotation (ann2), the annotation corresponding to\nthe next (newer) revision in history that modified the file using just\nthe diff between the two. If you don't discards ann1 you end up having\nboth ann1 and ann2.\n\n3 - So at the end of applying all the diff chain you get all the\nannotations for all the file revisions, each annotation requires only\nthe previous one and the corresponding diff.\n\n4 - You don't need to touch anymore an already calculated file, nor\napplying the corresponding diff more then one time.\n\nSo the complexity of annotating *all* the files revisions grows only\nlinearly with the revision list size.\n\n Marco\n"},{"id":"44823","messageId":"e5bfff550706120458o3b969c70rc065f67777ab1805@mail.gmail.com","threadId":"8532","inReplyTo":"e5bfff550706120427g7ad9d38bpc34d9ea284ace693@mail.gmail.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-12T11:58:25Z","receivedAt":"2007-06-12T11:58:25Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/12/07, Marco Costalba wrote:\n>\n> So the complexity of annotating *all* the files revisions grows only\n> linearly with the revision list size.\n>\n\nGoing from newest to oldest, after having elaborated all the diffs\nbetween revisions you have the newest file correctly annotated.\n\nGoing from oldest to newest, after having elaborated *in the same way\nas above* all the diffs between revisions, you have _all_ the files\ncorrectly annotated.\n"},{"id":"44836","messageId":"20070612135303.GT6073@spearce.org","threadId":"8532","inReplyTo":"e5bfff550706120427g7ad9d38bpc34d9ea284ace693@mail.gmail.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-12T13:53:03Z","receivedAt":"2007-06-12T13:53:03Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Marco Costalba wrote:\n> On 6/12/07, Junio C Hamano wrote:\n> >\n> > (1) Are you emulating CVS-like \"a file has an identity, and we\n> > follow its changes\" model? How does it handle file split,\n> > merge, and code movement in general?\n> >\n> \n> It uses 'git rev-list HEAD -- ' to get the list of revisions\n> that modified a path,\n\nSo apparently yes, qgit is emulating CVS. And yet much better things\nexist (git-blame).\n\n> I really would like to keep it like that because it is the way 'git'\n> works, and I would feel uncomfortable in filtering out git results, it\n> seems quite fragile to me.\n\nIts not \"the way git works\". Its just one way of looking at the data\nin the object database. And its not as accurate or as interesting\nas what git-blame does.\n\n> This means that file splits, merges, renames etc.. are handled as much\n> as they are handled in git. IOW *if* 'git rev-list HEAD -- '\n> returns a list of revisions taking in account all of the above, so it\n> will, automatically, do qgit.\n> \n> BTW _currentlly_ git-rev-list does not do that.\n\nAnd it may never do it. Most of the split/merge code is actually\nin git-blame and is probably too CPU intensive to slam into the\nmiddle of git-rev-list as a path limiter operator. But renames\n*might* someday be included.\n \n> Going from oldest to newest has this advantage:\n\nIt is unclear why you aren't just using `git blame --incremental`.\ngit-blame running in two passes (with and without -M -C -C) can yield\nsome very interesting results on files, like Git's own revision.c.\nAnd with the new -w flag that Junio just added, there's even more\ninteresting possibilities...\n\n;-)\n\n-- \nShawn.\n"},{"id":"44864","messageId":"7v645thto7.fsf@assigned-by-dhcp.pobox.com","threadId":"8532","inReplyTo":"20070612135303.GT6073@spearce.org","subject":"Re: That improved git-gui blame viewer..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-12T19:14:16Z","receivedAt":"2007-06-12T19:14:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Marco Costalba wrote:\n>> On 6/12/07, Junio C Hamano wrote:\n>> >\n>> > (1) Are you emulating CVS-like \"a file has an identity, and we\n>> > follow its changes\" model? How does it handle file split,\n>> > merge, and code movement in general?\n>> >\n>> \n>> It uses 'git rev-list HEAD -- ' to get the list of revisions\n>> that modified a path,\n>\n> So apparently yes, qgit is emulating CVS. And yet much better things\n> exist (git-blame).\n\nI would not use the word \"better\", as it depends on what you are\nlooking for.\n\n>> I really would like to keep it like that because it is the way 'git'\n>> works, and I would feel uncomfortable in filtering out git results, it\n>> seems quite fragile to me.\n>\n> Its not \"the way git works\". Its just one way of looking at the data\n> in the object database. And its not as accurate or as interesting\n> as what git-blame does.\n\nAgain, I would not say \"accurate\".\n\nThe way Marco describes is a perfectly valid way to satisfy\nexpectations of people migrating from CVS. It's more faithful\nreproduction of CVS annotate behaviour. In a sense, git-blame\ndoes too much, but that is exactly why these \"accurate and\ninteresting\" behaviours are optional.\n\n>> This means that file splits, merges, renames etc.. are handled as much\n>> as they are handled in git. IOW *if* 'git rev-list HEAD -- '\n>> returns a list of revisions taking in account all of the above, so it\n>> will, automatically, do qgit.\n>> \n>> BTW _currentlly_ git-rev-list does not do that.\n>\n> And it may never do it.\n\nOh, I can guarantee you that git-rev-list will never ever do\nthat. It is to traverse revisions while simplifying with path\nlimiters, and path limiters by definition will not look inside\ncontents. Think of it as asking \"Had my project consisted of\nonly arch/i386 and include/asm-i386 directories, what would the\nhistory be\".\n\nHowever, adding a new option to \"git log\" so that you can say\n'git log --single-follow=$this_file_at_the_tip $branch' is a\nseparate matter. I think it is a sensible thing to do. But\neven if we do that, I think --single-follow should limit itself\nto the rename following of \"diff -M/-C\" style. It is insane to\ninclude another path only because the result did copy&paste only\na handful lines out of during its history.\n"},{"id":"44915","messageId":"e5bfff550706130403y417b3e2dmd268c57c06b658fb@mail.gmail.com","threadId":"8532","inReplyTo":"20070612135303.GT6073@spearce.org","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-13T11:03:52Z","receivedAt":"2007-06-13T11:03:52Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/12/07, Shawn O. Pearce wrote:\n>\n> It is unclear why you aren't just using `git blame --incremental`.\n> git-blame running in two passes (with and without -M -C -C) can yield\n> some very interesting results on files, like Git's own revision.c.\n> And with the new -w flag that Junio just added, there's even more\n> interesting possibilities...\n>\n\nI will try to explore with git-blame better, thanks.\n\nMarco\n"},{"id":"44917","messageId":"e5bfff550706130411g151b76e3sa2e351aff5dbb35b@mail.gmail.com","threadId":"8532","inReplyTo":"7v645thto7.fsf@assigned-by-dhcp.pobox.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-13T11:11:16Z","receivedAt":"2007-06-13T11:11:16Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/12/07, Junio C Hamano wrote:\n>\n> However, adding a new option to \"git log\" so that you can say\n> 'git log --single-follow=$this_file_at_the_tip $branch' is a\n> separate matter. I think it is a sensible thing to do. But\n> even if we do that, I think --single-follow should limit itself\n> to the rename following of \"diff -M/-C\" style. It is insane to\n> include another path only because the result did copy&paste only\n> a handful lines out of during its history.\n>\n\nSome weeks ago I've started to experiment with git-log as a\nreplacement for git-rev-list (as suggested by Linus in a thread).\n\nI found some problems and so I stopped, but I think it's time to give\nit another look.\n\nMaybe this week-end I'll do some testing.\n\n\nMarco\n"},{"id":"44926","messageId":"e5bfff550706130544m40e2f210p2412ef7fde2867a1@mail.gmail.com","threadId":"8532","inReplyTo":"e5bfff550706130411g151b76e3sa2e351aff5dbb35b@mail.gmail.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-13T12:44:39Z","receivedAt":"2007-06-13T12:44:39Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/13/07, Marco Costalba wrote:\n>\n> Some weeks ago I've started to experiment with git-log as a\n> replacement for git-rev-list (as suggested by Linus in a thread).\n>\n> I found some problems and so I stopped\n\nNow I remember why I've quit with git-log: due to a big performance\nregression under Windows.\n\nI've made some tests on a git tree copied under Windows:\n\nrunning: git rev-list --header --parents --boundary HEAD > tmp.txt\n\n1,38s\n1,37s\n1,39s\n\nthen running: git log --pretty=raw -z --parents --boundary HEAD > tmp.txt\n\n8,87s\n8,87s\n8,86s\n8,86s\n\n\nSo at the end git-log seems 6,4 times slower the git-rev-list !\n\nThis would mean the end of the story for me.\n\n\nMarco\n"},{"id":"44927","messageId":"Pine.LNX.4.64.0706131344430.4059@racer.site","threadId":"8532","inReplyTo":"e5bfff550706130544m40e2f210p2412ef7fde2867a1@mail.gmail.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T12:45:49Z","receivedAt":"2007-06-13T12:45:49Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Marco Costalba wrote:\n\n> I've made some tests on a git tree copied under Windows:\n> \n> running: git rev-list --header --parents --boundary HEAD > tmp.txt\n> \n> [...]\n> \n> then running: git log --pretty=raw -z --parents --boundary HEAD > tmp.txt\n\nIsn't that a bit unfair? You should have passed \"--pretty=raw\" to \nrev-list, too.\n\nSince you already have that tree, could you try rev-list _with_ \n\"--pretty=raw\", just for comparison?\n\nThanks,\nDscho\n"},{"id":"44932","messageId":"e5bfff550706130708k601514c9o1ba2747582027699@mail.gmail.com","threadId":"8532","inReplyTo":"Pine.LNX.4.64.0706131344430.4059@racer.site","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-13T14:08:02Z","receivedAt":"2007-06-13T14:08:02Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/13/07, Johannes Schindelin wrote:\n> >\n> > then running: git log --pretty=raw -z --parents --boundary HEAD > tmp.txt\n>\n> Isn't that a bit unfair? You should have passed \"--pretty=raw\" to\n> rev-list, too.\n>\n> Since you already have that tree, could you try rev-list _with_\n> \"--pretty=raw\", just for comparison?\n>\n\nOf course, here we are\n\nrunning: git rev-list --pretty=raw --parents --boundary HEAD > tmp.txt\n\ntimes are: 1,16s - 1,11s - 1,13s\n\nBTW this performance gap seems to exsist only under Windows, under\nLinux if I remember correctly (cannot test now) the difference is\nsmaller.\n\n\nMarco\n"},{"id":"44943","messageId":"Pine.LNX.4.64.0706131553390.4059@racer.site","threadId":"8532","inReplyTo":"e5bfff550706130708k601514c9o1ba2747582027699@mail.gmail.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T14:58:00Z","receivedAt":"2007-06-13T14:58:00Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Marco Costalba wrote:\n\n> On 6/13/07, Johannes Schindelin wrote:\n> > >\n> > > then running: git log --pretty=raw -z --parents --boundary HEAD > tmp.txt\n> > \n> > Isn't that a bit unfair? You should have passed \"--pretty=raw\" to\n> > rev-list, too.\n> > \n> > Since you already have that tree, could you try rev-list _with_\n> > \"--pretty=raw\", just for comparison?\n> > \n> \n> Of course, here we are\n> \n> running: git rev-list --pretty=raw --parents --boundary HEAD > tmp.txt\n> \n> times are: 1,16s - 1,11s - 1,13s\n\nI just tested on cygwin. The funny thing is, I never get anything like 8 \nseconds (this is on git.git itself).\n\nFor me it is\n\n~1.0s\trev-list _without_ --pretty=raw\n~1.6s\trev-list _with_ --pretty=raw\n~1.4s\tlog _with_ --pretty=raw\n~3.5s\tlog _with_ --pretty=raw _and_ -z (!)\n\nSo, your delay could stem from -z doing weird things.\n\nFor fun, I also did it with the MinGW port:\n\n~0.7s\trev-list _without_ --pretty=raw\n~1.0s\trev-list _with_ --pretty=raw\n~0.8s\tlog _with_ --pretty=raw\n~1.9s\tlog _with_ --pretty=raw _and_ -z (!)\n\nDraw your conclusions, gentlemen.\n\nCiao,\nDscho\n"},{"id":"44959","messageId":"e5bfff550706130918k47b2d58dq73216732c90f65ce@mail.gmail.com","threadId":"8532","inReplyTo":"Pine.LNX.4.64.0706131553390.4059@racer.site","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-13T16:18:49Z","receivedAt":"2007-06-13T16:18:49Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/13/07, Johannes Schindelin wrote:\n> Hi,\n>\n>\n> I just tested on cygwin. The funny thing is, I never get anything like 8\n> seconds (this is on git.git itself).\n>\n> For me it is\n>\n> ~1.0s rev-list _without_ --pretty=raw\n> ~1.6s rev-list _with_ --pretty=raw\n> ~1.4s log _with_ --pretty=raw\n> ~3.5s log _with_ --pretty=raw _and_ -z (!)\n>\n> So, your delay could stem from -z doing weird things.\n>\n\nYes. I can confirm that without -z the speed is more or less the same\nof git-rev-list.\n\nI've tested with cygwin installation of git *.exe files.\n\n>\n> Draw your conclusions, gentlemen.\n>\n\nSomething slows down terribly git-log under Windows when -z option is\ngiven, expecially on cygwin.\n\n\nSome idea, sir?\n\n\nThanks\nMarco\n\nP.S: I really would need a zero terminated record for parsing reasons,\nand git-rev-list prints a zero terminated output too with --header\noption and it works correctly.\n"},{"id":"44962","messageId":"200706131827.08163.Josef.Weidendorfer@gmx.de","threadId":"8532","inReplyTo":"Pine.LNX.4.64.0706131553390.4059@racer.site","subject":"Re: That improved git-gui blame viewer..","fromName":"Josef Weidendorfer","fromEmail":"josef.weidendorfer@gmx.de","sentAt":"2007-06-13T16:27:07Z","receivedAt":"2007-06-13T16:27:07Z","isPatch":false,"sender":{"key":"josef.weidendorfer@gmx.de","avatar":null},"body":"On Wednesday 13 June 2007, Johannes Schindelin wrote:\n> ~1.4s\tlog _with_ --pretty=raw\n> ~3.5s\tlog _with_ --pretty=raw _and_ -z (!)\n\nThat happens on linux, too.\n\nI am not really familiar with that code, but comparision\nof the call graphs of \"git log\" vs. \"git log -z\", as\nproduced by callgrind (shameless plug) and visualized\nby kcachegrind (again, another shameless plug) shows\nthat the difference happens in log_tree_diff, called\nfrom log_tree_commit.\n\nWhile without \"-z\", log_tree_diff immediatly returns\nbecause \"opt->diff\" is 0, in the case of \"-z\",\nthe tree differences are fully done even not used at all ?!\n\nThe result is that \"log_tree_commit\" executes (inclusive\ncallees) 340 million instructions without \"-z\", while\nit runs 2300 million instructions in the \"-z\" case.\n\nJosef\n"},{"id":"44966","messageId":"Pine.LNX.4.64.0706131736260.4059@racer.site","threadId":"8532","inReplyTo":"e5bfff550706130918k47b2d58dq73216732c90f65ce@mail.gmail.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T16:40:15Z","receivedAt":"2007-06-13T16:40:15Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Marco Costalba wrote:\n\n> Something slows down terribly git-log under Windows when -z option is\n> given, expecially on cygwin.\n> \n> \n> Some idea, sir?\n\nNo. Not yet.\n\nCiao,\nDscho\n"},{"id":"44968","messageId":"Pine.LNX.4.64.0706131748400.4059@racer.site","threadId":"8532","inReplyTo":"200706131827.08163.Josef.Weidendorfer@gmx.de","subject":"Re: That improved git-gui blame viewer..","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T16:50:02Z","receivedAt":"2007-06-13T16:50:02Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Josef Weidendorfer wrote:\n\n> On Wednesday 13 June 2007, Johannes Schindelin wrote:\n> > ~1.4s\tlog _with_ --pretty=raw\n> > ~3.5s\tlog _with_ --pretty=raw _and_ -z (!)\n> \n> That happens on linux, too.\n> \n> I am not really familiar with that code, but comparision\n> of the call graphs of \"git log\" vs. \"git log -z\", as\n> produced by callgrind (shameless plug) and visualized\n> by kcachegrind (again, another shameless plug) shows\n> that the difference happens in log_tree_diff, called\n> >from log_tree_commit.\n\nThat's a cool trick!\n\nAnd indeed it is:\n\nThis code in revision.c is responsible:\n\n1181 opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);\n1182 if (opts > 0) {\n1183 revs->diff = 1;\n1184 i += opts - 1;\n1185 continue;\n1186 }\n\nNow, to tell which diff options actually need revs->diff to be set to 1, \nthat is the question.\n\nCiao,\nDscho\n"},{"id":"44969","messageId":"200706131854.59605.Josef.Weidendorfer@gmx.de","threadId":"8532","inReplyTo":"200706131827.08163.Josef.Weidendorfer@gmx.de","subject":"Re: That improved git-gui blame viewer..","fromName":"Josef Weidendorfer","fromEmail":"josef.weidendorfer@gmx.de","sentAt":"2007-06-13T16:54:58Z","receivedAt":"2007-06-13T16:54:58Z","isPatch":false,"sender":{"key":"josef.weidendorfer@gmx.de","avatar":null},"body":"On Wednesday 13 June 2007, Josef Weidendorfer wrote:\n> While without \"-z\", log_tree_diff immediatly returns\n> because \"opt->diff\" is 0, in the case of \"-z\",\n> the tree differences are fully done even not used at all ?!\n\nI wished git-gui and gitk would be better integrated for\nhistory/blame browsing; I also missed a text search functionality\nin the blame view of git-gui.\n\nNeverless...\n\nopt->diff is set to 1 in setup_revisions() whenever\ndiff_opt_parse() parses an option. And \"-z\" is\nparsed in diff_opt_parse().\n\nIn cd2bdc, Linus write in the commit log\n\n - make setup_revision set a flag (revs->diff) if the diff-related\n arguments were used. This allows \"git log\" to decide whether it wants\n to show diffs or not.\n\nSo why is \"-z\" regarded as tree-diff related, leading to calculating diffs?\n\nJosef\n"},{"id":"44971","messageId":"Pine.LNX.4.64.0706131804080.4059@racer.site","threadId":"8532","inReplyTo":"200706131854.59605.Josef.Weidendorfer@gmx.de","subject":"Re: That improved git-gui blame viewer..","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T17:05:55Z","receivedAt":"2007-06-13T17:05:55Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Josef Weidendorfer wrote:\n\n> opt->diff is set to 1 in setup_revisions() whenever\n> diff_opt_parse() parses an option. And \"-z\" is\n> parsed in diff_opt_parse().\n> \n> In cd2bdc, Linus write in the commit log\n> \n> - make setup_revision set a flag (revs->diff) if the diff-related\n> arguments were used. This allows \"git log\" to decide whether it wants\n> to show diffs or not.\n> \n> So why is \"-z\" regarded as tree-diff related, leading to calculating diffs?\n\nBecause it changes the behaviour of a diff. As far as I can tell, the line \ntermination does not have any effect on any output _except_ the diff \noutput.\n\nSo, I should have realized earlier that \"git-log -z\" _without_ anything in \nthe way of \"-p\", \"--stat\" or friends does not make sense at all.\n\nCiao,\nDscho\n"},{"id":"44975","messageId":"7v8xanepto.fsf@assigned-by-dhcp.pobox.com","threadId":"8532","inReplyTo":"200706131854.59605.Josef.Weidendorfer@gmx.de","subject":"Re: That improved git-gui blame viewer..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-13T17:17:55Z","receivedAt":"2007-06-13T17:17:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josef Weidendorfer writes:\n\n> On Wednesday 13 June 2007, Josef Weidendorfer wrote:\n>> While without \"-z\", log_tree_diff immediatly returns\n>> because \"opt->diff\" is 0, in the case of \"-z\",\n>> the tree differences are fully done even not used at all ?!\n>\n> I wished git-gui and gitk would be better integrated for\n> history/blame browsing; I also missed a text search functionality\n> in the blame view of git-gui.\n>\n> Neverless...\n>\n> opt->diff is set to 1 in setup_revisions() whenever\n> diff_opt_parse() parses an option. And \"-z\" is\n> parsed in diff_opt_parse().\n>\n> In cd2bdc, Linus write in the commit log\n>\n> - make setup_revision set a flag (revs->diff) if the diff-related\n> arguments were used. This allows \"git log\" to decide whether it wants\n> to show diffs or not.\n>\n> So why is \"-z\" regarded as tree-diff related, leading to calculating diffs?\n\nBecause of the thinko in that commit. He obviously meant\nthings like --stat, -p and its friends.\n"},{"id":"44981","messageId":"200706132017.31379.Josef.Weidendorfer@gmx.de","threadId":"8532","inReplyTo":"Pine.LNX.4.64.0706131804080.4059@racer.site","subject":"Re: That improved git-gui blame viewer..","fromName":"Josef Weidendorfer","fromEmail":"josef.weidendorfer@gmx.de","sentAt":"2007-06-13T18:17:31Z","receivedAt":"2007-06-13T18:17:31Z","isPatch":false,"sender":{"key":"josef.weidendorfer@gmx.de","avatar":null},"body":"On Wednesday 13 June 2007, Johannes Schindelin wrote:\n> So, I should have realized earlier that \"git-log -z\" _without_ anything in \n> the way of \"-p\", \"--stat\" or friends does not make sense at all.\n\nBut it does make sense: it separates commits using '\\0'.\n\nJosef\n"},{"id":"44984","messageId":"Pine.LNX.4.64.0706131931410.4059@racer.site","threadId":"8532","inReplyTo":"200706132017.31379.Josef.Weidendorfer@gmx.de","subject":"Re: That improved git-gui blame viewer..","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T18:34:50Z","receivedAt":"2007-06-13T18:34:50Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Josef Weidendorfer wrote:\n\n> On Wednesday 13 June 2007, Johannes Schindelin wrote:\n> > So, I should have realized earlier that \"git-log -z\" _without_ anything in \n> > the way of \"-p\", \"--stat\" or friends does not make sense at all.\n> \n> But it does make sense: it separates commits using '\\0'.\n\nIndeed, I missed that. Of course, you could do something as simple as \nthis, although it looks really hacky to me:\n\ndiff --git a/revision.c b/revision.c\nindex b12c25e..1f4590b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1180,7 +1180,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \n \t\t\topts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);\n \t\t\tif (opts > 0) {\n-\t\t\t\trevs->diff = 1;\n+\t\t\t\tif (strcmp(argv[i], \"-z\"))\n+\t\t\t\t\trevs->diff = 1;\n \t\t\t\ti += opts - 1;\n \t\t\t\tcontinue;\n \t\t\t}\n\nCiao,\nDscho\n"},{"id":"45018","messageId":"f4ptq3$71v$1@sea.gmane.org","threadId":"8532","inReplyTo":"7v645thto7.fsf@assigned-by-dhcp.pobox.com","subject":"Re: That improved git-gui blame viewer..","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-06-13T23:20:56Z","receivedAt":"2007-06-13T23:20:56Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> \"Shawn O. Pearce\" writes:\n> \n>> Marco Costalba wrote:\n>>> On 6/12/07, Junio C Hamano wrote:\n>>> >\n>>> > (1) Are you emulating CVS-like \"a file has an identity, and we\n>>> > follow its changes\" model? How does it handle file split,\n>>> > merge, and code movement in general?\n>>> >\n>>> \n>>> It uses 'git rev-list HEAD -- ' to get the list of revisions\n>>> that modified a path,\n>>\n>> So apparently yes, qgit is emulating CVS. And yet much better things\n>> exist (git-blame).\n> \n> I would not use the word \"better\", as it depends on what you are\n> looking for.\n[...]\n> The way Marco describes is a perfectly valid way to satisfy\n> expectations of people migrating from CVS. It's more faithful\n> reproduction of CVS annotate behaviour. In a sense, git-blame\n> does too much, but that is exactly why these \"accurate and\n> interesting\" behaviours are optional.\n\nPerhaps the qgit annotating would find it's way in core git as git-annotate\n(which currently is alias to git-blame + some compatibility options), or\nas an option (--bottom-up) to git-blame?\n\nOn the other hand side, blaming multiple files in parallel has sense I think\nonly for graphical viewer, not for command line command.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"45038","messageId":"20070614051754.GZ6073@spearce.org","threadId":"8532","inReplyTo":"200706131854.59605.Josef.Weidendorfer@gmx.de","subject":"Re: That improved git-gui blame viewer..","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-14T05:17:54Z","receivedAt":"2007-06-14T05:17:54Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Josef Weidendorfer wrote:\n> I wished git-gui and gitk would be better integrated for\n> history/blame browsing;\n\nI'm not sure when/if git-gui and gitk will ever integrate. It\nlooks like it would be a good bit of work. ;-)\n\n> I also missed a text search functionality\n> in the blame view of git-gui.\n\nThat's on my list of things to do very-soon-now. I'm hoping to\nget it into the next release of git-gui, which should be 0.8.0,\nand maybe will happen by early next week.\n\n-- \nShawn.\n"},{"id":"45042","messageId":"e5bfff550706132324p5978852dm4d4cdaf05d01ded8@mail.gmail.com","threadId":"8532","inReplyTo":"f4ptq3$71v$1@sea.gmane.org","subject":"Re: That improved git-gui blame viewer..","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2007-06-14T06:24:40Z","receivedAt":"2007-06-14T06:24:40Z","isPatch":false,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 6/14/07, Jakub Narebski wrote:\n>\n> On the other hand side, blaming multiple files in parallel has sense I think\n> only for graphical viewer, not for command line command.\n>\n\nYes, I agree. The only useful thing for command line is if you use it\ntogheter with some cache mechanism to allow following runs faster.\n\ngit-blame --cache-full-history (?)\n\n\nMarco\n"}]}