threads / patch / 15766

patchgit-gui: Fix the blame viewer destroy handler.

Subject: [PATCH] git-gui: Fix the blame viewer destroy handler.

## tl;dr

4 messages between Oct 3, 2008 and Oct 3, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Alexander Gavrilov· Oct 3, 2008, 07:36 UTC · lore

[PATCH] git-gui: Fix the blame window shape.

On modern high-resolution monitors the blame viewer window is very high, yet too narrow. This patch makes it gravitate to a more sane resolution, which takes the font size into account.

It also changes the default text view size to 80% of the window, and slightly modifies the border decorations for better appearance.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 lib/blame.tcl |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)
Show changes to lib/blame.tcl +13 −6
diff --git a/lib/blame.tcl b/lib/blame.tcl
index eb61374..221313c 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -69,6 +69,8 @@ constructor new {i_commit i_path i_jump} {
 	make_toplevel top w
 	wm title $top [append "[appname] ([reponame]): " [mc "File Viewer"]]
 
+	set font_w [font measure font_diff "0"]
+
 	frame $w.header -background gold
 	label $w.header.commit_l \
 		-text [mc "Commit:"] \
@@ -114,9 +116,9 @@ constructor new {i_commit i_path i_jump} {
 	pack $w_path -fill x -side right
 	pack $w.header.path_l -side right
 
-	panedwindow $w.file_pane -orient vertical
-	frame $w.file_pane.out
-	frame $w.file_pane.cm
+	panedwindow $w.file_pane -orient vertical -borderwidth 0 -sashwidth 3
+	frame $w.file_pane.out -relief flat -borderwidth 1
+	frame $w.file_pane.cm -relief sunken -borderwidth 1
 	$w.file_pane add $w.file_pane.out \
 		-sticky nsew \
 		-minsize 100 \
@@ -328,9 +330,14 @@ constructor new {i_commit i_path i_jump} {
 
 	set req_w [winfo reqwidth  $top]
 	set req_h [winfo reqheight $top]
-	set scr_h [expr {[winfo screenheight $top] - 100}]
-	if {$req_w < 600} {set req_w 600}
+	set scr_w [expr {[winfo screenwidth $top] - 40}]
+	set scr_h [expr {[winfo screenheight $top] - 120}]
+	set opt_w [expr {$font_w * (80 + 5*3 + 3)}]
+	if {$req_w < $opt_w} {set req_w $opt_w}
+	if {$req_w > $scr_w} {set req_w $scr_w}
+	set opt_h [expr {$req_w*4/3}]
 	if {$req_h < $scr_h} {set req_h $scr_h}
+	if {$req_h > $opt_h} {set req_h $opt_h}
 	set g "${req_w}x${req_h}"
 	wm geometry $top $g
 	update
@@ -338,7 +345,7 @@ constructor new {i_commit i_path i_jump} {
 	set old_height [winfo height $w.file_pane]
 	$w.file_pane sash place 0 \
 		[lindex [$w.file_pane sash coord 0] 0] \
-		[expr {int($old_height * 0.70)}]
+		[expr {int($old_height * 0.80)}]
 	bind $w.file_pane <Configure> \
 	"if {{$w.file_pane} eq {%W}} {[cb _resize %h]}"
 
-- 
1.6.0.20.g6148bc
Alexander Gavrilov· Oct 3, 2008, 07:36 UTC · re: Alexander Gavrilov · lore

[PATCH] git-gui: Add a search command to the blame viewer.

One of the largest deficiencies in the blame viewer at the moment is the impossibility to search for a text string. This commit fixes it by adding a Firefox-like search panel to the viewer.

The panel can be shown by pressing F7 or clicking a menu entry, and is hidden by pressing Esc. Find Next is available through the F3 key.

Implementation is based on the gitk code, but heavily refactored. It now also supports case-insensitive searches, and uses the text box background color to signal success or failure of the search.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
	I would have used Ctrl-F, but it is already occupied.
	
	-- Alexander
 git-gui.sh     |    3 +
 lib/blame.tcl  |   37 ++++++++++-
 lib/search.tcl |  190 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 227 insertions(+), 3 deletions(-)
 create mode 100644 lib/search.tcl
Show changes to 3 files +227 −3

git-gui.sh, lib/blame.tcl, lib/search.tcl

diff --git a/git-gui.sh b/git-gui.sh
index 79a108d..4f95139 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -591,6 +591,7 @@ bind . <Visibility> {
 
 if {[is_Windows]} {
 	wm iconbitmap . -default $oguilib/git-gui.ico
+	set ::tk::AlwaysShowSelection 1
 }
 
 ######################################################################
@@ -1067,6 +1068,8 @@ set selected_commit_type new
 set nullid "0000000000000000000000000000000000000000"
 set nullid2 "0000000000000000000000000000000000000001"
 
+set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
+
 ######################################################################
 ##
 ## task management
diff --git a/lib/blame.tcl b/lib/blame.tcl
index 221313c..a45784c 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -21,9 +21,11 @@ field w_amov     ; # text column: annotations + move tracking
 field w_asim     ; # text column: annotations (simple computation)
 field w_file     ; # text column: actual file data
 field w_cviewer  ; # pane showing commit message
+field finder     ; # find mini-dialog frame
 field status     ; # status mega-widget instance
 field old_height ; # last known height of $w.file_pane
 
+
 # Tk UI colors
 #
 variable active_color #c0edc5
@@ -59,7 +61,7 @@ field tooltip_timer     {} ; # Current timer event for our tooltip
 field tooltip_commit    {} ; # Commit(s) in tooltip
 
 constructor new {i_commit i_path i_jump} {
-	global cursor_ptr
+	global cursor_ptr M1B M1T have_tk85
 	variable active_color
 	variable group_colors
 
@@ -199,6 +201,11 @@ constructor new {i_commit i_path i_jump} {
 		-width 80 \
 		-xscrollcommand [list $w.file_pane.out.sbx set] \
 		-font font_diff
+	if {$have_tk85} {
+		$w_file configure -inactiveselectbackground darkblue
+	}
+	$w_file tag conf found \
+		-background yellow
 
 	set w_columns [list $w_amov $w_asim $w_line $w_file]
 
@@ -219,6 +226,11 @@ constructor new {i_commit i_path i_jump} {
 		-weight 1
 	grid rowconfigure $w.file_pane.out 0 -weight 1
 
+	set finder [::searchbar::new \
+		$w.file_pane.out.ff $w_file \
+		-column [expr {[llength $w_columns] - 1}] \
+		]
+
 	set w_cviewer $w.file_pane.cm.t
 	text $w_cviewer \
 		-background white \
@@ -259,6 +271,10 @@ constructor new {i_commit i_path i_jump} {
 		-label [mc "Copy Commit"] \
 		-command [cb _copycommit]
 	$w.ctxm add separator
+	$w.ctxm add command \
+		-label [mc "Find Text..."] \
+		-accelerator F7 \
+		-command [list searchbar::show $finder]
 	menu $w.ctxm.enc
 	build_encoding_menu $w.ctxm.enc [cb _setencoding]
 	$w.ctxm add cascade \
@@ -280,9 +296,15 @@ constructor new {i_commit i_path i_jump} {
 			$i tag conf color$g -background [lindex $group_colors $g]
 		}
 
+		if {$i eq $w_file} {
+			$w_file tag raise found
+		}
+		$i tag raise sel
+
 		$i conf -cursor $cursor_ptr
-		$i conf -yscrollcommand [list many2scrollbar \
-			$w_columns yview $w.file_pane.out.sby]
+		$i conf -yscrollcommand \
+			"[list ::searchbar::scrolled $finder]
+			 [list many2scrollbar $w_columns yview $w.file_pane.out.sby]"
 		bind $i <Button-1> "
 			[cb _hide_tooltip]
 			[cb _click $i @%x,%y]
@@ -319,6 +341,11 @@ constructor new {i_commit i_path i_jump} {
 	bind $w_cviewer <Tab>       "[list focus $w_file];break"
 	bind $w_cviewer <Button-1> [list focus $w_cviewer]
 	bind $w_file    <Visibility> [list focus $w_file]
+	bind $top       <F7>         [list searchbar::show $finder]
+	bind $top       <Escape>     [list searchbar::hide $finder]
+	bind $top       <F3>         [list searchbar::find_next $finder]
+	bind $top       <Shift-F3>   [list searchbar::find_prev $finder]
+	catch { bind $top <Shift-Key-XF86_Switch_VT_3> [list searchbar::find_prev $finder] }
 
 	grid configure $w.header -sticky ew
 	grid configure $w.file_pane -sticky nsew
@@ -873,6 +900,10 @@ method _showcommit {cur_w lno} {
 		foreach i $w_columns {
 			$i tag conf g$cmit -background $active_color
 			$i tag raise g$cmit
+			if {$i eq $w_file} {
+				$w_file tag raise found
+			}
+			$i tag raise sel
 		}
 
 		set author_name {}
diff --git a/lib/search.tcl b/lib/search.tcl
new file mode 100644
index 0000000..d292f20
--- /dev/null
+++ b/lib/search.tcl
@@ -0,0 +1,190 @@
+# incremental search panel
+# based on code from gitk, Copyright (C) Paul Mackerras
+
+class searchbar {
+
+field w
+field ctext
+
+field searchstring   {}
+field casesensitive  1
+field searchdirn     -forwards
+
+field smarktop
+field smarkbot
+
+constructor new {i_w i_text args} {
+	set w      $i_w
+	set ctext  $i_text
+
+	frame  $w
+	label  $w.l       -text [mc Find:]
+	button $w.bn      -text [mc Next] -command [cb find_next]
+	button $w.bp      -text [mc Prev] -command [cb find_prev]
+	checkbutton $w.cs -text [mc Case-Sensitive] \
+		-variable ${__this}::casesensitive -command [cb _incrsearch]
+	entry  $w.ent -textvariable ${__this}::searchstring -background lightgreen
+	pack   $w.l   -side left
+	pack   $w.cs  -side right
+	pack   $w.bp  -side right
+	pack   $w.bn  -side right
+	pack   $w.ent -side left -expand 1 -fill x
+
+	eval grid conf $w -sticky we $args
+	grid remove $w
+
+	trace add variable searchstring write [cb _incrsearch_cb]
+	
+	bind $w <Destroy> [cb delete_this]
+	return $this
+}
+
+method show {} {
+	if {![winfo ismapped $w]} {
+		grid $w
+	}
+	focus -force $w.ent
+}
+
+method hide {} {
+	if {[winfo ismapped $w]} {
+		focus $ctext
+		grid remove $w
+	}
+}
+
+method _get_new_anchor {} {
+	# use start of selection if it is visible,
+	# or the bounds of the visible area
+	set top    [$ctext index @0,0]
+	set bottom [$ctext index @0,[winfo height $ctext]]
+	set sel    [$ctext tag ranges sel]
+	if {$sel ne {}} {
+		set spos [lindex $sel 0]
+		if {[lindex $spos 0] >= [lindex $top 0] &&
+		    [lindex $spos 0] <= [lindex $bottom 0]} {
+			return $spos
+		}
+	}
+	if {$searchdirn eq "-forwards"} {
+		return $top
+	} else {
+		return $bottom
+	}
+}
+
+method _get_wrap_anchor {dir} {
+	if {$dir eq "-forwards"} {
+		return 1.0
+	} else {
+		return end
+	}
+}
+
+method _do_search {start {mlenvar {}} {dir {}} {endbound {}}} {
+	set cmd [list $ctext search]
+	if {$mlenvar ne {}} {
+		upvar $mlenvar mlen
+		lappend cmd -count mlen
+	}
+	if {!$casesensitive} {
+		lappend cmd -nocase
+	}
+	if {$dir eq {}} {
+		set dir $searchdirn
+	}
+	lappend cmd $dir -- $searchstring
+	if {$endbound ne {}} {
+		set here [eval $cmd [list $start] [list $endbound]]
+	} else {
+		set here [eval $cmd [list $start]]
+		if {$here eq {}} {
+			set here [eval $cmd [_get_wrap_anchor $this $dir]]
+		}
+	}
+	return $here
+}
+
+method _incrsearch_cb {name ix op} {
+	after idle [cb _incrsearch]
+}
+
+method _incrsearch {} {
+	$ctext tag remove found 1.0 end
+	if {[catch {$ctext index anchor}]} {
+		$ctext mark set anchor [_get_new_anchor $this]
+	}
+	if {$searchstring ne {}} {
+		set here [_do_search $this anchor mlen]
+		if {$here ne {}} {
+			$ctext see $here
+			$ctext tag remove sel 1.0 end
+			$ctext tag add sel $here "$here + $mlen c"
+			$w.ent configure -background lightgreen
+			_set_marks $this 1
+		} else {
+			$w.ent configure -background lightpink
+		}
+	}
+}
+
+method find_prev {} {
+	find_next $this -backwards
+}
+
+method find_next {{dir -forwards}} {
+	focus $w.ent
+	$w.ent icursor end
+	set searchdirn $dir
+	$ctext mark unset anchor
+	if {$searchstring ne {}} {
+		set start [_get_new_anchor $this]
+		if {$dir eq "-forwards"} {
+			set start "$start + 1c"
+		}
+		set match [_do_search $this $start mlen]
+		$ctext tag remove sel 1.0 end
+		if {$match ne {}} {
+			$ctext see $match
+			$ctext tag add sel $match "$match + $mlen c"
+		}
+	}
+}
+
+method _mark_range {first last} {
+	set mend $first.0
+	while {1} {
+		set match [_do_search $this $mend mlen -forwards $last.end]
+		if {$match eq {}} break
+		set mend "$match + $mlen c"
+		$ctext tag add found $match $mend
+	}
+}
+
+method _set_marks {doall} {
+	set topline [lindex [split [$ctext index @0,0] .] 0]
+	set botline [lindex [split [$ctext index @0,[winfo height $ctext]] .] 0]
+	if {$doall || $botline < $smarktop || $topline > $smarkbot} {
+		# no overlap with previous
+		_mark_range $this $topline $botline
+		set smarktop $topline
+		set smarkbot $botline
+	} else {
+		if {$topline < $smarktop} {
+			_mark_range $this $topline [expr {$smarktop-1}]
+			set smarktop $topline
+		}
+		if {$botline > $smarkbot} {
+			_mark_range $this [expr {$smarkbot+1}] $botline
+			set smarkbot $botline
+		}
+	}
+}
+
+method scrolled {} {
+	if {$searchstring ne {}} {
+		after idle [cb _set_marks 0]
+	}
+}
+
+}
\ No newline at end of file
-- 
1.6.0.20.g6148bc
Andreas Ericsson· Oct 3, 2008, 08:29 UTC · re: Alexander Gavrilov · lore

Re: [PATCH] git-gui: Add a search command to the blame viewer.

Alexander Gavrilov wrote:
Show 17 quoted lines
> One of the largest deficiencies in the blame viewer at
> the moment is the impossibility to search for a text
> string. This commit fixes it by adding a Firefox-like
> search panel to the viewer.
> 
> The panel can be shown by pressing F7 or clicking a
> menu entry, and is hidden by pressing Esc. Find Next
> is available through the F3 key.
> 
> Implementation is based on the gitk code, but heavily
> refactored. It now also supports case-insensitive
> searches, and uses the text box background color to
> signal success or failure of the search.
> 
> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
> ---
> 

What with me being a total tcl nubling, I can't comment on the patch, but the intentions are clearly full of win and awesome. I was utterly surprised to notice this functionality wasn't in there the first time I needed it.

Thanks.
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Alexander Gavrilov· Oct 3, 2008, 07:36 UTC · re: Alexander Gavrilov · lore

It did not delete the object, which is not very good. Also, destroy may be fired up for subwindows, so we should check %W.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
	My own bug. I hope that now I understand
	Tcl better than 2 months ago.
	
	-- Alexander
 lib/blame.tcl |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
Show changes to lib/blame.tcl +8 −1
diff --git a/lib/blame.tcl b/lib/blame.tcl
index a45784c..765d08c 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -377,11 +377,18 @@ constructor new {i_commit i_path i_jump} {
 	"if {{$w.file_pane} eq {%W}} {[cb _resize %h]}"
 
 	wm protocol $top WM_DELETE_WINDOW "destroy $top"
-	bind $top <Destroy> [cb _kill]
+	bind $top <Destroy> [cb _handle_destroy %W]
 
 	_load $this $i_jump
 }
 
+method _handle_destroy {win} {
+	if {$win eq $w} {
+		_kill $this
+		delete_this
+	}
+}
+
 method _kill {} {
 	if {$current_fd ne {}} {
 		kill_file_process $current_fd
-- 
1.6.0.20.g6148bc

← back to recent threads