View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006986 | ardour | features | public | 2016-08-27 17:20 | 2016-08-29 14:28 |
| Reporter | AdmiralBumbleBee | Assigned To | |||
| Priority | normal | Severity | tweak | Reproducibility | always |
| Status | new | Resolution | open | ||
| Platform | Any | ||||
| Summary | 0006986: Include more information in bindings_collision_dialog | ||||
| Description | Currently when there's a colliding binding made, the user is not told which binding it collides with or what keys they pressed. Ideally the error dialog should show which binding is colliding with the new assignment, what assignment is made, and potentially an option to jump to the old binding to replace it. Image attached of better behaviour. | ||||
| Steps To Reproduce | Create a binding that already exists. Look at the error dialog that shows up. | ||||
| Tags | No tags attached. | ||||
|
|
|
|
|
binding.patch (3,027 bytes)
From c321d82e2ad5c49aef55f1b847336026668badcf Mon Sep 17 00:00:00 2001
From: ArdourEnv <ardourenv@Roberts-iMac.home>
Date: Sun, 28 Aug 2016 11:53:48 -0400
Subject: [PATCH] Unfortunate little patch for more clear binding's error
dialog
---
gtk2_ardour/keyeditor.cc | 9 ++++++---
libs/gtkmm2ext/bindings.cc | 7 +++++++
libs/gtkmm2ext/gtkmm2ext/bindings.h | 1 +
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/gtk2_ardour/keyeditor.cc b/gtk2_ardour/keyeditor.cc
index aea8185..18d8a3d 100644
--- a/gtk2_ardour/keyeditor.cc
+++ b/gtk2_ardour/keyeditor.cc
@@ -62,10 +62,13 @@ using Gtkmm2ext::Bindings;
sigc::signal<void> KeyEditor::UpdateBindings;
-void bindings_collision_dialog (Gtk::Window& parent)
+void bindings_collision_dialog (Gtk::Window& parent, const string &missing_binding, Gtkmm2ext::KeyboardKey& key)
{
ArdourDialog dialog (parent, _("Colliding keybindings"), true);
- Label label (_("The key sequence is already bound. Please remove the other binding first."));
+ // Clearly _exceptionally_ ugly. I didn't want to mess with localization stuff.
+ string const label_text(string("The key sequence is already bound. Please remove the other binding first.").insert(33, " to " + key.display_label()).insert(17, missing_binding + " "));
+
+ Label label (label_text);
dialog.get_vbox()->pack_start (label, true, true);
dialog.add_button (_("Ok"), Gtk::RESPONSE_ACCEPT);
@@ -316,7 +319,7 @@ KeyEditor::Tab::bind (GdkEventKey* release_event, guint pressed_key)
Gtkmm2ext::KeyboardKey new_binding (mod, pressed_key);
if (bindings->is_bound (new_binding, Gtkmm2ext::Bindings::Press)) {
- bindings_collision_dialog (owner);
+ bindings_collision_dialog (owner, bindings->get_action_name_by_key(new_binding, Gtkmm2ext::Bindings::Press), new_binding);
return;
}
diff --git a/libs/gtkmm2ext/bindings.cc b/libs/gtkmm2ext/bindings.cc
index e25ed20..de45495 100644
--- a/libs/gtkmm2ext/bindings.cc
+++ b/libs/gtkmm2ext/bindings.cc
@@ -1066,6 +1066,13 @@ Bindings::is_bound (KeyboardKey const& kb, Operation op) const
return km.find(kb) != km.end();
}
+string const&
+Bindings::get_action_name_by_key (KeyboardKey const& kb, Operation op) const
+{
+ const KeybindingMap& km = get_keymap(op);
+ return km.find(kb)->second.action_name;
+}
+
bool
Bindings::is_registered (Operation op, std::string const& action_name) const
{
diff --git a/libs/gtkmm2ext/gtkmm2ext/bindings.h b/libs/gtkmm2ext/gtkmm2ext/bindings.h
index d216e04..05d44c3 100644
--- a/libs/gtkmm2ext/gtkmm2ext/bindings.h
+++ b/libs/gtkmm2ext/gtkmm2ext/bindings.h
@@ -178,6 +178,7 @@ class LIBGTKMM2EXT_API Bindings {
bool activate (MouseButton, Operation);
bool is_bound (KeyboardKey const&, Operation) const;
+ const std::string &get_action_name_by_key(KeyboardKey const&, Operation op) const;
bool is_registered (Operation op, std::string const& action_name) const;
KeyboardKey get_binding_for_action (Glib::RefPtr<Gtk::Action>, Operation& op);
--
2.7.4 (Apple Git-66)
|
|
|
Small patch added, not sure how you folks handle changing localized strings, so it was just hacked in there. |
|
|
_("text to be localized") string_compose (_("text to be localized with %1 placeholders %2"), first, second) there are plural and context-sensitive variants in libs/pbd/pbd/i18n.h |
|
|
better_binding.patch (3,122 bytes)
From 35238cd4f14f00509d1dfaf6d01566ec04eb9768 Mon Sep 17 00:00:00 2001
From: Robert Randolph <audiolabs@gmail.com>
Date: Sun, 28 Aug 2016 13:42:24 -0400
Subject: [PATCH] Bindings dialog change
---
gtk2_ardour/keyeditor.cc | 13 ++++++++-----
libs/gtkmm2ext/bindings.cc | 7 +++++++
libs/gtkmm2ext/gtkmm2ext/bindings.h | 1 +
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/gtk2_ardour/keyeditor.cc b/gtk2_ardour/keyeditor.cc
index aea8185..f971879 100644
--- a/gtk2_ardour/keyeditor.cc
+++ b/gtk2_ardour/keyeditor.cc
@@ -62,12 +62,15 @@ using Gtkmm2ext::Bindings;
sigc::signal<void> KeyEditor::UpdateBindings;
-void bindings_collision_dialog (Gtk::Window& parent)
+void bindings_collision_dialog (Gtk::Window& parent, const string &missing_binding, Gtkmm2ext::KeyboardKey& key)
{
ArdourDialog dialog (parent, _("Colliding keybindings"), true);
- Label label (_("The key sequence is already bound. Please remove the other binding first."));
-
- dialog.get_vbox()->pack_start (label, true, true);
+ Label label_error (_("The key sequence is already bound. Please remove the other binding first."));
+ Label label_missing (string_compose("%1: %2\n%3: %4", _("Shortcut"), key.display_label(), _("Action"), missing_binding));
+
+ label_missing.set_padding(0, 10);
+ dialog.get_vbox()->pack_start (label_error, true, true);
+ dialog.get_vbox()->pack_end (label_missing, true, true);
dialog.add_button (_("Ok"), Gtk::RESPONSE_ACCEPT);
dialog.show_all ();
dialog.run();
@@ -316,7 +319,7 @@ KeyEditor::Tab::bind (GdkEventKey* release_event, guint pressed_key)
Gtkmm2ext::KeyboardKey new_binding (mod, pressed_key);
if (bindings->is_bound (new_binding, Gtkmm2ext::Bindings::Press)) {
- bindings_collision_dialog (owner);
+ bindings_collision_dialog (owner, bindings->get_action_name_by_key(new_binding, Gtkmm2ext::Bindings::Press), new_binding);
return;
}
diff --git a/libs/gtkmm2ext/bindings.cc b/libs/gtkmm2ext/bindings.cc
index e25ed20..de45495 100644
--- a/libs/gtkmm2ext/bindings.cc
+++ b/libs/gtkmm2ext/bindings.cc
@@ -1066,6 +1066,13 @@ Bindings::is_bound (KeyboardKey const& kb, Operation op) const
return km.find(kb) != km.end();
}
+string const&
+Bindings::get_action_name_by_key (KeyboardKey const& kb, Operation op) const
+{
+ const KeybindingMap& km = get_keymap(op);
+ return km.find(kb)->second.action_name;
+}
+
bool
Bindings::is_registered (Operation op, std::string const& action_name) const
{
diff --git a/libs/gtkmm2ext/gtkmm2ext/bindings.h b/libs/gtkmm2ext/gtkmm2ext/bindings.h
index d216e04..05d44c3 100644
--- a/libs/gtkmm2ext/gtkmm2ext/bindings.h
+++ b/libs/gtkmm2ext/gtkmm2ext/bindings.h
@@ -178,6 +178,7 @@ class LIBGTKMM2EXT_API Bindings {
bool activate (MouseButton, Operation);
bool is_bound (KeyboardKey const&, Operation) const;
+ const std::string &get_action_name_by_key(KeyboardKey const&, Operation op) const;
bool is_registered (Operation op, std::string const& action_name) const;
KeyboardKey get_binding_for_action (Glib::RefPtr<Gtk::Action>, Operation& op);
--
2.7.4 (Apple Git-66)
|
|
|
Less idiotic patch for bindings added. Behaves properly with localizations. |
|
|
sorry to say, but indentation looks wonky. also, get_action_by_key() will crash the application if called for an unbound key. either note that clearly in the code, or have it return some other value if the key is not found. otherwise, great stuff. |
|
|
bindings.patch (6,007 bytes)
From 35238cd4f14f00509d1dfaf6d01566ec04eb9768 Mon Sep 17 00:00:00 2001
From: ArdourEnv <ardourenv@Roberts-iMac.home>
Date: Sun, 28 Aug 2016 13:42:24 -0400
Subject: [PATCH 1/2] Bindings dialog change
---
gtk2_ardour/keyeditor.cc | 13 ++++++++-----
libs/gtkmm2ext/bindings.cc | 7 +++++++
libs/gtkmm2ext/gtkmm2ext/bindings.h | 1 +
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/gtk2_ardour/keyeditor.cc b/gtk2_ardour/keyeditor.cc
index aea8185..f971879 100644
--- a/gtk2_ardour/keyeditor.cc
+++ b/gtk2_ardour/keyeditor.cc
@@ -62,12 +62,15 @@ using Gtkmm2ext::Bindings;
sigc::signal<void> KeyEditor::UpdateBindings;
-void bindings_collision_dialog (Gtk::Window& parent)
+void bindings_collision_dialog (Gtk::Window& parent, const string &missing_binding, Gtkmm2ext::KeyboardKey& key)
{
ArdourDialog dialog (parent, _("Colliding keybindings"), true);
- Label label (_("The key sequence is already bound. Please remove the other binding first."));
-
- dialog.get_vbox()->pack_start (label, true, true);
+ Label label_error (_("The key sequence is already bound. Please remove the other binding first."));
+ Label label_missing (string_compose("%1: %2\n%3: %4", _("Shortcut"), key.display_label(), _("Action"), missing_binding));
+
+ label_missing.set_padding(0, 10);
+ dialog.get_vbox()->pack_start (label_error, true, true);
+ dialog.get_vbox()->pack_end (label_missing, true, true);
dialog.add_button (_("Ok"), Gtk::RESPONSE_ACCEPT);
dialog.show_all ();
dialog.run();
@@ -316,7 +319,7 @@ KeyEditor::Tab::bind (GdkEventKey* release_event, guint pressed_key)
Gtkmm2ext::KeyboardKey new_binding (mod, pressed_key);
if (bindings->is_bound (new_binding, Gtkmm2ext::Bindings::Press)) {
- bindings_collision_dialog (owner);
+ bindings_collision_dialog (owner, bindings->get_action_name_by_key(new_binding, Gtkmm2ext::Bindings::Press), new_binding);
return;
}
diff --git a/libs/gtkmm2ext/bindings.cc b/libs/gtkmm2ext/bindings.cc
index e25ed20..de45495 100644
--- a/libs/gtkmm2ext/bindings.cc
+++ b/libs/gtkmm2ext/bindings.cc
@@ -1066,6 +1066,13 @@ Bindings::is_bound (KeyboardKey const& kb, Operation op) const
return km.find(kb) != km.end();
}
+string const&
+Bindings::get_action_name_by_key (KeyboardKey const& kb, Operation op) const
+{
+ const KeybindingMap& km = get_keymap(op);
+ return km.find(kb)->second.action_name;
+}
+
bool
Bindings::is_registered (Operation op, std::string const& action_name) const
{
diff --git a/libs/gtkmm2ext/gtkmm2ext/bindings.h b/libs/gtkmm2ext/gtkmm2ext/bindings.h
index d216e04..05d44c3 100644
--- a/libs/gtkmm2ext/gtkmm2ext/bindings.h
+++ b/libs/gtkmm2ext/gtkmm2ext/bindings.h
@@ -178,6 +178,7 @@ class LIBGTKMM2EXT_API Bindings {
bool activate (MouseButton, Operation);
bool is_bound (KeyboardKey const&, Operation) const;
+ const std::string &get_action_name_by_key(KeyboardKey const&, Operation op) const;
bool is_registered (Operation op, std::string const& action_name) const;
KeyboardKey get_binding_for_action (Glib::RefPtr<Gtk::Action>, Operation& op);
--
2.7.4 (Apple Git-66)
From 944d62e80058cb938731d1a7be9ae9f61d58eb63 Mon Sep 17 00:00:00 2001
From: Robert Randolph <audiolabs@gmail.com>
Date: Mon, 29 Aug 2016 10:19:04 -0400
Subject: [PATCH 2/2] Resolved potential crash in get_action_by_name() and
fixed indentation.
---
gtk2_ardour/keyeditor.cc | 10 +++++-----
libs/gtkmm2ext/bindings.cc | 6 +++++-
libs/gtkmm2ext/gtkmm2ext/bindings.h | 2 +-
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/gtk2_ardour/keyeditor.cc b/gtk2_ardour/keyeditor.cc
index f971879..302606b 100644
--- a/gtk2_ardour/keyeditor.cc
+++ b/gtk2_ardour/keyeditor.cc
@@ -65,12 +65,12 @@ sigc::signal<void> KeyEditor::UpdateBindings;
void bindings_collision_dialog (Gtk::Window& parent, const string &missing_binding, Gtkmm2ext::KeyboardKey& key)
{
ArdourDialog dialog (parent, _("Colliding keybindings"), true);
- Label label_error (_("The key sequence is already bound. Please remove the other binding first."));
- Label label_missing (string_compose("%1: %2\n%3: %4", _("Shortcut"), key.display_label(), _("Action"), missing_binding));
-
- label_missing.set_padding(0, 10);
+ Label label_error (_("The key sequence is already bound. Please remove the other binding first."));
+ Label label_missing (string_compose("%1: %2\n%3: %4", _("Shortcut"), key.display_label(), _("Action"), missing_binding));
+
+ label_missing.set_padding(0, 10);
dialog.get_vbox()->pack_start (label_error, true, true);
- dialog.get_vbox()->pack_end (label_missing, true, true);
+ dialog.get_vbox()->pack_end (label_missing, true, true);
dialog.add_button (_("Ok"), Gtk::RESPONSE_ACCEPT);
dialog.show_all ();
dialog.run();
diff --git a/libs/gtkmm2ext/bindings.cc b/libs/gtkmm2ext/bindings.cc
index de45495..52834cd 100644
--- a/libs/gtkmm2ext/bindings.cc
+++ b/libs/gtkmm2ext/bindings.cc
@@ -1070,7 +1070,11 @@ string const&
Bindings::get_action_name_by_key (KeyboardKey const& kb, Operation op) const
{
const KeybindingMap& km = get_keymap(op);
- return km.find(kb)->second.action_name;
+ if (is_bound(kb, op)) {
+ return km.find(kb)->second.action_name;
+ } else {
+ return _("no action bound");
+ }
}
bool
diff --git a/libs/gtkmm2ext/gtkmm2ext/bindings.h b/libs/gtkmm2ext/gtkmm2ext/bindings.h
index 05d44c3..9b5a923 100644
--- a/libs/gtkmm2ext/gtkmm2ext/bindings.h
+++ b/libs/gtkmm2ext/gtkmm2ext/bindings.h
@@ -178,7 +178,7 @@ class LIBGTKMM2EXT_API Bindings {
bool activate (MouseButton, Operation);
bool is_bound (KeyboardKey const&, Operation) const;
- const std::string &get_action_name_by_key(KeyboardKey const&, Operation op) const;
+ const std::string &get_action_name_by_key(KeyboardKey const&, Operation op) const;
bool is_registered (Operation op, std::string const& action_name) const;
KeyboardKey get_binding_for_action (Glib::RefPtr<Gtk::Action>, Operation& op);
--
2.7.4 (Apple Git-66)
|
|
|
crash and indentation fixed hopefully. D'oh. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-08-27 17:20 | AdmiralBumbleBee | New Issue | |
| 2016-08-27 17:20 | AdmiralBumbleBee | File Added: Screen Shot 2016-08-27 at 13.16.50.png | |
| 2016-08-28 16:00 | AdmiralBumbleBee | File Added: binding.patch | |
| 2016-08-28 16:00 | AdmiralBumbleBee | Note Added: 0018503 | |
| 2016-08-28 16:25 | paul | Note Added: 0018504 | |
| 2016-08-28 17:47 | AdmiralBumbleBee | File Added: better_binding.patch | |
| 2016-08-28 17:47 | AdmiralBumbleBee | Note Added: 0018505 | |
| 2016-08-29 01:21 | paul | Note Added: 0018507 | |
| 2016-08-29 14:28 | AdmiralBumbleBee | File Added: bindings.patch | |
| 2016-08-29 14:28 | AdmiralBumbleBee | Note Added: 0018511 |
