Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5

2015-04-01 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122149/
---

(Updated April 1, 2015, 4:16 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and David Faure.


Repository: kde-cli-tools


Description
---

KCmdLineArgs used to define many arguments. In this case it was using --icon 
and --caption. At the moment, since we don't have these options we are getting 
warnings such as:
kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
QCommandLineParser: option not defined: caption
kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
QCommandLineParser: option not defined: icon

This patch addresses this by adding them explicitly in here. I'm unsure if we 
want to do any further engineering or that's good enough.


Diffs
-

  kcmshell/main.cpp 98e646b 

Diff: https://git.reviewboard.kde.org/r/122149/diff/


Testing
---

Ran it again, now it doesn't complain.


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5

2015-01-19 Thread Aleix Pol Gonzalez


 On Jan. 19, 2015, 6:08 p.m., David Faure wrote:
  That's one option, and a good one for compat.
  
  The other option is to change the callers to pass a single dash instead, 
  then Qt will process such options automatically - right?
 
 Aleix Pol Gonzalez wrote:
 Yes, but then we can't integrate them in our code, or can we?
 
 Also it feels weird to rely on something that isn't listed in --help.
 
 David Faure wrote:
 The integration is automatic, given that argc,argv is passed to the QApp 
 constructor before it even gets to your code with QCommandLineParser etc.
 
 If all you want is to set the icon and title of the mainwindow, then Qt 
 will take care of that.
 
 It just doesn't work with --icon/--title, it requires -icon/-title (on 
 X11) or -qwindowicon/-qwindowtitle (on all platforms). Suboptimal, I know, 
 but I didn't manage to get something better (due to compat).
 
 (We could do some s/--icon/-qwindowicon/ and s/--title/-qwindowtitle/ 
 before the qapp ctor, but that would require calling some helper func in all 
 main()s -- and given int argc, char ** argv it would be horrible to write 
 such a function.)

Well, to be honest all I want is to resolve the warning.

Maybe I can just remove the code that checks whether the user is passing it as 
an argument and let Qt do the correct thing if those arguments are passed.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122149/#review74347
---


On Jan. 19, 2015, 4:21 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122149/
 ---
 
 (Updated Jan. 19, 2015, 4:21 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde-cli-tools
 
 
 Description
 ---
 
 KCmdLineArgs used to define many arguments. In this case it was using --icon 
 and --caption. At the moment, since we don't have these options we are 
 getting warnings such as:
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: caption
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: icon
 
 This patch addresses this by adding them explicitly in here. I'm unsure if we 
 want to do any further engineering or that's good enough.
 
 
 Diffs
 -
 
   kcmshell/main.cpp 98e646b 
 
 Diff: https://git.reviewboard.kde.org/r/122149/diff/
 
 
 Testing
 ---
 
 Ran it again, now it doesn't complain.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 122149: Fix QCommandLineParser warnings in kcmshell5

2015-01-19 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122149/
---

Review request for KDE Frameworks and David Faure.


Repository: kde-cli-tools


Description
---

KCmdLineArgs used to define many arguments. In this case it was using --icon 
and --caption. At the moment, since we don't have these options we are getting 
warnings such as:
kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
QCommandLineParser: option not defined: caption
kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
QCommandLineParser: option not defined: icon

This patch addresses this by adding them explicitly in here. I'm unsure if we 
want to do any further engineering or that's good enough.


Diffs
-

  kcmshell/main.cpp 98e646b 

Diff: https://git.reviewboard.kde.org/r/122149/diff/


Testing
---

Ran it again, now it doesn't complain.


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5

2015-01-19 Thread David Faure

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122149/#review74347
---


That's one option, and a good one for compat.

The other option is to change the callers to pass a single dash instead, then 
Qt will process such options automatically - right?

- David Faure


On Jan. 19, 2015, 3:21 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122149/
 ---
 
 (Updated Jan. 19, 2015, 3:21 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde-cli-tools
 
 
 Description
 ---
 
 KCmdLineArgs used to define many arguments. In this case it was using --icon 
 and --caption. At the moment, since we don't have these options we are 
 getting warnings such as:
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: caption
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: icon
 
 This patch addresses this by adding them explicitly in here. I'm unsure if we 
 want to do any further engineering or that's good enough.
 
 
 Diffs
 -
 
   kcmshell/main.cpp 98e646b 
 
 Diff: https://git.reviewboard.kde.org/r/122149/diff/
 
 
 Testing
 ---
 
 Ran it again, now it doesn't complain.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5

2015-01-19 Thread David Faure


 On Jan. 19, 2015, 5:08 p.m., David Faure wrote:
  That's one option, and a good one for compat.
  
  The other option is to change the callers to pass a single dash instead, 
  then Qt will process such options automatically - right?
 
 Aleix Pol Gonzalez wrote:
 Yes, but then we can't integrate them in our code, or can we?
 
 Also it feels weird to rely on something that isn't listed in --help.

The integration is automatic, given that argc,argv is passed to the QApp 
constructor before it even gets to your code with QCommandLineParser etc.

If all you want is to set the icon and title of the mainwindow, then Qt will 
take care of that.

It just doesn't work with --icon/--title, it requires -icon/-title (on X11) or 
-qwindowicon/-qwindowtitle (on all platforms). Suboptimal, I know, but I didn't 
manage to get something better (due to compat).

(We could do some s/--icon/-qwindowicon/ and s/--title/-qwindowtitle/ before 
the qapp ctor, but that would require calling some helper func in all main()s 
-- and given int argc, char ** argv it would be horrible to write such a 
function.)


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122149/#review74347
---


On Jan. 19, 2015, 3:21 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122149/
 ---
 
 (Updated Jan. 19, 2015, 3:21 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde-cli-tools
 
 
 Description
 ---
 
 KCmdLineArgs used to define many arguments. In this case it was using --icon 
 and --caption. At the moment, since we don't have these options we are 
 getting warnings such as:
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: caption
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: icon
 
 This patch addresses this by adding them explicitly in here. I'm unsure if we 
 want to do any further engineering or that's good enough.
 
 
 Diffs
 -
 
   kcmshell/main.cpp 98e646b 
 
 Diff: https://git.reviewboard.kde.org/r/122149/diff/
 
 
 Testing
 ---
 
 Ran it again, now it doesn't complain.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5

2015-01-19 Thread Aleix Pol Gonzalez


 On Jan. 19, 2015, 6:08 p.m., David Faure wrote:
  That's one option, and a good one for compat.
  
  The other option is to change the callers to pass a single dash instead, 
  then Qt will process such options automatically - right?

Yes, but then we can't integrate them in our code, or can we?

Also it feels weird to rely on something that isn't listed in --help.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122149/#review74347
---


On Jan. 19, 2015, 4:21 p.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122149/
 ---
 
 (Updated Jan. 19, 2015, 4:21 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kde-cli-tools
 
 
 Description
 ---
 
 KCmdLineArgs used to define many arguments. In this case it was using --icon 
 and --caption. At the moment, since we don't have these options we are 
 getting warnings such as:
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: caption
 kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: 
 QCommandLineParser: option not defined: icon
 
 This patch addresses this by adding them explicitly in here. I'm unsure if we 
 want to do any further engineering or that's good enough.
 
 
 Diffs
 -
 
   kcmshell/main.cpp 98e646b 
 
 Diff: https://git.reviewboard.kde.org/r/122149/diff/
 
 
 Testing
 ---
 
 Ran it again, now it doesn't complain.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel