Re: [PATCH (GITK) v3 3/6] gitk: Add accelerators to frequently used menu commands.

Previous thread: [PATCH] Document that git-log takes --all-match. by Mikael Magnusson on Sunday, November 2, 2008 - 2:32 pm. (1 message)

Next thread: [PATCH (GITK) v3 6/6] gitk: Explicitly position popup windows. by Alexander Gavrilov on Sunday, November 2, 2008 - 2:59 pm. (10 messages)
To: <git@...>
Cc: Paul Mackerras <paulus@...>
Date: Sunday, November 2, 2008 - 2:59 pm

This is a resend of my gitk patches rebased on top of the current
master. I also split the first patch into three parts.

Alexander Gavrilov (6):
gitk: Add Return and Escape bindings to dialogs.
gitk: Make gitk dialog windows transient.
gitk: Add accelerators to frequently used menu commands.
gitk: Make cherry-pick call git-citool on conflicts.
gitk: Implement a user-friendly Edit View dialog.
gitk: Explicitly position popup windows.

gitk | 375 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 303 insertions(+), 72 deletions(-)
--

To: <git@...>
Cc: Paul Mackerras <paulus@...>
Date: Sunday, November 2, 2008 - 2:59 pm

It is often more convenient to dismiss or accept a
dialog using the keyboard, than by clicking buttons
on the screen. This commit adds key binding to make
it possible with gitk's dialogs.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
gitk | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/gitk b/gitk
index 3fe1b47..edef9e2 100755
--- a/gitk
+++ b/gitk
@@ -1746,6 +1746,8 @@ proc show_error {w top msg} {
pack $w.ok -side bottom -fill x
bind $top <Visibility> "grab $top; focus $top"
bind $top <Key-Return> "destroy $top"
+ bind $top <Key-space> "destroy $top"
+ bind $top <Key-Escape> "destroy $top"
tkwait window $top
}

