git-gui: Convert blame to the "class" way of doing things

Our blame viewer code has historically been a mess simply
because the data for multiple viewers was all crammed into
a single pair of Tcl arrays.  This made the code hard to
read and even harder to maintain.

Now that we have a slightly better way of tracking the data
for our "meta-widgets" we can make use of it here in the
blame viewer to cleanup the code and make it easier to work
with long term.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/git-gui.sh b/git-gui.sh
index 67f993aa4bfc355aa2b581e5f850e765f3c7c7f4..568c814c60d4e8e9f73224c22a723eb2311594f4 100755 (executable)
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1590,7 +1590,7 @@ blame {
                exit 1
        }
        set current_branch [lindex $argv 0]
                exit 1
        }
        set current_branch [lindex $argv 0]

-       show_blame $current_branch [lindex $argv 1]
+       blame::new $current_branch [lindex $argv 1]

        return
 }
 citool -
        return
 }
 citool -

diff --git a/lib/blame.tcl b/lib/blame.tcl
index dc7b22f7b578e0eb973c262ca70c4be017401aea..37ce3e7a86a828d35369f2e7b59a94456c13e637 100644 (file)
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -1,20 +1,42 @@
 # git-gui blame viewer
 # Copyright (C) 2006, 2007 Shawn Pearce
 
 # git-gui blame viewer
 # Copyright (C) 2006, 2007 Shawn Pearce
 

