[PATCH] gitk: Add horizontal scrollbar to the diff view

[PATCH] gitk: Add horizontal scrollbar to the diff view

[Pekka Kaitaniemi]

Adding horizontal scroll bar makes the scrolling feature more
discoverable to the users.

Signed-off-by: Pekka Kaitaniemi <kaitanie@cc.helsinki.fi>
---
Here is a patch that adds horizontal scroll bar to the gitk diff
pane. It makes the scrolling feature a bit more discoverable and
accessible. The only downside is that it uses some screen real estate.

 gitk |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index f1f21e9..817e0ce 100755
--- a/gitk
+++ b/gitk
@@ -857,14 +857,17 @@ proc makewindow {} {
     set ctext .bleft.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-state disabled -font textfont \
-	-yscrollcommand scrolltext -wrap none
+	-yscrollcommand scrolltext  -wrap none \
+	-xscrollcommand ".bleft.sbhorizontal set"
     if {$have_tk85} {
 	$ctext conf -tabstyle wordprocessor
     }
     scrollbar .bleft.sb -command "$ctext yview"
+    scrollbar .bleft.sbhorizontal -command "$ctext xview" -orient h
     pack .bleft.top -side top -fill x
     pack .bleft.mid -side top -fill x
     pack .bleft.sb -side right -fill y
+    pack .bleft.sbhorizontal -side bottom -fill x -in .bleft
     pack $ctext -side left -fill both -expand 1
     lappend bglist $ctext
     lappend fglist $ctext
-- 
1.5.4.3

Re: [PATCH] gitk: Add horizontal scrollbar to the diff view

[Junio C Hamano]

Pekka Kaitaniemi <kaitanie@cc.helsinki.fi> writes:

> Adding horizontal scroll bar makes the scrolling feature more
> discoverable to the users.
>
> Signed-off-by: Pekka Kaitaniemi <kaitanie@cc.helsinki.fi>
> ---
> Here is a patch that adds horizontal scroll bar to the gitk diff
> pane. It makes the scrolling feature a bit more discoverable and
> accessible. The only downside is that it uses some screen real estate.

An obvious solution is to show the scrollbar on-demand (i.e. when the
lines are overlong), but I do not talk Tcl/Tk and do not know if you can
do that easily.

Re: [PATCH] gitk: Add horizontal scrollbar to the diff view

[Paul Mackerras]

Junio C Hamano writes:

> An obvious solution is to show the scrollbar on-demand (i.e. when the
> lines are overlong), but I do not talk Tcl/Tk and do not know if you can
> do that easily.

I don't know of any extremely easy way to do it; it's certainly
possible, but I think I would have to calculate the length of each
line as it is put in, so as to get the maximum, and then have a
handler for when the pane is resized, and pack and unpack the
scrollbar as necessary.

I think it's reasonable to have the scroll bar there always.  I think
that pane could look better using the grid geometry manager (instead
of pack), but that can be a separate patch.

Paul.

Re: [PATCH] gitk: Add horizontal scrollbar to the diff view

[Pekka Kaitaniemi]

Hi,

On Thu, Mar 06, 2008 at 09:30:42PM +1100, Paul Mackerras wrote:
> Junio C Hamano writes:
> 
> > An obvious solution is to show the scrollbar on-demand (i.e. when the
> > lines are overlong), but I do not talk Tcl/Tk and do not know if you can
> > do that easily.
> 
> I don't know of any extremely easy way to do it; it's certainly
> possible, but I think I would have to calculate the length of each
> line as it is put in, so as to get the maximum, and then have a
> handler for when the pane is resized, and pack and unpack the
> scrollbar as necessary.

I have spent some time today looking for examples on Tk scrollbar
handling (I'm not really a Tcl/Tk guru) and I haven't found any really
easy ways either.

> I think it's reasonable to have the scroll bar there always.  I think
> that pane could look better using the grid geometry manager (instead
> of pack), but that can be a separate patch.

I have modified the patch a bit to make the horizontal scrollbar a bit
narrower so the impact on screen real estate should be smaller
now.

The grid layout manager would probably be a bit better than pack for
the left pane. At least most examples of "text and two scrollbars"
case seem to be using it. Maybe I can try to prepare a patch that
converts the diff pane from pack to grid layout.

An interesting side effect of Tk scrollbars is that by default the
"elevator" size changes depending on the _visible_ content. So the
horizontal scrollbar "elevator" changes as the user scrolls the view
up and down.

Pekka

Re: [PATCH] gitk: Add horizontal scrollbar to the diff view

[Shawn O. Pearce]

Paul Mackerras <paulus@samba.org> wrote:
> Junio C Hamano writes:
> > An obvious solution is to show the scrollbar on-demand (i.e. when the
> > lines are overlong), but I do not talk Tcl/Tk and do not know if you can
> > do that easily.
> 
> I don't know of any extremely easy way to do it; it's certainly
> possible, but I think I would have to calculate the length of each
> line as it is put in, so as to get the maximum, and then have a
> handler for when the pane is resized, and pack and unpack the
> scrollbar as necessary.
> 
> I think it's reasonable to have the scroll bar there always.  I think
> that pane could look better using the grid geometry manager (instead
> of pack), but that can be a separate patch.

git-gui does this scrollbar on-demand thing in its revision list meta-widget,
which is lib/choose_rev.tcl.  The procedure in question is this, and it gets
installed as:

      listbox $w_list \
        -font font_diff \
        -width 50 \
        -height 10 \
        -selectmode browse \
        -exportselection false \
        -xscrollcommand [cb _sb_set $w.list.sbx h] \
        -yscrollcommand [cb _sb_set $w.list.sby v]


method _sb_set {sb orient first last} {
    set old_focus [focus -lastfor $w]
       
    if {$first == 0 && $last == 1} {
        if {[winfo exists $sb]} {
            destroy $sb
            if {$old_focus ne {}} {
                update
                focus $old_focus
            }
        }
        return
    }
       
    if {![winfo exists $sb]} {
        if {$orient eq {h}} {
            scrollbar $sb -orient h -command [list $w_list xview]
            pack $sb -fill x -side bottom -before $w_list
        } else {
            scrollbar $sb -orient v -command [list $w_list yview]
            pack $sb -fill y -side right -before $w_list
        }
        if {$old_focus ne {}} {
            update
            focus $old_focus
        }
    }
       
    catch {$sb set $first $last}
}


-- 
Shawn.

Re: [PATCH] gitk: Add horizontal scrollbar to the diff view

[Paul Mackerras]

Shawn O. Pearce writes:

> git-gui does this scrollbar on-demand thing in its revision list meta-widget,
> which is lib/choose_rev.tcl.  The procedure in question is this, and it gets
> installed as:

That's a nice idea, doing it in _sb_set.  However, since the text
widget adjusts the horizontal scroll bar depending on what's visible,
that would mean the scroll bar would appear and disappear as you
scrolled vertically through the text, which would be disconcerting.
So for a text I think it's better to have the scrollbar there
permanently.

Also, I wonder if you could use pack forget instead of destroying the
scrollbar?

Paul.

Re: [PATCH] gitk: Add horizontal scrollbar to the diff view

[Shawn O. Pearce]

Paul Mackerras <paulus@samba.org> wrote:
> Shawn O. Pearce writes:
> 
> > git-gui does this scrollbar on-demand thing in its revision list meta-widget,
> > which is lib/choose_rev.tcl.  The procedure in question is this, and it gets
> > installed as:
> 
> That's a nice idea, doing it in _sb_set.  However, since the text
> widget adjusts the horizontal scroll bar depending on what's visible,
> that would mean the scroll bar would appear and disappear as you
> scrolled vertically through the text, which would be disconcerting.
> So for a text I think it's better to have the scrollbar there
> permanently.

Yea.  For a list it looks a little funny too, but its better than
not having the scrollbar there at all and looking at a chopped name
that you can't tell apart from another chopped name.  Which was a
bug I suffered from in the early dark ages of git-gui at my day
job.  Imagine Aunt Tillies blindly merging branches because they
cannot see the full branch names.  :-)
 
> Also, I wonder if you could use pack forget instead of destroying the
> scrollbar?

Because I didn't even know about that option to pack.  I'll have to
look into switching the code to do that.  Thanks for the suggestion.

-- 
Shawn.

[PATCH] gitk: Add horizontal scrollbar to the diff view

[Pekka Kaitaniemi]

Adding horizontal scroll bar makes the scrolling feature more
discoverable to the users. The horizontal scrollbar is a bit narrower
than vertical ones so we don't make too big impact on available screen
real estate. The text and scrollbar widget layout is done using grid
geometry manager.

An interesting side effect of Tk scrollbars is that the "elevator"
size changes depending on the visible content. So the horizontal
scrollbar "elevator" changes as the user scrolls the view up and down.

Signed-off-by: Pekka Kaitaniemi <kaitanie@cc.helsinki.fi>
---
A modified version of the earlier patch that adds horizontal
scrollbar. This version uses grid layout for text and scrollbar
widgets. I have tested this patch with Tk 8.4 and 8.5 on Linux.

The width of the horizontal scrollbar (-width 10) seems to be OK with
Tk 8.5 on Linux (the default seems to be about 10 as well). I don't
know what it looks like on Windows and Mac, however.

 gitk |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gitk b/gitk
index f1f21e9..429b091 100755
--- a/gitk
+++ b/gitk
@@ -827,6 +827,7 @@ proc makewindow {} {
     }
     frame .bleft.top
     frame .bleft.mid
+    frame .bleft.bottom
 
     button .bleft.top.search -text [mc "Search"] -command dosearch
     pack .bleft.top.search -side left -padx 5
@@ -854,18 +855,25 @@ proc makewindow {} {
     checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \
 	-command changeignorespace -variable ignorespace
     pack .bleft.mid.ignspace -side left -padx 5
-    set ctext .bleft.ctext
+    set ctext .bleft.bottom.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-state disabled -font textfont \
-	-yscrollcommand scrolltext -wrap none
+	-yscrollcommand scrolltext -wrap none \
+	-xscrollcommand ".bleft.bottom.sbhorizontal set"
     if {$have_tk85} {
 	$ctext conf -tabstyle wordprocessor
     }
-    scrollbar .bleft.sb -command "$ctext yview"
+    scrollbar .bleft.bottom.sb -command "$ctext yview"
+    scrollbar .bleft.bottom.sbhorizontal -command "$ctext xview" -orient h \
+	-width 10
     pack .bleft.top -side top -fill x
     pack .bleft.mid -side top -fill x
-    pack .bleft.sb -side right -fill y
-    pack $ctext -side left -fill both -expand 1
+    grid $ctext .bleft.bottom.sb -sticky nsew
+    grid .bleft.bottom.sbhorizontal -sticky ew
+    grid columnconfigure .bleft.bottom 0 -weight 1
+    grid rowconfigure .bleft.bottom 0 -weight 1
+    grid rowconfigure .bleft.bottom 1 -weight 0
+    pack .bleft.bottom -side top -fill both -expand 1
     lappend bglist $ctext
     lappend fglist $ctext
 
@@ -5604,7 +5612,7 @@ proc searchmarkvisible {doall} {
 proc scrolltext {f0 f1} {
     global searchstring
 
-    .bleft.sb set $f0 $f1
+    .bleft.bottom.sb set $f0 $f1
     if {$searchstring ne {}} {
 	searchmarkvisible 0
     }
-- 
1.5.4.3