@@ -1769,6 +1771,9 @@ proc confirm_popup msg {
button $w.cancel -text [mc Cancel] -command "destroy $w"
pack $w.cancel -side right -fill x
bind $w <Visibility> "grab $w; focus $w"
+ bind $w <Key-Return> "set confirm_ok 1; destroy $w"
+ bind $w <Key-space> "set confirm_ok 1; destroy $w"
+ bind $w <Key-Escape> "destroy $w"
tkwait window $w
return $confirm_ok
}
@@ -2611,6 +2616,7 @@ proc keys {} {
-justify left -bg white -border 2 -relief groove
pack $w.m -side top -fill both -padx 2 -pady 2
button $w.ok -text [mc "Close"] -command "destroy $w" -default active
+ bind $w <Key-Escape> [list destroy $w]
pack $w.ok -side bottom
bind $w <Visibility> "focus $w.ok"
bind $w <Key-Escape> "destroy $w"
@@ -3533,6 +3539,7 @@ proc vieweditor {top n title} {
frame $top.buts
button $top.buts.ok -text [mc "OK"] -command [list newviewok $top $n]
button $top.buts.can -text [mc "Cancel"] -command [list destroy $top]
+ bind $top <Escape> [list destroy $top]
grid $top.buts.ok $top.buts.can
grid columnconfigure $top.buts 0 -weight 1 -uniform a
grid columnconfigure $top.buts 1 -weight 1 -uniform a
@@ -7793,6 +7800...

To: Alexander Gavrilov <angavrilov@...>
Cc: <git@...>
Date: Friday, November 7, 2008 - 7:41 am

Thanks, applied.

Paul.
--

To: <git@...>
Cc: Paul Mackerras <paulus@...>
Date: Sunday, November 2, 2008 - 2:59 pm

Transient windows are always kept above their parent,
and don't occupy any space in the taskbar, which is useful
for dialogs. Also, when transient is used, it is important
to bind windows to the correct parent.

This commit adds transient annotations to all dialogs,
ensures usage of the correct parent for error and
confirmation popups, and, as a side job, makes gitk
preserve the create tag dialog window in case of errors.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
gitk | 46 ++++++++++++++++++++++++++++------------------
1 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/gitk b/gitk
index edef9e2..41d3d2d 100755
--- a/gitk
+++ b/gitk
@@ -1751,19 +1751,19 @@ proc show_error {w top msg} {
tkwait window $top
}

-proc error_popup msg {
+proc error_popup {msg {owner .}} {
set w .error
toplevel $w
- wm transient $w .
+ wm transient $w $owner
show_error $w $w $msg
}

-proc confirm_popup msg {
+proc confirm_popup {msg {owner .}} {
global confirm_ok
set confirm_ok 0
set w .confirm
toplevel $w
- wm transient $w .
+ wm transient $w $owner
message $w.m -text $msg -justify center -aspect 400
pack $w.m -side top -fill x -padx 20 -pady 20
button $w.ok -text [mc OK] -command "set confirm_ok 1; destroy $w"
@@ -2546,6 +2546,7 @@ proc about {} {
}
toplevel $w
wm title $w [mc "About gitk"]
+ wm transient $w .
message $w.m -text [mc "
Gitk - a commit viewer for git

@@ -2574,6 +2575,7 @@ proc keys {} {
}
toplevel $w
wm title $w [mc "Gitk key bindings"]
+ wm transient $w .
message $w.m -text "
[mc "Gitk key bindings:"]

@@ -3503,6 +3505,7 @@ proc vieweditor {top n title} {

toplevel $top
wm title $top $title
+ wm transient $top .
label $top.nl -text [mc "Name"]
entry $top.name -width 20 -textvariable newviewname($n)
grid $top.nl $top.name -sticky w -pady 5
@@ -3572,9 +3575,7 @@ pr...

To: Alexander Gavrilov <angavrilov@...>
Cc: <git@...>
Date: Friday, November 7, 2008 - 7:41 am

Thanks, applied.

Paul.
--

To: <git@...>
Cc: Paul Mackerras <paulus@...>
Date: Sunday, November 2, 2008 - 2:59 pm

This commit documents keyboard accelerators used for menu
commands in the menu, as it is usually done, and adds some
more, e.g. F4 to invoke Edit View.

The changes include a workaround for handling Shift-F4 on
systems where XKB binds special XF86_Switch_VT_* symbols
to Ctrl-Alt-F* combinations. Tk often receives these codes
when Shift-F* is pressed, so it is necessary to bind the
relevant actions to them as well.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
gitk | 36 +++++++++++++++++++++++++++++-------
1 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/gitk b/gitk
index 41d3d2d..f747c70 100755
--- a/gitk
+++ b/gitk
@@ -1801,6 +1801,11 @@ proc setoptions {} {
# command to invoke for command, or {variable value} for radiobutton
proc makemenu {m items} {
menu $m
+ if {[tk windowingsystem] eq {aqua}} {
+ set M1T Cmd
+ } else {
+ set M1T Ctrl
+ }
foreach i $items {
set name [mc [lindex $i 1]]
set type [lindex $i 2]
@@ -1826,7 +1831,9 @@ proc makemenu {m items} {
-value [lindex $thing 1]
}
}
- eval $m add $params [lrange $i 4 end]
+ set tail [lrange $i 4 end]
+ regsub -all {\$M1T\y} $tail $M1T tail
+ eval $m add $params $tail
if {$type eq "cascade"} {
makemenu $m.$submenu $thing
}
@@ -1860,17 +1867,17 @@ proc makewindow {} {
makemenu .bar {
{mc "File" cascade {
{mc "Update" command updatecommits -accelerator F5}
- {mc "Reload" command reloadcommits}
+ {mc "Reload" command reloadcommits -accelerator $M1T-F5}
{mc "Reread references" command rereadrefs}
- {mc "List references" command showrefs}
- {mc "Quit" command doquit}
+ {mc "List references" command showrefs -accelerator F2}
+ {mc "Quit" command doquit -accelerator $M1T-Q}
}}
{mc "Edit" cascade {
{mc "Preferences" command doprefs}
}}
{mc "View" cascade {
- {mc "New view..." command {newview 0}}
- {mc "Edit view..." command editview -state disabled}
+ ...

To: Alexander Gavrilov <angavrilov@...>
Cc: <git@...>
Date: Friday, November 7, 2008 - 7:50 am

This is solving the problem that the $M1T doesn't get expanded in the
call below because it's inside {}. If we are going to have a magic
string that gets expanded like this, I'd rather it didn't look like a
variable reference, because that is confusing.

Alternatively, we could define a [meta] function that does this:

proc meta {x} {
if {[tk windowingsystem] eq "aqua"} {
return Cmd-$x
}
return Ctrl-$x
}

and then use -accelerator [meta F5] in the makemenu call, and not have
any magic string substitution in makemenu. That will work since the
eval will evaluate the [].

Paul.
--

To: Paul Mackerras <paulus@...>
Cc: <git@...>
Date: Sunday, November 9, 2008 - 7:21 am

For some reason it didn't work. Maybe I did something wrong.

--- >8 ---
Subject: [PATCH] gitk: Add accelerators to frequently used menu commands.

This commit documents keyboard accelerators used for menu
commands in the menu, as it is usually done, and adds some
more, e.g. F4 to invoke Edit View.

The changes include a workaround for handling Shift-F4 on
systems where XKB binds special XF86_Switch_VT_* symbols
to Ctrl-Alt-F* combinations. Tk often receives these codes
when Shift-F* is pressed, so it is necessary to bind the
relevant actions to them as well.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
gitk | 36 +++++++++++++++++++++++++++++-------
1 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/gitk b/gitk
index 41d3d2d..a9618dc 100755
--- a/gitk
+++ b/gitk
@@ -1801,6 +1801,11 @@ proc setoptions {} {
# command to invoke for command, or {variable value} for radiobutton
proc makemenu {m items} {
menu $m
+ if {[tk windowingsystem] eq {aqua}} {
+ set Meta1 Cmd
+ } else {
+ set Meta1 Ctrl
+ }
foreach i $items {
set name [mc [lindex $i 1]]
set type [lindex $i 2]
@@ -1826,7 +1831,9 @@ proc makemenu {m items} {
-value [lindex $thing 1]
}
}
- eval $m add $params [lrange $i 4 end]
+ set tail [lrange $i 4 end]
+ regsub -all {\yMeta1\y} $tail $Meta1 tail
+ eval $m add $params $tail
if {$type eq "cascade"} {
makemenu $m.$submenu $thing
}
@@ -1860,17 +1867,17 @@ proc makewindow {} {
makemenu .bar {
{mc "File" cascade {
{mc "Update" command updatecommits -accelerator F5}
- {mc "Reload" command reloadcommits}
+ {mc "Reload" command reloadcommits -accelerator Meta1-F5}
{mc "Reread references" command rereadrefs}
- {mc "List references" command showrefs}
- {mc "Quit" command doquit}
+ {mc "List references" command showrefs -accelerator F2}
+ {mc "Quit" command doquit -accelerator Meta1-Q}
}}
{mc "Edit" cascade {
{mc "Pr...

To: <git@...>
Cc: Paul Mackerras <paulus@...>
Date: Sunday, November 2, 2008 - 2:59 pm

Now that git-gui has facilities to help users resolve
conflicts, it makes sense to launch it from other GUI
tools when they happen.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
gitk | 40 +++++++++++++++++++++++++++++++++++++++-
1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index f747c70..71f1b27 100755
--- a/gitk
+++ b/gitk
@@ -8110,6 +8110,32 @@ proc mkbrgo {top} {
}
}

+proc exec_citool {tool_args {baseid {}}} {
+ global commitinfo env
+
+ set save_env [array get env GIT_AUTHOR_*]
+
+ if {$baseid ne {}} {
+ if {![info exists commitinfo($baseid)]} {
+ getcommit $baseid
+ }
+ set author [lindex $commitinfo($baseid) 1]
+ set date [lindex $commitinfo($baseid) 2]
+ if {[regexp {^\s*(\S.*\S|\S)\s*<(.*)>\s*$} \
+ $author author name email]
+ && $date ne {}} {
+ set env(GIT_AUTHOR_NAME) $name
+ set env(GIT_AUTHOR_EMAIL) $email
+ set env(GIT_AUTHOR_DATE) $date
+ }
+ }
+
+ eval exec git citool $tool_args &
+
+ array unset env GIT_AUTHOR_*
+ array set env $save_env
+}
+
proc cherrypick {} {
global rowmenuid curview
global mainhead mainheadid
@@ -8128,7 +8154,19 @@ proc cherrypick {} {
# no error occurs, and exec takes that as an indication of error...
if {[catch {exec sh -c "git cherry-pick -r $rowmenuid 2>&1"} err]} {
notbusy cherrypick
- error_popup $err
+ if {[regexp -line \
+ {Entry '(.*)' would be overwritten by merge} $err msg fname]} {
+ error_popup [mc "Cherry-pick failed:
+file '%s' had local modifications.
+
+Please commit, reset or stash your changes." $fname]
+ } elseif {[regexp -line {^CONFLICT \(.*\):} $err msg]} {
+ # Force citool to read MERGE_MSG
+ file delete [file join [gitdir] "GITGUI_MSG"]
+ exec_citool {} $rowmenuid
+ } else {
+ error_popup $err
+ }
return
}
set newhead [exec git rev-parse HEAD]
--
1.6.0.3.15.gb8d36

--

To: Alexander Gavrilov <angavrilov@...>
Cc: <git@...>
Date: Friday, November 7, 2008 - 7:51 am

The resolution capabilities of git citool seem to be that it detects
the conflict markers and lets you run meld on the 3 versions. Have I
missed anything there? Do people find that fixing things manually in
meld is sufficient, or do we really want something more powerful?

Paul.
--

To: <git@...>
Cc: Paul Mackerras <paulus@...>
Date: Sunday, November 2, 2008 - 2:59 pm

Originally gitk required the user to specify all limiting
options manually in the same field with the list of commits.
It is rather unfriendly for new users, who may not know
which options can be used, or, indeed, that it is possible
to specify them at all.

This patch modifies the dialog to present the most useful
options as individual fields. Note that options that may
be useful to an extent, but produce a severely broken view,
are deliberately not included.

It is still possible to specify options in the commit list
field, but when the dialog is reopened, they will be extracted
into their own separate fields.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
gitk | 207 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 163 insertions(+), 44 deletions(-)

diff --git a/gitk b/gitk
index 71f1b27..5b4eaa2 100755
--- a/gitk
+++ b/gitk
@@ -3479,8 +3479,8 @@ proc shellsplit {str} {
# Code to implement multiple views

proc newview {ishighlight} {
- global nextviewnum newviewname newviewperm newishighlight
- global newviewargs revtreeargs viewargscmd newviewargscmd curview
+ global nextviewnum newviewname newishighlight
+ global revtreeargs viewargscmd newviewopts curview

set newishighlight $ishighlight
set top .gitkview
@@ -3489,9 +3489,9 @@ proc newview {ishighlight} {
return
}
set newviewname($nextviewnum) "[mc "View"] $nextviewnum"
- set newviewperm($nextviewnum) 0
- set newviewargs($nextviewnum) [shellarglist $revtreeargs]
- set newviewargscmd($nextviewnum) $viewargscmd($curview)
+ set newviewopts($nextviewnum,perm) 0
+ set newviewopts($nextviewnum,cmd) $viewargscmd($curview)
+ decode_view_opts $nextviewnum $revtreeargs
vieweditor $top $nextviewnum [mc "Gitk view definition"]
}

@@ -3505,53 +3505,167 @@ proc edit_or_newview {} {
}
}

+set known_view_options {
+ {perm b . {} {mc "Remember this view"}}
+ {args ...

Previous thread: [PATCH] Document that git-log takes --all-match. by Mikael Magnusson on Sunday, November 2, 2008 - 2:32 pm. (1 message)

Next thread: [PATCH (GITK) v3 6/6] gitk: Explicitly position popup windows. by Alexander Gavrilov on Sunday, November 2, 2008 - 2:59 pm. (10 messages)