Merge lp:~ralsina/ubuntuone-control-panel/go-native into lp:ubuntuone-control-panel

Proposed by Roberto Alsina on 2012-03-06
Status: Merged
Approved by: Roberto Alsina on 2012-03-09
Approved revision: 286
Merged at revision: 280
Proposed branch: lp:~ralsina/ubuntuone-control-panel/go-native
Merge into: lp:ubuntuone-control-panel
Diff against target: 70 lines (+14/-4)
3 files modified
ubuntuone/controlpanel/gui/qt/addfolder.py (+12/-1)
ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py (+1/-1)
ubuntuone/controlpanel/utils/windows.py (+1/-2)
To merge this branch: bzr merge lp:~ralsina/ubuntuone-control-panel/go-native
Reviewer Review Type Date Requested Status
Natalia Bidart 2012-03-06 Approve on 2012-03-09
Manuel de la Peña (community) Approve on 2012-03-09
Review via email: mp+96123@code.launchpad.net

Commit Message

 - Switched to the native file chooser on Linux (Fixes LP: #947711).

Description of the Change

Only use the non-native file chooser on Windows

To post a comment you must log in.
Natalia Bidart (nataliabidart) wrote :

When opening the file choose from the Folders tab, I'm getting:

  File "/home/nessita/canonical/controlpanel/review_go-native/ubuntuone/controlpanel/gui/qt/__init__.py", line 94, in inner
    res = yield f(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/nessita/canonical/controlpanel/review_go-native/ubuntuone/controlpanel/gui/qt/addfolder.py", line 61, in on_clicked
    options=FILE_CHOOSER_OPTIONS)
TypeError: QFileDialog.getExistingDirectory(QWidget parent=None, QString caption=QString(), QString directory=QString(), QFileDialog.Options options=QFileDialog.ShowDirsOnly): argument 'options' has unexpected type 'int'

review: Needs Fixing
279. By Roberto Alsina on 2012-03-06

No, 0 is not a good option

Roberto Alsina (ralsina) wrote :

> When opening the file choose from the Folders tab, I'm getting:
>
> File "/home/nessita/canonical/controlpanel/review_go-
> native/ubuntuone/controlpanel/gui/qt/__init__.py", line 94, in inner
> res = yield f(*args, **kwargs)
> File "/usr/lib/python2.7/dist-packages/twisted/internet/defer.py", line
> 1039, in _inlineCallbacks
> result = g.send(result)
> File "/home/nessita/canonical/controlpanel/review_go-
> native/ubuntuone/controlpanel/gui/qt/addfolder.py", line 61, in on_clicked
> options=FILE_CHOOSER_OPTIONS)
> TypeError: QFileDialog.getExistingDirectory(QWidget parent=None, QString
> caption=QString(), QString directory=QString(), QFileDialog.Options
> options=QFileDialog.ShowDirsOnly): argument 'options' has unexpected type
> 'int'

I forgot to IRL test the last revno. So, I added the right constant now, but I amhaving two concerns about this branch:

1) It's importing PyQt outside qt/
2) I created a linux.py just for a constant

Natalia Bidart (nataliabidart) wrote :

> I forgot to IRL test the last revno. So, I added the right constant now, but I
> amhaving two concerns about this branch:
>
> 1) It's importing PyQt outside qt/
> 2) I created a linux.py just for a constant

What about if you just do the following in ubuntuone/controlpanel/utils/__init__.py?

if windows:
    FILE_CHOOSER_OPTIONS = QtGui.QFileDialog.DontUseNativeDialog | QtGui.QFileDialog.ShowDirsOnly
else:
    FILE_CHOOSER_OPTIONS = QtGui.QFileDialog.ShowDirsOnly

280. By Roberto Alsina on 2012-03-09

simplify a bit

Roberto Alsina (ralsina) wrote :

> What about if you just do the following in
> ubuntuone/controlpanel/utils/__init__.py?
>
> if windows:
> FILE_CHOOSER_OPTIONS = QtGui.QFileDialog.DontUseNativeDialog |
> QtGui.QFileDialog.ShowDirsOnly
> else:
> FILE_CHOOSER_OPTIONS = QtGui.QFileDialog.ShowDirsOnly