-set next_browser_id 0
-
-proc show_blame {commit path} {
-       global next_browser_id blame_status blame_data
-
-       if {[winfo ismapped .]} {
-               set w .browser[incr next_browser_id]
-               set tl $w
-               toplevel $w
-       } else {
-               set w {}
-               set tl .
-       }
-       set blame_status($w) {Loading current file content...}
+class blame {
+
+field commit  ; # input commit to blame
+field path    ; # input filename to view in $commit
+
+field w
+field w_line
+field w_load
+field w_file
+field w_cmit
+field status
+
+field highlight_line   -1 ; # current line selected
+field highlight_commit {} ; # sha1 of commit selected
+
+field total_lines       0  ; # total length of file
+field blame_lines       0  ; # number of lines computed
+field commit_count      0  ; # number of commits in $commit_list
+field commit_list      {}  ; # list of commit sha1 in receipt order
+field order                ; # array commit -> receipt order
+field header               ; # array commit,key -> header field
+field line_commit          ; # array line -> sha1 commit
+field line_file            ; # array line -> file name
+
+field r_commit      ; # commit currently being parsed
+field r_orig_line   ; # original line number
+field r_final_line  ; # final line number
+field r_line_count  ; # lines in this region
+
+constructor new {i_commit i_path} {
+       set commit $i_commit
+       set path   $i_path
+
+       make_toplevel top w
+       wm title $top "[appname] ([reponame]): File Viewer"
+       set status "Loading $commit:$path..."

 
        label $w.path -text "$commit:$path" \
                -anchor w \
 
        label $w.path -text "$commit:$path" \
                -anchor w \


@@ -70,7 +92,8 @@ proc show_blame {commit path} {
        grid rowconfigure $w.out 0 -weight 1
        pack $w.out -fill both -expand 1
 
        grid rowconfigure $w.out 0 -weight 1
        pack $w.out -fill both -expand 1
 

-       label $w.status -textvariable blame_status($w) \
+       label $w.status \
+               -textvariable @status \

                -anchor w \
                -justify left \
                -borderwidth 1 \
                -anchor w \
                -justify left \
                -borderwidth 1 \


@@ -95,8 +118,14 @@ proc show_blame {commit path} {
        pack $w.cm -side bottom -fill x
 
        menu $w.ctxm -tearoff 0
        pack $w.cm -side bottom -fill x
 
        menu $w.ctxm -tearoff 0

-       $w.ctxm add command -label "Copy Commit" \
-               -command "blame_copycommit $w \$cursorW @\$cursorX,\$cursorY"
+       $w.ctxm add command \
+               -label "Copy Commit" \
+               -command [cb _copycommit]
+
+       set w_line $w.out.linenumber_t
+       set w_load $w.out.loaded_t
+       set w_file $w.out.file_t
+       set w_cmit $w.cm.t

 
        foreach i [list \
                $w.out.loaded_t \
 
        foreach i [list \
                $w.out.loaded_t \


@@ -111,14 +140,7 @@ proc show_blame {commit path} {
                        $w.out.linenumber_t \
                        $w.out.file_t \
                        ] yview $w.out.sby]
                        $w.out.linenumber_t \
                        $w.out.file_t \
                        ] yview $w.out.sby]

-               bind $i <Button-1> "
-                       blame_click {$w} \\
-                               $w.cm.t \\
-                               $w.out.linenumber_t \\
-                               $w.out.file_t \\
-                               $i @%x,%y
-                       focus $i
-               "
+               bind $i <Button-1> "[cb _click $i @%x,%y]; focus $i"

                bind_button3 $i "
                        set cursorX %x
                        set cursorY %y
                bind_button3 $i "
                        set cursorX %x
                        set cursorY %y


@@ -145,157 +167,122 @@ proc show_blame {commit path} {
        }
 
        bind $w.cm.t <Button-1> "focus $w.cm.t"
        }
 
        bind $w.cm.t <Button-1> "focus $w.cm.t"

-       bind $tl <Visibility> "focus $tl"
-       bind $tl <Destroy> "
-               array unset blame_status {$w}
-               array unset blame_data $w,*
-       "
-       wm title $tl "[appname] ([reponame]): File Viewer"
-
-       set blame_data($w,commit_count) 0
-       set blame_data($w,commit_list) {}
-       set blame_data($w,total_lines) 0
-       set blame_data($w,blame_lines) 0
-       set blame_data($w,highlight_commit) {}
-       set blame_data($w,highlight_line) -1
+       bind $top <Visibility> "focus $top"
+       bind $top <Destroy> [list delete_this $this]

 
        set cmd [list git cat-file blob "$commit:$path"]
        set fd [open "| $cmd" r]
        fconfigure $fd -blocking 0 -translation lf -encoding binary
 
        set cmd [list git cat-file blob "$commit:$path"]
        set fd [open "| $cmd" r]
        fconfigure $fd -blocking 0 -translation lf -encoding binary

-       fileevent $fd readable [list read_blame_catfile \
-               $fd $w $commit $path \
-               $w.cm.t $w.out.loaded_t $w.out.linenumber_t $w.out.file_t]
+       fileevent $fd readable [cb _read_file $fd]

 }
 
 }
 

-proc read_blame_catfile {fd w commit path w_cmit w_load w_line w_file} {
-       global blame_status blame_data
-
-       if {![winfo exists $w_file]} {
-               catch {close $fd}
-               return
-       }
-
-       set n $blame_data($w,total_lines)
+method _read_file {fd} {

        $w_load conf -state normal
        $w_line conf -state normal
        $w_file conf -state normal
        while {[gets $fd line] >= 0} {
                regsub "\r\$" $line {} line
        $w_load conf -state normal
        $w_line conf -state normal
        $w_file conf -state normal
        while {[gets $fd line] >= 0} {
                regsub "\r\$" $line {} line

-               incr n
+               incr total_lines

                $w_load insert end "\n"
                $w_load insert end "\n"

-               $w_line insert end "$n\n" linenumber
+               $w_line insert end "$total_lines\n" linenumber

                $w_file insert end "$line\n"
        }
        $w_load conf -state disabled
        $w_line conf -state disabled
        $w_file conf -state disabled
                $w_file insert end "$line\n"
        }
        $w_load conf -state disabled
        $w_line conf -state disabled
        $w_file conf -state disabled

-       set blame_data($w,total_lines) $n

 
        if {[eof $fd]} {
                close $fd
 
        if {[eof $fd]} {
                close $fd

-               blame_incremental_status $w
-               set cmd [list git blame -M -C --incremental]
-               lappend cmd $commit -- $path
+               _status $this
+               set cmd [list git blame -M -C --incremental $commit -- $path]

                set fd [open "| $cmd" r]
                fconfigure $fd -blocking 0 -translation lf -encoding binary
                set fd [open "| $cmd" r]
                fconfigure $fd -blocking 0 -translation lf -encoding binary

-               fileevent $fd readable [list read_blame_incremental $fd $w \
-                       $w_load $w_cmit $w_line $w_file]
-       }
-}
-
-proc read_blame_incremental {fd w w_load w_cmit w_line w_file} {
-       global blame_status blame_data
-
-       if {![winfo exists $w_file]} {
-               catch {close $fd}
-               return
+               fileevent $fd readable [cb _read_blame $fd]

        }
        }

+} ifdeleted { catch {close $fd} }

 
 

+method _read_blame {fd} {

        while {[gets $fd line] >= 0} {
                if {[regexp {^([a-z0-9]{40}) (\d+) (\d+) (\d+)$} $line line \
                        cmit original_line final_line line_count]} {
        while {[gets $fd line] >= 0} {
                if {[regexp {^([a-z0-9]{40}) (\d+) (\d+) (\d+)$} $line line \
                        cmit original_line final_line line_count]} {

-                       set blame_data($w,commit) $cmit
-                       set blame_data($w,original_line) $original_line
-                       set blame_data($w,final_line) $final_line
-                       set blame_data($w,line_count) $line_count
+                       set r_commit     $cmit
+                       set r_orig_line  $original_line
+                       set r_final_line $final_line
+                       set r_line_count $line_count

 
 

-                       if {[catch {set g $blame_data($w,$cmit,order)}]} {
+                       if {[catch {set g $order($cmit)}]} {

                                $w_line tag conf g$cmit
                                $w_file tag conf g$cmit
                                $w_line tag raise in_sel
                                $w_file tag raise in_sel
                                $w_file tag raise sel
                                $w_line tag conf g$cmit
                                $w_file tag conf g$cmit
                                $w_line tag raise in_sel
                                $w_file tag raise in_sel
                                $w_file tag raise sel

-                               set blame_data($w,$cmit,order) $blame_data($w,commit_count)
-                               incr blame_data($w,commit_count)
-                               lappend blame_data($w,commit_list) $cmit
+                               set order($cmit) $commit_count
+                               incr commit_count
+                               lappend commit_list $cmit

                        }
                } elseif {[string match {filename *} $line]} {
                        set file [string range $line 9 end]
                        }
                } elseif {[string match {filename *} $line]} {
                        set file [string range $line 9 end]

-                       set n $blame_data($w,line_count)
-                       set lno $blame_data($w,final_line)
-                       set cmit $blame_data($w,commit)
+                       set n    $r_line_count
+                       set lno  $r_final_line
+                       set cmit $r_commit

 
                        while {$n > 0} {
 
                        while {$n > 0} {

-                               if {[catch {set g g$blame_data($w,line$lno,commit)}]} {
+                               if {[catch {set g g$line_commit($lno)}]} {

                                        $w_load tag add annotated $lno.0 "$lno.0 lineend + 1c"
                                } else {
                                        $w_line tag remove g$g $lno.0 "$lno.0 lineend + 1c"
                                        $w_file tag remove g$g $lno.0 "$lno.0 lineend + 1c"
                                }
 
                                        $w_load tag add annotated $lno.0 "$lno.0 lineend + 1c"
                                } else {
                                        $w_line tag remove g$g $lno.0 "$lno.0 lineend + 1c"
                                        $w_file tag remove g$g $lno.0 "$lno.0 lineend + 1c"
                                }
 

-                               set blame_data($w,line$lno,commit) $cmit
-                               set blame_data($w,line$lno,file) $file
+                               set line_commit($lno) $cmit
+                               set line_file($lno)   $file

                                $w_line tag add g$cmit $lno.0 "$lno.0 lineend + 1c"
                                $w_file tag add g$cmit $lno.0 "$lno.0 lineend + 1c"
 
                                $w_line tag add g$cmit $lno.0 "$lno.0 lineend + 1c"
                                $w_file tag add g$cmit $lno.0 "$lno.0 lineend + 1c"
 

-                               if {$blame_data($w,highlight_line) == -1} {
+                               if {$highlight_line == -1} {

                                        if {[lindex [$w_file yview] 0] == 0} {
                                                $w_file see $lno.0
                                        if {[lindex [$w_file yview] 0] == 0} {
                                                $w_file see $lno.0

-                                               blame_showcommit $w $w_cmit $w_line $w_file $lno
+                                               _showcommit $this $lno

                                        }
                                        }

-                               } elseif {$blame_data($w,highlight_line) == $lno} {
-                                       blame_showcommit $w $w_cmit $w_line $w_file $lno
+                               } elseif {$highlight_line == $lno} {
+                                       _showcommit $this $lno

                                }
 
                                incr n -1
                                incr lno
                                }
 
                                incr n -1
                                incr lno

-                               incr blame_data($w,blame_lines)
+                               incr blame_lines

                        }
 
                        }
 

-                       set hc $blame_data($w,highlight_commit)
+                       set hc $highlight_commit

                        if {$hc ne {}
                        if {$hc ne {}

-                               && [expr {$blame_data($w,$hc,order) + 1}]
-                                       == $blame_data($w,$cmit,order)} {
-                               blame_showcommit $w $w_cmit $w_line $w_file \
-                                       $blame_data($w,highlight_line)
+                               && [expr {$order($hc) + 1}] == $order($cmit)} {
+                               _showcommit $this $highlight_line

                        }
                        }

-               } elseif {[regexp {^([a-z-]+) (.*)$} $line line header data]} {
-                       set blame_data($w,$blame_data($w,commit),$header) $data
+               } elseif {[regexp {^([a-z-]+) (.*)$} $line line key data]} {
+                       set header($r_commit,$key) $data

                }
        }
 
        if {[eof $fd]} {
                close $fd
                }
        }
 
        if {[eof $fd]} {
                close $fd

-               set blame_status($w) {Annotation complete.}
+               set status {Annotation complete.}

        } else {
        } else {

-               blame_incremental_status $w
+               _status $this

        }
        }

-}
-
-proc blame_incremental_status {w} {
-       global blame_status blame_data
+} ifdeleted { catch {close $fd} }

 
 

-       set have  $blame_data($w,blame_lines)
-       set total $blame_data($w,total_lines)
+method _status {} {
+       set have  $blame_lines
+       set total $total_lines

        set pdone 0
        if {$total} {set pdone [expr {100 * $have / $total}]}
 
        set pdone 0
        if {$total} {set pdone [expr {100 * $have / $total}]}
 

-       set blame_status($w) [format \
+       set status [format \

                "Loading annotations... %i of %i lines annotated (%2i%%)" \
                $have $total $pdone]
 }
 
                "Loading annotations... %i of %i lines annotated (%2i%%)" \
                $have $total $pdone]
 }
 

-proc blame_click {w w_cmit w_line w_file cur_w pos} {
+method _click {cur_w pos} {

        set lno [lindex [split [$cur_w index $pos] .] 0]
        if {$lno eq {}} return
 
        set lno [lindex [split [$cur_w index $pos] .] 0]
        if {$lno eq {}} return
 


@@ -304,24 +291,24 @@ proc blame_click {w w_cmit w_line w_file cur_w pos} {
        $w_line tag add in_sel $lno.0 "$lno.0 + 1 line"
        $w_file tag add in_sel $lno.0 "$lno.0 + 1 line"
 
        $w_line tag add in_sel $lno.0 "$lno.0 + 1 line"
        $w_file tag add in_sel $lno.0 "$lno.0 + 1 line"
 

-       blame_showcommit $w $w_cmit $w_line $w_file $lno
+       _showcommit $this $lno

 }
 
 }
 

-set blame_colors {
+variable blame_colors {

        #ff4040
        #ff40ff
        #4040ff
 }
 
        #ff4040
        #ff40ff
        #4040ff
 }
 

-proc blame_showcommit {w w_cmit w_line w_file lno} {
-       global blame_colors blame_data repo_config
+method _showcommit {lno} {
+       global repo_config
+       variable blame_colors

 
 

-       set cmit $blame_data($w,highlight_commit)
-       if {$cmit ne {}} {
-               set idx $blame_data($w,$cmit,order)
+       if {$highlight_commit ne {}} {
+               set idx $order($highlight_commit)

                set i 0
                foreach c $blame_colors {
                set i 0
                foreach c $blame_colors {

-                       set h [lindex $blame_data($w,commit_list) [expr {$idx - 1 + $i}]]
+                       set h [lindex $commit_list [expr {$idx - 1 + $i}]]

                        $w_line tag conf g$h -background white
                        $w_file tag conf g$h -background white
                        incr i
                        $w_line tag conf g$h -background white
                        $w_file tag conf g$h -background white
                        incr i


@@ -330,14 +317,15 @@ proc blame_showcommit {w w_cmit w_line w_file lno} {
 
        $w_cmit conf -state normal
        $w_cmit delete 0.0 end
 
        $w_cmit conf -state normal
        $w_cmit delete 0.0 end

-       if {[catch {set cmit $blame_data($w,line$lno,commit)}]} {
+       if {[catch {set cmit $line_commit($lno)} myerr]} {
+               puts "myerr = $myerr"

                set cmit {}
                $w_cmit insert end "Loading annotation..."
        } else {
                set cmit {}
                $w_cmit insert end "Loading annotation..."
        } else {

-               set idx $blame_data($w,$cmit,order)
+               set idx $order($cmit)

                set i 0
                foreach c $blame_colors {
                set i 0
                foreach c $blame_colors {

-                       set h [lindex $blame_data($w,commit_list) [expr {$idx - 1 + $i}]]
+                       set h [lindex $commit_list [expr {$idx - 1 + $i}]]

                        $w_line tag conf g$h -background $c
                        $w_file tag conf g$h -background $c
                        incr i
                        $w_line tag conf g$h -background $c
                        $w_file tag conf g$h -background $c
                        incr i


@@ -346,18 +334,18 @@ proc blame_showcommit {w w_cmit w_line w_file lno} {
                set author_name {}
                set author_email {}
                set author_time {}
                set author_name {}
                set author_email {}
                set author_time {}

-               catch {set author_name $blame_data($w,$cmit,author)}
-               catch {set author_email $blame_data($w,$cmit,author-mail)}
-               catch {set author_time [clock format $blame_data($w,$cmit,author-time)]}
+               catch {set author_name $header($cmit,author)}
+               catch {set author_email $header($cmit,author-mail)}
+               catch {set author_time [clock format $header($cmit,author-time)]}

 
                set committer_name {}
                set committer_email {}
                set committer_time {}
 
                set committer_name {}
                set committer_email {}
                set committer_time {}

-               catch {set committer_name $blame_data($w,$cmit,committer)}
-               catch {set committer_email $blame_data($w,$cmit,committer-mail)}
-               catch {set committer_time [clock format $blame_data($w,$cmit,committer-time)]}
+               catch {set committer_name $header($cmit,committer)}
+               catch {set committer_email $header($cmit,committer-mail)}
+               catch {set committer_time [clock format $header($cmit,committer-time)]}

 
 

-               if {[catch {set msg $blame_data($w,$cmit,message)}]} {
+               if {[catch {set msg $header($cmit,message)}]} {

                        set msg {}
                        catch {
                                set fd [open "| git cat-file commit $cmit" r]
                        set msg {}
                        catch {
                                set fd [open "| git cat-file commit $cmit" r]


@@ -377,29 +365,29 @@ proc blame_showcommit {w w_cmit w_line w_file lno} {
                                set author_name [encoding convertfrom $enc $author_name]
                                set committer_name [encoding convertfrom $enc $committer_name]
 
                                set author_name [encoding convertfrom $enc $author_name]
                                set committer_name [encoding convertfrom $enc $committer_name]
 

-                               set blame_data($w,$cmit,author) $author_name
-                               set blame_data($w,$cmit,committer) $committer_name
+                               set header($cmit,author) $author_name
+                               set header($cmit,committer) $committer_name

                        }
                        }

-                       set blame_data($w,$cmit,message) $msg
+                       set header($cmit,message) $msg

                }
 
                $w_cmit insert end "commit $cmit\n"
                $w_cmit insert end "Author: $author_name $author_email $author_time\n"
                $w_cmit insert end "Committer: $committer_name $committer_email $committer_time\n"
                }
 
                $w_cmit insert end "commit $cmit\n"
                $w_cmit insert end "Author: $author_name $author_email $author_time\n"
                $w_cmit insert end "Committer: $committer_name $committer_email $committer_time\n"

-               $w_cmit insert end "Original File: [escape_path $blame_data($w,line$lno,file)]\n"
+               $w_cmit insert end "Original File: [escape_path $line_file($lno)]\n"

                $w_cmit insert end "\n"
                $w_cmit insert end $msg
        }
        $w_cmit conf -state disabled
 
                $w_cmit insert end "\n"
                $w_cmit insert end $msg
        }
        $w_cmit conf -state disabled
 

-       set blame_data($w,highlight_line) $lno
-       set blame_data($w,highlight_commit) $cmit
+       set highlight_line $lno
+       set highlight_commit $cmit

 }
 
 }
 

-proc blame_copycommit {w i pos} {
-       global blame_data
-       set lno [lindex [split [$i index $pos] .] 0]
-       if {![catch {set commit $blame_data($w,line$lno,commit)}]} {
+method _copycommit {} {
+       set pos @$::cursorX,$::cursorY
+       set lno [lindex [split [$::cursorW index $pos] .] 0]
+       if {![catch {set commit $line_commit($lno)}]} {

                clipboard clear
                clipboard append \
                        -format STRING \
                clipboard clear
                clipboard append \
                        -format STRING \


@@ -407,3 +395,5 @@ proc blame_copycommit {w i pos} {
                        -- $commit
        }
 }
                        -- $commit
        }
 }

+
+}

diff --git a/lib/browser.tcl b/lib/browser.tcl
index f9a4e1d52c2f3823278c09e0241a463aca5a0ee4..fd86b11217cae2eab380362363cd7f4e8f79296a 100644 (file)
--- a/lib/browser.tcl
+++ b/lib/browser.tcl
@@ -140,7 +140,7 @@ method _enter {} {
                                append p [lindex $n 1]
                        }
                        append p $name
                                append p [lindex $n 1]
                        }
                        append p $name

-                       show_blame $browser_commit $p
+                       blame::new $browser_commit $p

                }
                }
        }
                }
                }
        }