patchgitk: Add horizontal scrollbar to the diff view

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(-)

Pekka Kaitaniemi <kaitanie@cc.helsinki.fi> 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.

Junio C Hamano writes:

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.

Hi,

On Thu, Mar 06, 2008 at 09:30:42PM +1100, Paul Mackerras wrote:

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 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

Paul Mackerras <paulus@samba.org> wrote:

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 O. Pearce writes:

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.

Paul Mackerras <paulus@samba.org> wrote:

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.  :-)
 

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.