Merge lp:~mcintire-evan/unity/add-format-option into lp:unity

Proposed by Didier Roche-Tolomelli
Status: Rejected
Rejected by: Andrea Azzarone
Proposed branch: lp:~mcintire-evan/unity/add-format-option
Merge into: lp:unity
Diff against target: 54 lines (+37/-0)
1 file modified
launcher/VolumeLauncherIcon.cpp (+37/-0)
To merge this branch: bzr merge lp:~mcintire-evan/unity/add-format-option
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Disapprove
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+282282@code.launchpad.net

Description of the change

I've uploaded a branch, but it's not ready for review, I just need a bit of help. http://bazaar.launchpad.net/~mcintire-evan/unity/add-format-option/revision/4068 is what I have so far (I do need to rebase the code, but I'll do that after I get this bit working) I have some comments in there with what the problems are (at least what I think), and I've done some research and toying with solutions, but I don't know what the preferable way to go about it is; one of my ideas was to add a GetDeviceIdentifier() function to Volume and use that, but I'm not sure. I'd greatly appreciate some insight/help with this, thanks!

To post a comment you must log in.
Revision history for this message
Andrea Azzarone (azzar1) wrote :

I'll rebase this branch to make it depend on lp:~3v1n0/unity/launcher-filemanager-integration.

review: Approve
Revision history for this message
Andrea Azzarone (azzar1) :
review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:4068
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mcintire-evan/unity/add-format-option/+merge/282282/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-ci/1394/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-xenial-amd64-ci/44/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-xenial-armhf-ci/44/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-xenial-i386-ci/44/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-ci/1394/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :
review: Disapprove
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Yeah, I agree something had to be improved.

I had my review ready, but for some reason I didn't submit it. So, here's for the record.

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

Thanks Marco, but it seems Andrea made a branch going off of this that works as I had intended when I started this, thanks though!

Unmerged revisions

4068. By Evan McIntire

Begin adding format option to quicklists

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/VolumeLauncherIcon.cpp'
2--- launcher/VolumeLauncherIcon.cpp 2015-01-22 15:05:34 +0000
3+++ launcher/VolumeLauncherIcon.cpp 2016-01-12 10:32:24 +0000
4@@ -168,6 +168,7 @@
5 MenuItemsVector result;
6
7 AppendOpenItem(result);
8+ AppendFormatItem(result);
9 AppendSeparatorItem(result);
10 AppendNameItem(result);
11 AppendSeparatorItem(result);
12@@ -241,6 +242,42 @@
13
14 menu.push_back(menu_item);
15 }
16+
17+ void AppendFormatItem(MenuItemsVector& menu)
18+ {
19+ glib::Object<DbusmenuMenuitem> menu_item(dbusmenu_menuitem_new());
20+
21+ dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Format"));
22+ dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
23+ dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
24+
25+ gsignals_.Add(new ItemSignal(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED, [this] (DbusmenuMenuitem*, unsigned timestamp) {
26+ OpenFormatPrompt();
27+ }));
28+
29+ menu.push_back(menu_item);
30+ }
31+
32+ void OpenFormatPrompt()
33+ {
34+ //TODO: Clean vars up at the end of the function
35+ GAppInfo *appInfo;
36+ gchar *cmdline;
37+
38+ //This line is how it is done in nautilus, but we don't have access to the GVolume directly in this code
39+ //device_identifier = g_volume_get_identifier (volume, G_VOLUME_IDENTIFIER_KIND_UNIX_DEVICE);
40+
41+ //Fails with error "Invalid byte sequence in conversion input", and I don't even think it's the correct identifier
42+ auto const& identifier = volume_->GetIdentifier();
43+
44+ cmdline = g_strconcat ("gnome-disks ",
45+ "--block-device ", identifier, " ",
46+ "--format-device ",
47+ NULL);
48+
49+ appInfo = g_app_info_create_from_commandline (cmdline, NULL, G_APP_INFO_CREATE_NONE, NULL);
50+ g_app_info_launch (appInfo, NULL, NULL, NULL);
51+ }
52
53 void AppendEjectItem(MenuItemsVector& menu)
54 {