Merge lp:~josharenson/unity8/doc_args into lp:unity8

Proposed by Josh Arenson
Status: Superseded
Proposed branch: lp:~josharenson/unity8/doc_args
Merge into: lp:unity8
Diff against target: 87 lines (+32/-4)
1 file modified
src/main.cpp (+32/-4)
To merge this branch: bzr merge lp:~josharenson/unity8/doc_args
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Albert Astals Cid Pending
Review via email: mp+217158@code.launchpad.net

This proposal supersedes a proposal from 2014-04-22.

This proposal has been superseded by a proposal from 2014-04-25.

Description of the change

 * Are there any related MPs required for this MP to build/function as expected? Please list.
lp:~aacid/unity8/killqt51

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Compiled code, verified new feature works as expected, and ran ctest

 * Did you make sure that your branch does not contain spurious tags?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Dependent branch does. This MP does not

 * If you changed the UI, has there been a design review?
na

To post a comment you must log in.
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

Should increase the qt5 version number dependency in debian/control to 5.2 since QCommandLineParser is new for 5.2
Also probably in the find_package of CMakeListst.txt

review: Needs Fixing
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

And you're missing https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8 as part of your description of the change

Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

And finally, we don't support Qt translation system (we are using gettext) so for now i think we can just pretend we don't need to translate the commandline options and just drop the QCoreApplication::translate() and just pass the English text as second param of QCommandLineOption.

Saviq, what's your opinion?

Revision history for this message
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal

Yeah, let's leave the help in English. It's not like they're general-use.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

In case you're confused by all the failings → whitespace issues

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal

src/main.cpp: bad whitespace in lines 67, 84

Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

you probably want to merge lp:~aacid/unity8/killqt51 into here (you'll have to resubmit the MR setting a prerequisite branch)

Revision history for this message
Josh Arenson (josharenson) wrote : Posted in a previous version of this proposal

> you probably want to merge lp:~aacid/unity8/killqt51 into here (you'll have
> to resubmit the MR setting a prerequisite branch)

So if I clean up my white space, depend on _your_ MP, and resubmit _this_ MP in the proper format, we should be good to go?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

You didn't merge Albert's branch into this one, and you didn't put it in the "prerequisite" field when resubmitting.

lp:~josharenson/unity8/doc_args updated
860. By Josh Arenson

Merged https://code.launchpad.net/+branch/~aacid/unity8/killqt51

861. By Josh Arenson

Added windowgeometry to argument list

862. By Josh Arenson

Handle windowgeometry argument with new argument parsing scheme.

This should retain backwards compatibility.

863. By Josh Arenson

Refactored ApplicationArguments constructor(s)

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/main.cpp'
2--- src/main.cpp 2014-04-02 11:19:54 +0000
3+++ src/main.cpp 2014-04-24 23:30:06 +0000
4@@ -18,6 +18,7 @@
5 */
6
7 // Qt
8+#include <QCommandLineParser>
9 #include <QtQuick/QQuickView>
10 #include <QtGui/QGuiApplication>
11 #include <QtQml/QQmlEngine>
12@@ -43,6 +44,28 @@
13
14 QGuiApplication::setApplicationName("Unity 8");
15 QGuiApplication *application;
16+
17+ QCommandLineParser parser;
18+ parser.setApplicationDescription("Description: Unity 8 Shell");
19+ parser.addHelpOption();
20+
21+ QCommandLineOption fullscreenOption("fullscreen",
22+ "Run in fullscreen");
23+ parser.addOption(fullscreenOption);
24+
25+ QCommandLineOption framelessOption("frameless",
26+ "Run without window borders");
27+ parser.addOption(framelessOption);
28+
29+ QCommandLineOption mousetouchOption("mousetouch",
30+ "Allow the mouse to provide touch input");
31+ parser.addOption(mousetouchOption);
32+
33+ QCommandLineOption testabilityOption("testability",
34+ "DISCOURAGED: Please set QT_LOAD_TESTABILITY instead. \n \
35+Load the testability driver");
36+ parser.addOption(testabilityOption);
37+
38 if (isUbuntuMirServer) {
39 QLibrary unityMir("unity-mir", 1);
40 unityMir.load();
41@@ -59,6 +82,11 @@
42 application = new QGuiApplication(argc, (char**)argv);
43 }
44
45+ // Treat args with single dashes the same as arguments with two dashes
46+ // Ex: -fullscreen == --fullscreen
47+ parser.setSingleDashWordOptionMode(QCommandLineParser::ParseAsLongOptions);
48+ parser.process(*application);
49+
50 QString indicatorProfile = qgetenv("UNITY_INDICATOR_PROFILE");
51 if (indicatorProfile.isEmpty()) {
52 indicatorProfile = "phone";
53@@ -71,7 +99,7 @@
54
55 // The testability driver is only loaded by QApplication but not by QGuiApplication.
56 // However, QApplication depends on QWidget which would add some unneeded overhead => Let's load the testability driver on our own.
57- if (args.contains(QLatin1String("-testability")) || getenv("QT_LOAD_TESTABILITY")) {
58+ if (parser.isSet(testabilityOption) || getenv("QT_LOAD_TESTABILITY")) {
59 QLibrary testLib(QLatin1String("qttestability"));
60 if (testLib.load()) {
61 typedef void (*TasInitialize)(void);
62@@ -94,14 +122,14 @@
63 view->engine()->setBaseUrl(QUrl::fromLocalFile(::qmlDirectory()));
64 view->rootContext()->setContextProperty("applicationArguments", &qmlArgs);
65 view->rootContext()->setContextProperty("indicatorProfile", indicatorProfile);
66- if (args.contains(QLatin1String("-frameless"))) {
67+ if (parser.isSet(framelessOption)) {
68 view->setFlags(Qt::FramelessWindowHint);
69 }
70
71 // You will need this if you want to interact with touch-only components using a mouse
72 // Needed only when manually testing on a desktop.
73 MouseTouchAdaptor *mouseTouchAdaptor = 0;
74- if (args.contains(QLatin1String("-mousetouch"))) {
75+ if (parser.isSet(mousetouchOption)) {
76 mouseTouchAdaptor = new MouseTouchAdaptor;
77 application->installNativeEventFilter(mouseTouchAdaptor);
78 }
79@@ -125,7 +153,7 @@
80 view->setSource(source);
81 view->setColor("transparent");
82
83- if (qgetenv("QT_QPA_PLATFORM") == "ubuntu" || isUbuntuMirServer || args.contains(QLatin1String("-fullscreen"))) {
84+ if (qgetenv("QT_QPA_PLATFORM") == "ubuntu" || isUbuntuMirServer || parser.isSet(fullscreenOption)) {
85 view->showFullScreen();
86 } else {
87 view->show();

Subscribers

People subscribed via source and target branches