Sure, done in revno 280 (no need for the | ShowDirsOnly). It would still be importing QtGui outside qt/ though

Manuel de la Peña (mandel) wrote :

Did an IRL and got the native dialog.

review: Approve
281. By Roberto Alsina on 2012-03-09

moved constants into folders.py

282. By Roberto Alsina on 2012-03-09

move things around

283. By Roberto Alsina on 2012-03-09

merged trunk

284. By Roberto Alsina on 2012-03-09

stupid me

285. By Roberto Alsina on 2012-03-09

engage brain before coding

Natalia Bidart (nataliabidart) wrote :

Looks great!

review: Approve
286. By Roberto Alsina on 2012-03-09

typo

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ubuntuone/controlpanel/gui/qt/addfolder.py'
2--- ubuntuone/controlpanel/gui/qt/addfolder.py 2012-01-18 19:52:25 +0000
3+++ ubuntuone/controlpanel/gui/qt/addfolder.py 2012-03-09 14:19:21 +0000
4@@ -19,6 +19,7 @@
5 """The user interface for the control panel for Ubuntu One."""
6
7 from __future__ import division
8+import sys
9
10 from PyQt4 import QtGui, QtCore
11 from twisted.internet import defer
12@@ -32,6 +33,16 @@
13 logger = setup_logging('qt.addfolder')
14
15 CLOSE = QtGui.QMessageBox.Close
16+# NOTE: this is temporary because of a Qt bug that will be fixed
17+# on Qt 4.9. You cannot, in general, use sys.platform in these
18+# modules. Do not do this. Please. This is an exception, one
19+# time only, pinky-swear, cross my heart and hope to die if
20+# I lie.
21+FILE_CHOOSER_OPTIONS = QtGui.QFileDialog.ShowDirsOnly
22+if sys.platform == 'win32':
23+ FILE_CHOOSER_OPTIONS |= QtGui.QFileDialog.DontUseNativeDialog
24+# Yes, that is true. If you do that again, you will be hunted
25+# down and taught a lesson. You will be sorry.
26
27
28 class AddFolderButton(cache.Cache, QtGui.QPushButton):
29@@ -57,7 +68,7 @@
30 home_dir = yield self.backend.get_home_dir()
31 folder = QtGui.QFileDialog.getExistingDirectory(
32 parent=self, directory=home_dir,
33- options=QtGui.QFileDialog.DontUseNativeDialog)
34+ options=FILE_CHOOSER_OPTIONS)
35 folder = unicode(QtCore.QDir.toNativeSeparators(folder))
36 logger.debug('on_add_folder_button_clicked: user requested folder '
37 'creation for path %r', folder)
38
39=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py'
40--- ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py 2012-03-02 17:13:04 +0000
41+++ ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py 2012-03-09 14:19:21 +0000
42@@ -81,7 +81,7 @@
43 home_dir = yield self.ui.backend.get_home_dir()
44 self.assertEqual(FakedFileDialog.args, ())
45 self.assertEqual(FakedFileDialog.kwargs, {
46- 'options': gui.QtGui.QFileDialog.DontUseNativeDialog,
47+ 'options': gui.FILE_CHOOSER_OPTIONS,
48 'directory': home_dir,
49 'parent': self.ui,
50 })
51
52=== modified file 'ubuntuone/controlpanel/utils/windows.py'
53--- ubuntuone/controlpanel/utils/windows.py 2011-11-15 12:36:15 +0000
54+++ ubuntuone/controlpanel/utils/windows.py 2012-03-09 14:19:21 +0000
55@@ -15,7 +15,7 @@
56 # You should have received a copy of the GNU General Public License along
57 # with this program. If not, see <http://www.gnu.org/licenses/>.
58
59-"""Autoupdate functions for windows."""
60+"""Miscelaneous functions and constants for windows."""
61
62 import os
63 import sys
64@@ -24,7 +24,6 @@
65 # pylint: disable=F0401
66 import win32api
67 # pylint: enable=F0401
68-
69 from twisted.internet import defer
70 from twisted.internet.utils import getProcessValue
71

Subscribers

People subscribed via source and target branches