Merge lp:~ruben-verweij/unity/fix-677594-workspaces into lp:unity

Proposed by Ruben Verweij
Status: Merged
Merged at revision: 860
Proposed branch: lp:~ruben-verweij/unity/fix-677594-workspaces
Merge into: lp:unity
Diff against target: 137 lines (+63/-13)
4 files modified
src/LauncherController.cpp (+36/-13)
src/LauncherController.h (+4/-0)
src/unityshell.cpp (+18/-0)
src/unityshell.h (+5/-0)
To merge this branch: bzr merge lp:~ruben-verweij/unity/fix-677594-workspaces
Reviewer Review Type Date Requested Status
David Barth (community) Approve
Jason Smith (community) Approve
Ruben Verweij (community) Needs Resubmitting
Alex Launi (community) Needs Fixing
Neil J. Patel Pending
Review via email: mp+43280@code.launchpad.net

This proposal supersedes a proposal from 2010-12-08.

Description of the change

Thanks for your quick reply and feedback!
I have updated the code as you have suggested, but I can't test it.
I am running Natty, how do I compile "my version" of Unity and run it to test whether it works?
Thanks again for your help!

To post a comment you must log in.
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Hi,

You don't need to use gconf to do this, you can just ask compiz directly for the number of workspaces with screen->vpSize ().width ();

Also if you want to be notified when this number changes you can implement a

bool
UnityScreen::setOptionForPlugin (const char *plugin, const char *name, CompOption::Value &v) and make sure that UnityScreen inherits ScreenInterface (and that we call ScreenInterface::setHandler (screen) in the UnityScreen construtor) and then check for plugin == "core" and name == "hsize" and react on that change.

See git.compiz.org/compiz/core/plugins/cube/src/cube.cpp:1622 for some ideas on this.

Remind me that this method is full of suck and I need to change it.

Revision history for this message
Neil J. Patel (njpatel) wrote : Posted in a previous version of this proposal

As per sams comment

review: Needs Fixing
Revision history for this message
Alex Launi (alexlauni) wrote :

Thanks for updating. You have conflicts in this branch though.
152 +>>>>>>> MERGE-SOURCE

That will cause the code to fail to compile.
There are instructions for building Unity from source available at https://wiki.ubuntu.com/Unity/InstallationGuideFromSource

review: Needs Fixing
Revision history for this message
Ruben Verweij (ruben-verweij) wrote :

Okay, I have resolved the conflicts and I hope it works now.

review: Needs Resubmitting
682. By Ruben Verweij

Conflicts resolved, remaining changes: modified:
      src/LauncherController.cpp
      src/LauncherController.h
        - Add a function that displays/hides the workspace launcher when the number
          of workspaces changes
      src/unity.cpp
      src/unity.h
       - Add a function that calls the function in the LauncherController if the
         number of workspaces has changed

Revision history for this message
Alex Launi (alexlauni) wrote :

Looks good but before we merge it we need you to sign the Canonical contributer agreement. It's a quick, but necessary step to getting your code into the tree. Luckily you only need to sign it once and it will apply to all other Canonical project contributions you may make in the future. http://www.canonical.com/contributors Make sure to CC David Barth when you send it in.

Revision history for this message
Ruben Verweij (ruben-verweij) wrote :

Okay, it's done. Thanks for your help!

-----Original Message-----
From: Alex Launi <email address hidden>
Reply-to: <email address hidden>
To: <email address hidden>
Subject: Re: [Merge] lp:~ruben-verweij/unity/fix-677594-workspaces into
lp:unity
Date: Fri, 11 Feb 2011 19:01:24 -0000

Looks good but before we merge it we need you to sign the Canonical contributer agreement. It's a quick, but necessary step to getting your code into the tree. Luckily you only need to sign it once and it will apply to all other Canonical project contributions you may make in the future. http://www.canonical.com/contributors Make sure to CC David Barth when you send it in.

Revision history for this message
Jason Smith (jassmith) wrote :

This patch leaks the Expo Icon every time the workspaces are turned off and back on

review: Needs Fixing
Revision history for this message
Jason Smith (jassmith) wrote :

I revoke my previous comment, I was wrong. +1 to merge.

review: Approve
Revision history for this message
David Barth (dbarth) wrote :

