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

Proposed by Roberto Alsina
Status: Merged
Approved by: Roberto Alsina
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 Approve
Manuel de la Peña (community) Approve
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.
Revision history for this message
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

No, 0 is not a good option

Revision history for this message
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

Revision history for this message
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

simplify a bit

Revision history for this message
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

Revision history for this message
Manuel de la Peña (mandel) wrote :

Did an IRL and got the native dialog.

review: Approve
281. By Roberto Alsina

moved constants into folders.py

282. By Roberto Alsina

move things around

283. By Roberto Alsina

merged trunk

284. By Roberto Alsina

stupid me

285. By Roberto Alsina

engage brain before coding

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks great!

review: Approve
286. By Roberto Alsina

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