{"thread":{"id":"4029","subject":"gitk highlight feature","startedAt":"2006-05-02T23:44:22Z","lastAt":"2006-05-21T00:40:14Z","messageCount":12,"participants":["Paul Mackerras","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"19390","messageId":"17495.61142.677439.171773@cargo.ozlabs.ibm.com","threadId":"4029","inReplyTo":null,"subject":"gitk highlight feature","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-05-02T23:44:22Z","receivedAt":"2006-05-02T23:44:22Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"I just pushed out some changes to gitk which allow you to use one view\nto highlight another (see the \"Highlight\" submenu under the \"View\"\nmenu), and which allow you to specify arbitrary git-rev-list arguments\nfor a view.  The arguments string uses shell quoting conventions.\n\nI had been thinking of having fields in the view editor dialog where\nyou could put in refs that you did and didn't want included, date\nspecifiers, etc., all in separate fields with suitable labels.  Now\nI'm thinking that it's probably just as convenient to put\n\"ORIG_HEAD..\" into the git-rev-list arguments field as it is to put\n\"ORIG_HEAD\" in the \"Don't include commits reachable from this\" field.\nThere may be an argument for having fields for \"Exclude commits before\nthis date\" and \"Exclude commits after this date\", because those things\noften have spaces in them (e.g. \"2 weeks ago\") which would have to be\nquoted in the git-rev-list arguments field.\n\nThoughts?\n\nPaul.\n"},{"id":"19391","messageId":"7vejzcj8da.fsf@assigned-by-dhcp.cox.net","threadId":"4029","inReplyTo":"17495.61142.677439.171773@cargo.ozlabs.ibm.com","subject":"Re: gitk highlight feature","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-02T23:48:49Z","receivedAt":"2006-05-02T23:48:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras <paulus@samba.org> writes:\n\n> I just pushed out some changes to gitk which allow you to use one view\n> to highlight another (see the \"Highlight\" submenu under the \"View\"\n> menu), and which allow you to specify arbitrary git-rev-list arguments\n> for a view.  The arguments string uses shell quoting conventions.\n\nEh, the string entered by me is quoted by the program, or do I\nhave to quote it myself?  I suspect it should not be so bad to\ncode, even if you have to do it with tcl ;-).\n\n> I had been thinking of having fields in the view editor dialog where\n> you could put in refs that you did and didn't want included, date\n> specifiers, etc., all in separate fields with suitable labels.  Now\n> I'm thinking that it's probably just as convenient to put\n> \"ORIG_HEAD..\" into the git-rev-list arguments field as it is to put\n> \"ORIG_HEAD\" in the \"Don't include commits reachable from this\" field.\n> There may be an argument for having fields for \"Exclude commits before\n> this date\" and \"Exclude commits after this date\", because those things\n> often have spaces in them (e.g. \"2 weeks ago\") which would have to be\n> quoted in the git-rev-list arguments field.\n>\n> Thoughts?\n\nCalendar widgets.  BTW, \"rev-list --since=2.days.ago\" would work\nrather well ;-).\n"},{"id":"19394","messageId":"Pine.LNX.4.64.0605021659430.4086@g5.osdl.org","threadId":"4029","inReplyTo":"17495.61142.677439.171773@cargo.ozlabs.ibm.com","subject":"Re: gitk highlight feature","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-03T00:07:23Z","receivedAt":"2006-05-03T00:07:23Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 3 May 2006, Paul Mackerras wrote:\n>\n> I just pushed out some changes to gitk which allow you to use one view\n> to highlight another (see the \"Highlight\" submenu under the \"View\"\n> menu), and which allow you to specify arbitrary git-rev-list arguments\n> for a view.  The arguments string uses shell quoting conventions.\n\nOk. It looks interesting. I don't have a particular issue to try it with, \nbut the bold-face certainly stands out enough that it was easy to ask for \na high-light of any commits that changed the kernel/ subdirectory, and it \nwas visually very obvious.\n\nThe interface does feel a bit awkward, though. Separating out \"view\" and \n\"highlight\" into two separate things seems wrong. Wouldn't it be better to \njust have multiple views in the \"view\" menu, and just a way to mark one or \nmore of them as \"highliht views\". \n\nIe not a separate menu, but having the current \"view\" radio buttons be \nmore flexible. \n\n> I had been thinking of having fields in the view editor dialog where\n> you could put in refs that you did and didn't want included, date\n> specifiers, etc., all in separate fields with suitable labels.  Now\n> I'm thinking that it's probably just as convenient to put\n> \"ORIG_HEAD..\" into the git-rev-list arguments field as it is to put\n> \"ORIG_HEAD\" in the \"Don't include commits reachable from this\" field.\n\nYeah. I think it's easier with a single thing, just let people stick it \nthere.\n\n> There may be an argument for having fields for \"Exclude commits before\n> this date\" and \"Exclude commits after this date\", because those things\n> often have spaces in them (e.g. \"2 weeks ago\") which would have to be\n> quoted in the git-rev-list arguments field.\n\nI alwaus use \"2.weeks.ago\" instead, but I guess you could do a calendar \nwidget or something.\n\n\t\tLinus\n"},{"id":"19395","messageId":"17495.63101.163740.649150@cargo.ozlabs.ibm.com","threadId":"4029","inReplyTo":"7vejzcj8da.fsf@assigned-by-dhcp.cox.net","subject":"Re: gitk highlight feature","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-05-03T00:17:01Z","receivedAt":"2006-05-03T00:17:01Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> Eh, the string entered by me is quoted by the program, or do I\n> have to quote it myself?  I suspect it should not be so bad to\n> code, even if you have to do it with tcl ;-).\n\nIt's split up into arguments at whitespace unless the whitespace is\nquoted, just like the shell does.  I wrote tcl code to do the\nsplitting and quoting according to the shell rules.  So you can, for\nexample, put:\n\n\t--unpacked --since='2 days ago'\n\nin there and it will pass two arguments to git-rev-list, the second\ncontaining the embedded spaces (but without the single-quotes).\n\nPaul.\n"},{"id":"19396","messageId":"Pine.LNX.4.64.0605021721540.4086@g5.osdl.org","threadId":"4029","inReplyTo":"Pine.LNX.4.64.0605021659430.4086@g5.osdl.org","subject":"Re: gitk highlight feature","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-03T00:23:00Z","receivedAt":"2006-05-03T00:23:00Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 2 May 2006, Linus Torvalds wrote:\n> \n> Ie not a separate menu, but having the current \"view\" radio buttons be \n> more flexible. \n\nThe obvious way to do it would be to just have two buttons per view: one \nexclusive one (for the main view - only one selected at a time), and the \nother one for the \"highlight\" one where you could allow multiple views to \nbe selected for highlighting.\n\n\t\tLinus\n"},{"id":"19398","messageId":"17496.7073.507895.484698@cargo.ozlabs.ibm.com","threadId":"4029","inReplyTo":"Pine.LNX.4.64.0605021721540.4086@g5.osdl.org","subject":"Re: gitk highlight feature","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-05-03T02:55:29Z","receivedAt":"2006-05-03T02:55:29Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> The obvious way to do it would be to just have two buttons per view: one \n> exclusive one (for the main view - only one selected at a time), and the \n> other one for the \"highlight\" one where you could allow multiple views to \n> be selected for highlighting.\n\nThat's hard to do in a menu, but I could do it in a separate pane.\nOr, I could draw a series of tabs between the menu bar and the\ngraph/headline/author/date panes.  Each tab would have the view name,\na radiobutton to select it for highlighting, and a close button.\n\nPaul.\n"},{"id":"19448","messageId":"Pine.LNX.4.64.0605030946260.4086@g5.osdl.org","threadId":"4029","inReplyTo":"17496.7073.507895.484698@cargo.ozlabs.ibm.com","subject":"Re: gitk highlight feature","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-03T16:57:51Z","receivedAt":"2006-05-03T16:57:51Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 3 May 2006, Paul Mackerras wrote:\n>\n> Linus Torvalds writes:\n> \n> > The obvious way to do it would be to just have two buttons per view: one \n> > exclusive one (for the main view - only one selected at a time), and the \n> > other one for the \"highlight\" one where you could allow multiple views to \n> > be selected for highlighting.\n> \n> That's hard to do in a menu, but I could do it in a separate pane.\n> Or, I could draw a series of tabs between the menu bar and the\n> graph/headline/author/date panes.  Each tab would have the view name,\n> a radiobutton to select it for highlighting, and a close button.\n\nActually, having used this thing (and thought about) some more, I'm afraid \nthat I'll have to just admit that my whole \"highlight\" thing was \nmisdesigned.\n\nOne is just an interface issue, and I think that it would be fixed by just \nremoving the highlight entry from the \"view\" menu, and making it a menu of \nits own. That, I think, would fix the user interface to be more obvious \nand intuitive.\n\nBut the real thing I found is that when I decided I wanted to highlight, I \ndidn't actually want to highlight by \"git-rev-list\" at all. At least not \nmost of the time.\n\nSo far, what I've wanted to highlight by is:\n\n - \"does it touch this file/directory/pathspec\"\n\n   This is _close_ to \"git-rev-list\", and you can (and do) actually \n   implement it as that, but it's stupid to do it that way. You just spend \n   extra time. It's literally much better to do\n\n\tcat commit-list | git-diff-tree -s --stdin -- <pathspec>\n\n   which is a hell of a lot more efficient, since you already have the \n   commit-list you're interested in (and, in fact, this allows you to do \n   things efficiently only for the current _visible_ commits, if you want \n   to, which might be an important optimization for large views).\n\n - \"Does the author/committer match xyz*\"\n\n   I ended up using the \"search\" button for this, and it worked, but the \n   highlight feature would just have done it much better. Especially if \n   there was a way to do \"go to next highlight\", instead of just \"go to \n   next commit\"\n\n   Again, this is actually most easily done by just using the commit-list \n   you already _have_. Not with adding a new argument to \"git-rev-list\" \n   and trying to filter two views. Also, again, this can actually be done \n   for just the \"visible\" ones, not the whole view.\n\n - \"highlight the commits [not] reachable from the selected commit\"\n\n   Now, this literally is \"git-rev-list\", but it's a variation on it: you \n   don't want a new view, you really want the _old_ view (exact same \n   git-rev-list arguments), but you add \"^commit\" to the list of \n   revisions. And then you (conditionally) invert the highlighting if you \n   wanted \"reachable\" rather than \"not reachable\".\n\nAnd the current \"create a new view, and overlay that on the old one\" \napproach really isn't that good. My mistake, it was me who suggested it.\n\nWhat do you think? \n\n\t\tLinus\n"},{"id":"19479","messageId":"17497.14311.121872.249120@cargo.ozlabs.ibm.com","threadId":"4029","inReplyTo":"Pine.LNX.4.64.0605030946260.4086@g5.osdl.org","subject":"Re: gitk highlight feature","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-05-03T23:08:23Z","receivedAt":"2006-05-03T23:08:23Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> So far, what I've wanted to highlight by is:\n> \n>  - \"does it touch this file/directory/pathspec\"\n> \n>    This is _close_ to \"git-rev-list\", and you can (and do) actually \n>    implement it as that, but it's stupid to do it that way. You just spend \n>    extra time. It's literally much better to do\n> \n> \tcat commit-list | git-diff-tree -s --stdin -- <pathspec>\n> \n>    which is a hell of a lot more efficient, since you already have the \n\nOn my G5:\n\n$ git-rev-list HEAD >all-revs\n$ time git-rev-list HEAD -- arch/powerpc >ppc-revs\nreal    0m2.463s\nuser    0m2.265s\nsys     0m0.191s\n$ time git-diff-tree -s --stdin -- arch/powerpc <all-revs >ppc-revs2\nreal    0m5.269s\nuser    0m4.794s\nsys     0m0.462s\n$ wc ppc-revs*\n   892    892  36572 ppc-revs\n  5875   5875 240875 ppc-revs2\n\nWhy does git-diff-tree -s --stdin produce so many more revisions than\ngit-rev-list?  The git-diff-tree output includes commits such as\n6ba815de, which only affects arch/i386/kernel/timers/timer_tsc.c.\nConfused.\n\n>    commit-list you're interested in (and, in fact, this allows you to do \n>    things efficiently only for the current _visible_ commits, if you want \n>    to, which might be an important optimization for large views).\n\nI'd have to try it.  The overhead of fork/exec/process startup might\nbe too much to do every time the user scrolls the window, though.\nMaybe what we need is some way to have git-diff-tree run as a helper\nprocess where I can write some commits to it, then write something\nthat says \"that's all for now\" and have it finish what it's doing,\nwrite some \"end of output\" indicator and flush its output buffers -\nand still have the process and the pipes to/from it available for\nanother round later.\n\n>  - \"Does the author/committer match xyz*\"\n> \n>    I ended up using the \"search\" button for this, and it worked, but the \n>    highlight feature would just have done it much better. Especially if \n>    there was a way to do \"go to next highlight\", instead of just \"go to \n>    next commit\"\n\nThe \"/\" and \"?\" keys go to the next and previous find hit.  It sounds\nlike the search function is actually what you want for this, since it\ndoes highlight the commits that match, but maybe the problem is that\nthe yellow highlighting tends to go away too easily.\n\nHow about a way to do a search and use the results to create a\nbold highlight, which will persist even if you do another search?\nEither a separate \"find & highlight\" button, or a way to transfer the\ncurrent find hits to the highlight?\n\nAs for \"go to next/previous highlight\", that sounds useful.  Rusty\nwants \"go to next/previous commit that is a child/parent of this one\"\nand \"go to next/previous commit that is a descendent/ancestor of this\none and has a ref (tag, head, etc.)\".\n\nHow about shift-up and shift-down for \"go to previous/next highlight\"?\n\nPaul.\n"},{"id":"19481","messageId":"Pine.LNX.4.64.0605031612230.4086@g5.osdl.org","threadId":"4029","inReplyTo":"17497.14311.121872.249120@cargo.ozlabs.ibm.com","subject":"Re: gitk highlight feature","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-03T23:25:57Z","receivedAt":"2006-05-03T23:25:57Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 4 May 2006, Paul Mackerras wrote:\n> \n> On my G5:\n> \n> $ git-rev-list HEAD >all-revs\n> $ time git-rev-list HEAD -- arch/powerpc >ppc-revs\n> real    0m2.463s\n> user    0m2.265s\n> sys     0m0.191s\n> $ time git-diff-tree -s --stdin -- arch/powerpc <all-revs >ppc-revs2\n> real    0m5.269s\n> user    0m4.794s\n> sys     0m0.462s\n\nRight. One of them prunes the history (git-rev-list). The other one does \nnot.\n\nThat said, you need an \"-r\", I guess, to \"git-diff-tree\".\n\n> Why does git-diff-tree -s --stdin produce so many more revisions than\n> git-rev-list?\n\nI guess it's officially a FAQ by now: see the \"bug in git log\" thread the \nother day, and\n\n        http://www.gelato.unsw.edu.au/archives/git/0604/19180.html\n\nso in general you can get _more_ of the \"real commits\" with \"git-rev-list \n| git-diff-tree --stdin\", because it won't prune out history on \nuninteresting branches. At the same time, that effect is counter-acted by \ngit-diff-tree normally ignoring merges by default, which is a bigger \nissue.\n\nYour big problem is just the lack of \"-r\", though:\n\n> The git-diff-tree output includes commits such as\n> 6ba815de, which only affects arch/i386/kernel/timers/timer_tsc.c.\n> Confused.\n\nWithout the -r, git-diff-tree won't actually recurse, so it hits the \n\"arch/\" part (which does differ, and matches te revspec), and decides it's \ndone.\n\nWith the \"-r\" thing, you still have a noticeable difference, but now it's \ndue to the difference between \"log\" and \"diff\":\n\n   git-rev-list HEAD -- arch/powerpc | wc -l\n    -> 892\n   git-rev-list HEAD | git-diff-tree -r -s --stdin -- arch/powerpc/ | wc -l\n    -> 838\n\nie \"diff\" doesn't show merges nomally (with -m, you _will_ get merges, but \nyou'll sometimes get them twice - once against each parent ;)\n\n\t\tLinus\n"},{"id":"20310","messageId":"17518.24000.952384.563448@cargo.ozlabs.ibm.com","threadId":"4029","inReplyTo":"Pine.LNX.4.64.0605030946260.4086@g5.osdl.org","subject":"Re: gitk highlight feature","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-05-20T00:07:28Z","receivedAt":"2006-05-20T00:07:28Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> But the real thing I found is that when I decided I wanted to highlight, I \n> didn't actually want to highlight by \"git-rev-list\" at all. At least not \n> most of the time.\n\nI just pushed some changes to the \"new\" branch of the gitk.git\nrepository which change the way we do highlighting.  There is now a\nrow of controls across the middle of the window, just below the row\ncontaining the sha1 ID, \"Find\" button, etc., which controls the\nhighlighting.  There are (currently) three ways to do highlighting: by\npath, by view, and by author/committer.  The author/committer matching\nis case-insensitive (since I'm clearly an insensitive sort of guy :)\nand matches any of the given strings anywhere in the author and\ncommitter fields.  The path and author/committer entry widgets take a\nwhitespace-delimited list of paths or names of interest, using shell\nquoting rules, so you can put for example:\n\n\"david s. miller\" benh\n\nin there and you'll get commits from either davem or benh highlighted.\n\nDo people think this is useful and on the right track interface-wise?\n\nPaul.\n"},{"id":"20326","messageId":"Pine.LNX.4.64.0605200934240.10823@g5.osdl.org","threadId":"4029","inReplyTo":"17518.24000.952384.563448@cargo.ozlabs.ibm.com","subject":"Re: gitk highlight feature","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-20T16:42:30Z","receivedAt":"2006-05-20T16:42:30Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 20 May 2006, Paul Mackerras wrote:\n> \n> I just pushed some changes to the \"new\" branch of the gitk.git\n> repository which change the way we do highlighting.  There is now a\n> row of controls across the middle of the window, just below the row\n> containing the sha1 ID, \"Find\" button, etc., which controls the\n> highlighting.\n\nOk, this is much nicer, I think this ends up being much closer to what I \nreally wanted, even if I didn't know quite how I wanted it.\n\nI think the \"Find\" field should highlight things too. Right now there's no \nway to get highlighting for somebody having signed-off on a patch, for \nexample, even though you can _search_ for it.\n\nAlso, right now it says \"Author/committer\", but it actually only triggers \non author. Which may be the right thing to do (it's certainly what I'd \nnormally want to see), but it doesn't match the description. \n\n\t\tLinus\n"},{"id":"20357","messageId":"17519.46830.626777.438845@cargo.ozlabs.ibm.com","threadId":"4029","inReplyTo":"Pine.LNX.4.64.0605200934240.10823@g5.osdl.org","subject":"Re: gitk highlight feature","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2006-05-21T00:40:14Z","receivedAt":"2006-05-21T00:40:14Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> I think the \"Find\" field should highlight things too. Right now there's no \n> way to get highlighting for somebody having signed-off on a patch, for \n> example, even though you can _search_ for it.\n\nYes, I think the way to go is to extend the highlight feature to be\nable to do everything that the \"Find\" function can do, and then I\nthink the \"Find\" function won't be needed any more.\n\n> Also, right now it says \"Author/committer\", but it actually only triggers \n> on author. Which may be the right thing to do (it's certainly what I'd \n> normally want to see), but it doesn't match the description. \n\nIf the author matches, it highlights both the headline and the\nauthor.  If the committer matches, it highlights only the headline (as\nit does for any other highlighting).  Try it with \"torvalds\" in the\nauthor/committer field on commit a54c9d30 (compared to cb46c370,\nsay).  If that's confusing I can change it of course.\n\nPaul.\n"}]}