Ruben completed the (c) assignment process.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/LauncherController.cpp'
2--- src/LauncherController.cpp 2011-02-10 01:18:16 +0000
3+++ src/LauncherController.cpp 2011-02-10 21:57:02 +0000
4@@ -47,7 +47,11 @@
5 _device_section = new DeviceLauncherSection (_launcher);
6 _device_section->IconAdded.connect (sigc::mem_fun (this, &LauncherController::OnIconAdded));
7
8- InsertExpoAction ();
9+ _num_workspaces = _screen->vpSize ().width ();
10+ if(_num_workspaces > 1)
11+ {
12+ InsertExpoAction ();
13+ }
14 InsertTrash ();
15
16 g_timeout_add (500, (GSourceFunc) &LauncherController::BamfTimerCallback, this);
17@@ -159,21 +163,40 @@
18 RegisterIcon (icon);
19 }
20
21+LauncherController::UpdateNumWorkspaces (int workspaces)
22+{
23+ if ((_num_workspaces == 0) && (workspaces > 0))
24+ {
25+ InsertExpoAction ();
26+ }
27+ else if((_num_workspaces > 0) && (workspaces == 0))
28+ {
29+ RemoveExpoAction ();
30+ }
31+
32+ _num_workspaces = workspaces;
33+}
34+
35 void
36 LauncherController::InsertExpoAction ()
37 {
38- SimpleLauncherIcon *expoIcon;
39- expoIcon = new SimpleLauncherIcon (_launcher);
40-
41- expoIcon->SetTooltipText (_("Workspace Switcher"));
42- expoIcon->SetIconName ("workspace-switcher");
43- expoIcon->SetQuirk (LauncherIcon::QUIRK_VISIBLE, true);
44- expoIcon->SetQuirk (LauncherIcon::QUIRK_RUNNING, false);
45- expoIcon->SetIconType (LauncherIcon::TYPE_EXPO);
46-
47- expoIcon->MouseClick.connect (sigc::mem_fun (this, &LauncherController::OnExpoClicked));
48-
49- RegisterIcon (expoIcon);
50+ _expoIcon = new SimpleLauncherIcon (_launcher);
51+
52+ _expoIcon->SetTooltipText (_("Workspace Switcher"));
53+ _expoIcon->SetIconName ("workspace-switcher");
54+ _expoIcon->SetQuirk (LauncherIcon::QUIRK_VISIBLE, true);
55+ _expoIcon->SetQuirk (LauncherIcon::QUIRK_RUNNING, false);
56+ _expoIcon->SetIconType (LauncherIcon::TYPE_EXPO);
57+
58+ _expoIcon->MouseClick.connect (sigc::mem_fun (this, &LauncherController::OnExpoClicked));
59+
60+ RegisterIcon (_expoIcon);
61+}
62+
63+void
64+LauncherController::RemoveExpoAction ()
65+{
66+ _model->RemoveIcon (__expoIcon);
67 }
68
69 void
70
71=== modified file 'src/LauncherController.h'
72--- src/LauncherController.h 2011-02-09 07:58:39 +0000
73+++ src/LauncherController.h 2011-02-10 21:57:02 +0000
74@@ -61,6 +61,8 @@
75 PlaceLauncherSection* _place_section;
76 DeviceLauncherSection* _device_section;
77 LauncherEntryRemoteModel* _remote_model;
78+ SimpleLauncherIcon* _expoIcon;
79+ int _num_workspaces;
80
81 void SortAndSave ();
82
83@@ -71,7 +73,9 @@
84 void OnLauncerEntryRemoteAdded (LauncherEntryRemote *entry);
85 void OnLauncerEntryRemoteRemoved (LauncherEntryRemote *entry);
86
87+ void UpdateNumWorkspaces (int workspaces);
88 void InsertExpoAction ();
89+ void RemoveExpoAction ();
90
91 void InsertTrash ();
92
93
94=== modified file 'src/unityshell.cpp'
95--- src/unityshell.cpp 2011-02-09 21:00:00 +0000
96+++ src/unityshell.cpp 2011-02-10 21:57:02 +0000
97@@ -478,6 +478,24 @@
98 }
99 }
100
101+/* Handle changes in the number of workspaces by showing the switcher
102+ * or not showing the switcher */
103+bool
104+UnityScreen::setOptionForPlugin(const char *plugin, const char *name,
105+ CompOption::Value &v)
106+{
107+ bool status;
108+ status = screen->setOptionForPlugin (plugin, name, v);
109+ if (status)
110+ {
111+ if (strcmp (plugin, "core") == 0 && strcmp (name, "hsize") == 0)
112+ {
113+ controller->UpdateNumWorkspaces(screen->vpSize ().width ());
114+ }
115+ }
116+ return status;
117+}
118+
119 static gboolean
120 write_logger_data_to_disk (gpointer data)
121 {
122
123=== modified file 'src/unityshell.h'
124--- src/unityshell.h 2011-02-03 16:14:23 +0000
125+++ src/unityshell.h 2011-02-10 21:57:02 +0000
126@@ -111,6 +111,11 @@
127 /* handle option changes and change settings inside of the
128 * panel and dock views */
129 void optionChanged (CompOption *, Options num);
130+
131+ /* Handle changes in the number of workspaces by showing the switcher
132+ * or not showing the switcher */
133+ bool setOptionForPlugin(const char *plugin, const char *name,
134+ CompOption::Value &v);
135
136 /* init plugin actions for screen */
137 bool initPluginForScreen (CompPlugin *p);