{"thread":{"id":"12528","subject":"[PATCH] gitk: Add horizontal scrollbar to the diff view","startedAt":"2008-03-05T22:51:23Z","lastAt":"2008-03-08T05:51:57Z","messageCount":7,"participants":["Pekka Kaitaniemi","Junio C Hamano","Paul Mackerras","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"71140","messageId":"20080305225123.GA8485@localdomain","threadId":"12528","inReplyTo":null,"subject":"[PATCH] gitk: Add horizontal scrollbar to the diff view","fromName":"Pekka Kaitaniemi","fromEmail":"kaitanie@cc.helsinki.fi","sentAt":"2008-03-05T22:51:23Z","receivedAt":"2008-03-05T22:51:23Z","isPatch":true,"sender":{"key":"kaitanie@cc.helsinki.fi","avatar":null},"body":"Adding horizontal scroll bar makes the scrolling feature more\ndiscoverable to the users.\n\nSigned-off-by: Pekka Kaitaniemi \n---\nHere is a patch that adds horizontal scroll bar to the gitk diff\npane. It makes the scrolling feature a bit more discoverable and\naccessible. The only downside is that it uses some screen real estate.\n\n gitk | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex f1f21e9..817e0ce 100755\n--- a/gitk\n+++ b/gitk\n@@ -857,14 +857,17 @@ proc makewindow {} {\n set ctext .bleft.ctext\n text $ctext -background $bgcolor -foreground $fgcolor \\\n \t-state disabled -font textfont \\\n-\t-yscrollcommand scrolltext -wrap none\n+\t-yscrollcommand scrolltext -wrap none \\\n+\t-xscrollcommand \".bleft.sbhorizontal set\"\n if {$have_tk85} {\n \t$ctext conf -tabstyle wordprocessor\n }\n scrollbar .bleft.sb -command \"$ctext yview\"\n+ scrollbar .bleft.sbhorizontal -command \"$ctext xview\" -orient h\n pack .bleft.top -side top -fill x\n pack .bleft.mid -side top -fill x\n pack .bleft.sb -side right -fill y\n+ pack .bleft.sbhorizontal -side bottom -fill x -in .bleft\n pack $ctext -side left -fill both -expand 1\n lappend bglist $ctext\n lappend fglist $ctext\n-- \n1.5.4.3\n\n"},{"id":"71197","messageId":"7vy78wgyu1.fsf@gitster.siamese.dyndns.org","threadId":"12528","inReplyTo":"20080305225123.GA8485@localdomain","subject":"Re: [PATCH] gitk: Add horizontal scrollbar to the diff view","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-06T06:18:14Z","receivedAt":"2008-03-06T06:18:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pekka Kaitaniemi writes:\n\n> Adding horizontal scroll bar makes the scrolling feature more\n> discoverable to the users.\n>\n> Signed-off-by: Pekka Kaitaniemi \n> ---\n> Here is a patch that adds horizontal scroll bar to the gitk diff\n> pane. It makes the scrolling feature a bit more discoverable and\n> accessible. The only downside is that it uses some screen real estate.\n\nAn obvious solution is to show the scrollbar on-demand (i.e. when the\nlines are overlong), but I do not talk Tcl/Tk and do not know if you can\ndo that easily.\n\n\n"},{"id":"71217","messageId":"18383.51154.695501.543720@cargo.ozlabs.ibm.com","threadId":"12528","inReplyTo":"7vy78wgyu1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gitk: Add horizontal scrollbar to the diff view","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-03-06T10:30:42Z","receivedAt":"2008-03-06T10:30:42Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> An obvious solution is to show the scrollbar on-demand (i.e. when the\n> lines are overlong), but I do not talk Tcl/Tk and do not know if you can\n> do that easily.\n\nI don't know of any extremely easy way to do it; it's certainly\npossible, but I think I would have to calculate the length of each\nline as it is put in, so as to get the maximum, and then have a\nhandler for when the pane is resized, and pack and unpack the\nscrollbar as necessary.\n\nI think it's reasonable to have the scroll bar there always. I think\nthat pane could look better using the grid geometry manager (instead\nof pack), but that can be a separate patch.\n\nPaul.\n"},{"id":"71239","messageId":"20080306115347.GA11808@localdomain","threadId":"12528","inReplyTo":"18383.51154.695501.543720@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] gitk: Add horizontal scrollbar to the diff view","fromName":"Pekka Kaitaniemi","fromEmail":"kaitanie@cc.helsinki.fi","sentAt":"2008-03-06T11:53:47Z","receivedAt":"2008-03-06T11:53:47Z","isPatch":true,"sender":{"key":"kaitanie@cc.helsinki.fi","avatar":null},"body":"Hi,\n\nOn Thu, Mar 06, 2008 at 09:30:42PM +1100, Paul Mackerras wrote:\n> Junio C Hamano writes:\n> \n> > An obvious solution is to show the scrollbar on-demand (i.e. when the\n> > lines are overlong), but I do not talk Tcl/Tk and do not know if you can\n> > do that easily.\n> \n> I don't know of any extremely easy way to do it; it's certainly\n> possible, but I think I would have to calculate the length of each\n> line as it is put in, so as to get the maximum, and then have a\n> handler for when the pane is resized, and pack and unpack the\n> scrollbar as necessary.\n\nI have spent some time today looking for examples on Tk scrollbar\nhandling (I'm not really a Tcl/Tk guru) and I haven't found any really\neasy ways either.\n\n> I think it's reasonable to have the scroll bar there always. I think\n> that pane could look better using the grid geometry manager (instead\n> of pack), but that can be a separate patch.\n\nI have modified the patch a bit to make the horizontal scrollbar a bit\nnarrower so the impact on screen real estate should be smaller\nnow.\n\nThe grid layout manager would probably be a bit better than pack for\nthe left pane. At least most examples of \"text and two scrollbars\"\ncase seem to be using it. Maybe I can try to prepare a patch that\nconverts the diff pane from pack to grid layout.\n\nAn interesting side effect of Tk scrollbars is that by default the\n\"elevator\" size changes depending on the _visible_ content. So the\nhorizontal scrollbar \"elevator\" changes as the user scrolls the view\nup and down.\n\nPekka\n\n"},{"id":"71395","messageId":"20080308015357.GU8410@spearce.org","threadId":"12528","inReplyTo":"18383.51154.695501.543720@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] gitk: Add horizontal scrollbar to the diff view","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-08T01:53:57Z","receivedAt":"2008-03-08T01:53:57Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Paul Mackerras wrote:\n> Junio C Hamano writes:\n> > An obvious solution is to show the scrollbar on-demand (i.e. when the\n> > lines are overlong), but I do not talk Tcl/Tk and do not know if you can\n> > do that easily.\n> \n> I don't know of any extremely easy way to do it; it's certainly\n> possible, but I think I would have to calculate the length of each\n> line as it is put in, so as to get the maximum, and then have a\n> handler for when the pane is resized, and pack and unpack the\n> scrollbar as necessary.\n> \n> I think it's reasonable to have the scroll bar there always. I think\n> that pane could look better using the grid geometry manager (instead\n> of pack), but that can be a separate patch.\n\ngit-gui does this scrollbar on-demand thing in its revision list meta-widget,\nwhich is lib/choose_rev.tcl. The procedure in question is this, and it gets\ninstalled as:\n\n listbox $w_list \\\n -font font_diff \\\n -width 50 \\\n -height 10 \\\n -selectmode browse \\\n -exportselection false \\\n -xscrollcommand [cb _sb_set $w.list.sbx h] \\\n -yscrollcommand [cb _sb_set $w.list.sby v]\n\n\nmethod _sb_set {sb orient first last} {\n set old_focus [focus -lastfor $w]\n \n if {$first == 0 && $last == 1} {\n if {[winfo exists $sb]} {\n destroy $sb\n if {$old_focus ne {}} {\n update\n focus $old_focus\n }\n }\n return\n }\n \n if {![winfo exists $sb]} {\n if {$orient eq {h}} {\n scrollbar $sb -orient h -command [list $w_list xview]\n pack $sb -fill x -side bottom -before $w_list\n } else {\n scrollbar $sb -orient v -command [list $w_list yview]\n pack $sb -fill y -side right -before $w_list\n }\n if {$old_focus ne {}} {\n update\n focus $old_focus\n }\n }\n \n catch {$sb set $first $last}\n}\n\n\n-- \nShawn.\n"},{"id":"71404","messageId":"18386.9974.50409.525039@cargo.ozlabs.ibm.com","threadId":"12528","inReplyTo":"20080308015357.GU8410@spearce.org","subject":"Re: [PATCH] gitk: Add horizontal scrollbar to the diff view","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2008-03-08T05:41:10Z","receivedAt":"2008-03-08T05:41:10Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Shawn O. Pearce writes:\n\n> git-gui does this scrollbar on-demand thing in its revision list meta-widget,\n> which is lib/choose_rev.tcl. The procedure in question is this, and it gets\n> installed as:\n\nThat's a nice idea, doing it in _sb_set. However, since the text\nwidget adjusts the horizontal scroll bar depending on what's visible,\nthat would mean the scroll bar would appear and disappear as you\nscrolled vertically through the text, which would be disconcerting.\nSo for a text I think it's better to have the scrollbar there\npermanently.\n\nAlso, I wonder if you could use pack forget instead of destroying the\nscrollbar?\n\nPaul.\n"},{"id":"71405","messageId":"20080308055157.GA8410@spearce.org","threadId":"12528","inReplyTo":"18386.9974.50409.525039@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] gitk: Add horizontal scrollbar to the diff view","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-03-08T05:51:57Z","receivedAt":"2008-03-08T05:51:57Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Paul Mackerras wrote:\n> Shawn O. Pearce writes:\n> \n> > git-gui does this scrollbar on-demand thing in its revision list meta-widget,\n> > which is lib/choose_rev.tcl. The procedure in question is this, and it gets\n> > installed as:\n> \n> That's a nice idea, doing it in _sb_set. However, since the text\n> widget adjusts the horizontal scroll bar depending on what's visible,\n> that would mean the scroll bar would appear and disappear as you\n> scrolled vertically through the text, which would be disconcerting.\n> So for a text I think it's better to have the scrollbar there\n> permanently.\n\nYea. For a list it looks a little funny too, but its better than\nnot having the scrollbar there at all and looking at a chopped name\nthat you can't tell apart from another chopped name. Which was a\nbug I suffered from in the early dark ages of git-gui at my day\njob. Imagine Aunt Tillies blindly merging branches because they\ncannot see the full branch names. :-)\n \n> Also, I wonder if you could use pack forget instead of destroying the\n> scrollbar?\n\nBecause I didn't even know about that option to pack. I'll have to\nlook into switching the code to do that. Thanks for the suggestion.\n\n-- \nShawn.\n"}]}