Merge lp:~mcintire-evan/unity/add_screenshot_shortcut_hints into lp:unity

Proposed by Evan McIntire
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: no longer in the source branch.
Merged at revision: 4057
Proposed branch: lp:~mcintire-evan/unity/add_screenshot_shortcut_hints
Merge into: lp:unity
Diff against target: 98 lines (+37/-4)
4 files modified
shortcuts/AbstractShortcutHint.h (+2/-1)
shortcuts/CompizShortcutModeller.cpp (+10/-0)
shortcuts/MockShortcutHint.h (+3/-0)
shortcuts/ShortcutHint.cpp (+22/-3)
To merge this branch: bzr merge lp:~mcintire-evan/unity/add_screenshot_shortcut_hints
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Review via email: mp+280671@code.launchpad.net

Commit message

ShortcutHint: Add key shortcut hints for Screenshot and Window Screenshot

Key bindings are dynamically taken from gnome settings.

Description of the change

ShortcutHint: Add key shortcut hints for Screenshot and Window Screenshot

Key bindings are dynamically taken from gnome settings.

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Hi, thanks for you contribution.

Please, revert the changes in the po folder, these are generated automatically.

As for making this option dynamic I think there's some little more work involved:
 - enable the GNOME option type in AbstractShortcutHint.h
 - inside ShortcutHint.cpp read the options from:
   org.gnome.settings-daemon.plugins.media-keys <key_name>
   You can see how to read gsettings from UnitySettings.cpp,
   while in GnomeGrabber::Impl::GrabDBusAccelerator you can see how
   to parse them.
 - In CompizShortcutModeller.cpp you should add something like:
     std::make_shared<shortcut::Hint>(menubar, "", "", _("Take a screenshot."),
                                      shortcut::OptionType::GNOME,
                                      "screenshot")
     std::make_shared<shortcut::Hint>(menubar, "", "", _("Take a screenshot of the current window."),
                                      shortcut::OptionType::GNOME,
                                      "window-screenshot")

It should be fun :)

Revision history for this message
Evan McIntire (mcintire-evan) wrote :

Thanks! Do I read and parse the settings in the files you mentioned or in ShortcitHint.cpp? Other than that, everything is pretty clear

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Thanks! Do I read and parse the settings in the files you mentioned or in
> ShortcitHint.cpp? Other than that, everything is pretty clear

I'd do that in ShortcutHint.cpp, in CompizShortcutModeller just create the hint with a type and a setting key.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

That's great, there are some fixes to do though...

See the inline comments for details, main fixes are at http://pastebin.ubuntu.com/14072232/

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Looks great now.

Thanks a lot!!

review: Approve
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Sorry, there's a compilation issue because a different variable name has been used:

/«BUILDDIR»/unity-7.4.0+16.04.20151217/shortcuts/ShortcutHint.cpp: In member function 'virtual bool unity::shortcut::Hint::Fill()':
/«BUILDDIR»/unity-7.4.0+16.04.20151217/shortcuts/ShortcutHint.cpp:161:46: error: 'settings' was not declared in this scope
       glib::String key(g_settings_get_string(settings, arg1().c_str()));

review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Thanks, it compiles fine now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'shortcuts/AbstractShortcutHint.h'
2--- shortcuts/AbstractShortcutHint.h 2013-01-21 14:06:15 +0000
3+++ shortcuts/AbstractShortcutHint.h 2015-12-17 21:58:02 +0000
4@@ -36,7 +36,8 @@
5 COMPIZ_KEY = 0,
6 COMPIZ_METAKEY,
7 COMPIZ_MOUSE,
8- HARDCODED
9+ HARDCODED,
10+ GNOME
11 /* GSETTINGS,
12 * GCONF */
13 };
14
15=== modified file 'shortcuts/CompizShortcutModeller.cpp'
16--- shortcuts/CompizShortcutModeller.cpp 2015-12-11 12:44:53 +0000
17+++ shortcuts/CompizShortcutModeller.cpp 2015-12-17 21:58:02 +0000
18@@ -242,6 +242,16 @@
19 _("Moves focus between indicators."),
20 shortcut::OptionType::HARDCODED,
21 _("Cursor Left or Right")));
22+
23+ hints.push_back(std::make_shared<shortcut::Hint>(menubar, "", "",
24+ _("Take a screenshot."),
25+ shortcut::OptionType::GNOME,
26+ "screenshot"));
27+
28+ hints.push_back(std::make_shared<shortcut::Hint>(menubar, "", "",
29+ _("Take a screenshot of the current window."),
30+ shortcut::OptionType::GNOME,
31+ "window-screenshot"));
32 }
33
34 void CompizModeller::AddSwitcherHints(std::list<shortcut::AbstractHint::Ptr> &hints, bool ws_enabled)
35
36=== modified file 'shortcuts/MockShortcutHint.h'
37--- shortcuts/MockShortcutHint.h 2013-01-19 00:37:45 +0000
38+++ shortcuts/MockShortcutHint.h 2015-12-17 21:58:02 +0000
39@@ -59,6 +59,9 @@
40 value = arg1();
41 shortkey = prefix() + value() + postfix();
42 return true;
43+ case OptionType::GNOME:
44+ value = arg1();
45+ return true;
46 }
47
48 return false;
49
50=== modified file 'shortcuts/ShortcutHint.cpp'
51--- shortcuts/ShortcutHint.cpp 2013-01-19 00:37:45 +0000
52+++ shortcuts/ShortcutHint.cpp 2015-12-17 21:58:02 +0000
53@@ -21,16 +21,19 @@
54
55 #include <core/core.h> // Compiz...
56 #include <NuxCore/Logger.h>
57+#include <UnityCore/GLibWrapper.h>
58
59 #include "ShortcutHintPrivate.h"
60
61-DECLARE_LOGGER(logger, "unity.shortcut");
62-
63 namespace unity
64 {
65 namespace shortcut
66 {
67-
68+namespace
69+{
70+ const std::string GNOME_MEDIA_SETTINGS = "org.gnome.settings-daemon.plugins.media-keys";
71+ DECLARE_LOGGER(logger, "unity.shortcut");
72+}
73 // Ctor
74 Hint::Hint(std::string const& category,
75 std::string const& prefix,
76@@ -152,6 +155,22 @@
77 shortkey = prefix() + value() + postfix();
78 }
79 return true;
80+ case OptionType::GNOME:
81+ {
82+ glib::Object<GSettings> key_settings(g_settings_new(GNOME_MEDIA_SETTINGS.c_str()));
83+ glib::String key(g_settings_get_string(key_settings, arg1().c_str()));
84+
85+ std::string temp(impl::GetTranslatableLabel(key.Str()));
86+ temp = impl::ProperCase(temp);
87+
88+ if (value() != temp)
89+ {
90+ value = temp;
91+ shortkey = value();
92+ }
93+
94+ return true;
95+ }
96
97 default:
98 LOG_WARNING(logger) << "Unable to find the option type" << static_cast<unsigned>(type());