Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts
Hi David, Actually Im a TCL primer. This is the first time Im dealing with. That is why I kept it simple ( ie both accel-key and accel-label need to be defined in config ). I think, git-cola is using Qt style representation accel-key in the config file. Is git-cola is an official tool from git ? like git-gui? if not , My suggesion is, it is better to seperate config fields according to application-domain like, "git-gui-accel = " etc.. Other vise there is good chance for conflicts. ( Eg: consider the case that, was assined to a custom tool by git-cola ) Currently this patch will not handle any conflicting shortcuts. I think custom shortcuts will overwrite the other. On Thu, Mar 31, 2016 at 10:11 PM, David Aguilarwrote: > Hello, > > On Tue, Mar 29, 2016 at 11:38:10AM +, Harish K wrote: >> --- >> git-gui/lib/tools.tcl | 16 +--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl >> index 6ec9411..749bc67 100644 >> --- a/git-gui/lib/tools.tcl >> +++ b/git-gui/lib/tools.tcl >> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { >> } >> >> proc tools_populate_one {fullname} { >> - global tools_menubar tools_menutbl tools_id >> + global tools_menubar tools_menutbl tools_id repo_config >> >> if {![info exists tools_id]} { >> set tools_id 0 >> @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} { >> } >> } >> >> - tools_create_item $parent command \ >> + if {[info exists repo_config(guitool.$fullname.accelerator)] && [info >> exists repo_config(guitool.$fullname.accelerator-label)]} { >> + set accele_key $repo_config(guitool.$fullname.accelerator) >> + set accel_label >> $repo_config(guitool.$fullname.accelerator-label) >> + tools_create_item $parent command \ >> -label [lindex $names end] \ >> - -command [list tools_exec $fullname] >> + -command [list tools_exec $fullname] \ >> + -accelerator $accel_label >> + bind . $accele_key [list tools_exec $fullname] >> + } else { >> + tools_create_item $parent command \ >> + -label [lindex $names end] \ >> + -command [list tools_exec $fullname] >> + } >> } >> >> proc tools_exec {fullname} { >> >> -- >> https://github.com/git/git/pull/220 > > We also support "custom guitools" in git-cola using this same > mechanism. If this gets accepted then we'll want to make > similar change there. > > There's always a small risk that user-defined tools can conflict > with builtin shortcuts, but otherwise this seems like a pretty > nice feature. Curious, what is the behavior in the event of a > conflict? Do the builtins win? IIRC, Qt handles this by > disabling the shortcut and warning that it's ambiguous. > > Please documentation guitool..accellerator[-label] in > Documentation/config.txt otherwise users will not know that it > exists. > > It would also be good for the docs to clarify what the > accelerators look like in case we need to munge them when making > it work in cola via Qt, which has its own mechanism for > associating actions with shortcuts. Documented examples with > one and two modifier keys would be helpful. > > > cheers, > -- > David -- -Regards Harish.K -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts
On Tue, Mar 29, 2016 at 11:29:41AM +, Harish K wrote: > --- > git-gui/lib/tools.tcl | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) I forgot to mention that git-gui has its own repository. The git project merges the upstream repo as a subtree into its git-gui directory. You should probably clone the git-gui repository and create a patch there so that it can be applied to the upstream repo: http://repo.or.cz/w/git-gui.git -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts
Hello, On Tue, Mar 29, 2016 at 11:38:10AM +, Harish K wrote: > --- > git-gui/lib/tools.tcl | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl > index 6ec9411..749bc67 100644 > --- a/git-gui/lib/tools.tcl > +++ b/git-gui/lib/tools.tcl > @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { > } > > proc tools_populate_one {fullname} { > - global tools_menubar tools_menutbl tools_id > + global tools_menubar tools_menutbl tools_id repo_config > > if {![info exists tools_id]} { > set tools_id 0 > @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} { > } > } > > - tools_create_item $parent command \ > + if {[info exists repo_config(guitool.$fullname.accelerator)] && [info > exists repo_config(guitool.$fullname.accelerator-label)]} { > + set accele_key $repo_config(guitool.$fullname.accelerator) > + set accel_label > $repo_config(guitool.$fullname.accelerator-label) > + tools_create_item $parent command \ > -label [lindex $names end] \ > - -command [list tools_exec $fullname] > + -command [list tools_exec $fullname] \ > + -accelerator $accel_label > + bind . $accele_key [list tools_exec $fullname] > + } else { > + tools_create_item $parent command \ > + -label [lindex $names end] \ > + -command [list tools_exec $fullname] > + } > } > > proc tools_exec {fullname} { > > -- > https://github.com/git/git/pull/220 We also support "custom guitools" in git-cola using this same mechanism. If this gets accepted then we'll want to make similar change there. There's always a small risk that user-defined tools can conflict with builtin shortcuts, but otherwise this seems like a pretty nice feature. Curious, what is the behavior in the event of a conflict? Do the builtins win? IIRC, Qt handles this by disabling the shortcut and warning that it's ambiguous. Please documentation guitool..accellerator[-label] in Documentation/config.txt otherwise users will not know that it exists. It would also be good for the docs to clarify what the accelerators look like in case we need to munge them when making it work in cola via Qt, which has its own mechanism for associating actions with shortcuts. Documented examples with one and two modifier keys would be helpful. cheers, -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html