Re: Review Request 123874: Fix build with Qt >= 5.4.2

[Thiago Macieira]

On Thursday 21 May 2015 17:32:33 Hrvoje Senjan wrote:
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123874/
> ---
> 
> Review request for Build System, Phonon and Harald Sitter.
> 
> 
> Repository: phonon
> 
> 
> Description
> ---
> 
> Or to be more accurate, with commit 3eca75de67b3fd2c890715b30c7899cebc096fe9
> I am not convinced this is best solution, but requires least changes to
> current cmake code.

Upgrade past commit 083c9269ed73e8771e1dbe10812696b45b7389f3 and report back 
if it worked.

If there are problems, let's fix them in Qt itself. If there are issues that 
can't be fixed in Qt itself, then kde-multimedia would be the wrong mailing 
list to discuss it.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center
  PGP/GPG: 0x6EF45358; fingerprint:
  E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358

Re: Review Request 123806: [klipper] Ignore empty / blank entries

[Thomas Lübking]

> On Mai 21, 2015, 5:02 nachm., Patrick Eigensatz wrote:
> > klipper/klipper.kcfg, line 24
> > 
> >
> > **Question:** Compiled plasma-workspace using *kdesrc-build 
> > --include-dependencies plasma-workspace*, applied patch and compiled again.
> > -> Build broken because g++ could not
> > locate *klippersettings.h* included in *historyitem.cpp*. Shouldn't 
> > this file be generated by *cmake*?
> > If I generate *klippersettings{.h,.cpp}* using *kconfig_compiler 
> > klipper.kcfg klippersettings.kcfgc* and run *kdesrc* again,
> > I get a build error:
> > 
> > CMakeFiles/testHistory.dir/__/historyitem.cpp.o: In function 
> > `KlipperSettings::allowWhitespaceEntries()':
> > /home/patrick/kdesrc/plasma-workspace/klipper/klippersettings.h:72: 
> > undefined reference to `KlipperSettings::self()'
> > 
> > 
> > Note: The whole file is full of *return self()->member*, I don't know 
> > why it fails on line 72 if it worked
> > the for last 20 members. (I didn't change anything, original 
> > kconfig_compiler output)

> Shouldn't this file be generated by cmake?

Yes, in the build dir.

> I get a build error:

That's a linker error. No idea about the actual reason (w/o seeing the source), 
but kconfig_compiler_kf5 should be used to generate that file.
I assume you don't have that tool and that's why the config build failed in the 
first place?

Arch ships it with the kconfig package, your distro might have it in some 
kconfig-dev?


- Thomas


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


On Mai 21, 2015, 5 nachm., Patrick Eigensatz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123806/
> ---
> 
> (Updated Mai 21, 2015, 5 nachm.)
> 
> 
> Review request for kde-workspace, KDE Usability and Patrick Eigensatz.
> 
> 
> Bugs: 159267 and 192922
> https://bugs.kde.org/show_bug.cgi?id=159267
> https://bugs.kde.org/show_bug.cgi?id=192922
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> [PATCH] plasma-workspace: klipper: Fix #192922 Ignore blank entries
> 
> QString::isEmpty() is used to check if the string only consists of whitespace 
> characters. If it does, the creation of the HistoryStringItem fails.
> 
> 
> Diffs
> -
> 
>   klipper/generalconfig.ui f513e9c 
>   klipper/historyitem.cpp 36cbe61 
>   klipper/klipper.h 6952b11 
>   klipper/klipper.cpp 798b49f 
>   klipper/klipper.kcfg a03dd16 
> 
> Diff: https://git.reviewboard.kde.org/r/123806/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Patrick Eigensatz
> 
>

Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

[Gregor Mi]

> On April 20, 2015, 10:36 p.m., Thomas Pfeiffer wrote:
> > What would likely be confusing is that the two button modes have different 
> > interaction flows: The "End Process" mode requires to first select a 
> > process and then press the button to work, whereas the "Kill specific 
> > window" mode requires to first press the button and then select the window 
> > to kill, and users have no easy way to understand how each one works and 
> > why they work differently.
> > 
> > The ellipsis in the label "End Process..." adds to that confusion. It 
> > indicates that further input is necessary before the action can take 
> > effect. While the button does open a dialog to confirm killing the selected 
> > process, ellipses are actually reserved for actions where a dialog asks for 
> > new information, such as the "Save As..." button, not for actions that 
> > require confirmation.
> > 
> > To avoid this confusion, it should be possible to also click "End 
> > Process..." even if no processes has been selected, whuch would then ask 
> > the user to select the process to kill. This could theoretically be done 
> > similarly to the "Kill specific window" function: Click the "End 
> > Process..." button and then click the process in the list that you want to 
> > end. Alternatively, if no process had been selected when "End Process..." 
> > is clicked, a dialog could be opened where the process to kill would be 
> > selected. Of course the current flow of ending a process could and should 
> > still work.
> 
> Gregor Mi wrote:
> Thanks for the feedback! The ellipsis in "End Process..." is the original 
> design. According to your explanation this was wrong in the first place. What 
> about removing the ellipses in both menu items so we will end up with "End 
> Process" and "Kill a specific window"?
> 
> Thomas Pfeiffer wrote:
> "Kill specific window" does always need additional input until it really 
> does something, doesn't it? As I understood it merely changes the cursor to 
> the kill cursor and then the user has to select the window to kill, right?
> 
> Gregor Mi wrote:
> Erm, right. To be exact, "Kill specific window..." only shows a message 
> box. But in the end - after the user presses the keyboard shortcut - the user 
> has to select a window. So this seems to be a special case. The intention 
> behind all this is to increase the discoverability of the hidden xkill 
> feature.
> 
> Gregor Mi wrote:
> > To avoid this confusion, it should be possible to also click "End 
> Process..." even if no processes has been selected, whuch would then ask the 
> user to select the process to kill.
> 
> If "End Process" is clicked with no processes selected, there will be a 
> message box which says that the user has to select one more more processes 
> first.
> 
> > This could theoretically be done similarly to the "Kill specific 
> window" function: Click the "End Process..." button and then click the 
> process in the list that you want to end.
> 
> I think "Kill specific windows" should be considered as the special case 
> here. Changing or extending the "End Process" workflow would introduce more 
> complexity to the code.
> 
> > Alternatively, if no process had been selected when "End Process..." is 
> clicked, a dialog could be opened where the process to kill would be 
> selected. Of course the current flow of ending a p>rocess could and should 
> still work.
> 
> This would be a topic for another RR.
> 
> Summary of final changes for this RR:
> 
> - I would change the "End Process..." to "End Process" (remove ellipsis). 
> Ok?
> - I am not sure if the ellipsis of "Kill specific window..." should be 
> removed or not.
> 
> Thomas Pfeiffer wrote:
> To be honest: The more I think about this feature, the less sense it 
> makes to me in general.
> What is XKill's usecase anyway? If an application works normally, one can 
> just quit it normally. If an application is frozen, KWin quite reliably 
> detects that when trying to close the window and allows to kill the process.
> Usually "End process" is used for applications that do not have a window 
> (either because they don't have a GUI in the first place or because the 
> window has disappeared but the process is still there), in which case xkill 
> would not help anyway.
> Yes, there may be some situations where it's useful, but they are really 
> corner cases. Yes, very few people know it's there, but very few people miss 
> the function, either. I heve never wished there was a way to hard kill a 
> window without using the Close button in the windeco, and I have never met 
> one who did.
> 
> So we're complicating the GUI with a split button only to explain a 
> feature to users which the vast majority of them don't need in the first 
> place.
> 
> If I have missed the "killer usecase for xkill" which makes it necessary 
> to make it a lot more visible

Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

[Thomas Pfeiffer]

> On April 20, 2015, 10:36 p.m., Thomas Pfeiffer wrote:
> > What would likely be confusing is that the two button modes have different 
> > interaction flows: The "End Process" mode requires to first select a 
> > process and then press the button to work, whereas the "Kill specific 
> > window" mode requires to first press the button and then select the window 
> > to kill, and users have no easy way to understand how each one works and 
> > why they work differently.
> > 
> > The ellipsis in the label "End Process..." adds to that confusion. It 
> > indicates that further input is necessary before the action can take 
> > effect. While the button does open a dialog to confirm killing the selected 
> > process, ellipses are actually reserved for actions where a dialog asks for 
> > new information, such as the "Save As..." button, not for actions that 
> > require confirmation.
> > 
> > To avoid this confusion, it should be possible to also click "End 
> > Process..." even if no processes has been selected, whuch would then ask 
> > the user to select the process to kill. This could theoretically be done 
> > similarly to the "Kill specific window" function: Click the "End 
> > Process..." button and then click the process in the list that you want to 
> > end. Alternatively, if no process had been selected when "End Process..." 
> > is clicked, a dialog could be opened where the process to kill would be 
> > selected. Of course the current flow of ending a process could and should 
> > still work.
> 
> Gregor Mi wrote:
> Thanks for the feedback! The ellipsis in "End Process..." is the original 
> design. According to your explanation this was wrong in the first place. What 
> about removing the ellipses in both menu items so we will end up with "End 
> Process" and "Kill a specific window"?
> 
> Thomas Pfeiffer wrote:
> "Kill specific window" does always need additional input until it really 
> does something, doesn't it? As I understood it merely changes the cursor to 
> the kill cursor and then the user has to select the window to kill, right?
> 
> Gregor Mi wrote:
> Erm, right. To be exact, "Kill specific window..." only shows a message 
> box. But in the end - after the user presses the keyboard shortcut - the user 
> has to select a window. So this seems to be a special case. The intention 
> behind all this is to increase the discoverability of the hidden xkill 
> feature.
> 
> Gregor Mi wrote:
> > To avoid this confusion, it should be possible to also click "End 
> Process..." even if no processes has been selected, whuch would then ask the 
> user to select the process to kill.
> 
> If "End Process" is clicked with no processes selected, there will be a 
> message box which says that the user has to select one more more processes 
> first.
> 
> > This could theoretically be done similarly to the "Kill specific 
> window" function: Click the "End Process..." button and then click the 
> process in the list that you want to end.
> 
> I think "Kill specific windows" should be considered as the special case 
> here. Changing or extending the "End Process" workflow would introduce more 
> complexity to the code.
> 
> > Alternatively, if no process had been selected when "End Process..." is 
> clicked, a dialog could be opened where the process to kill would be 
> selected. Of course the current flow of ending a p>rocess could and should 
> still work.
> 
> This would be a topic for another RR.
> 
> Summary of final changes for this RR:
> 
> - I would change the "End Process..." to "End Process" (remove ellipsis). 
> Ok?
> - I am not sure if the ellipsis of "Kill specific window..." should be 
> removed or not.
> 
> Thomas Pfeiffer wrote:
> To be honest: The more I think about this feature, the less sense it 
> makes to me in general.
> What is XKill's usecase anyway? If an application works normally, one can 
> just quit it normally. If an application is frozen, KWin quite reliably 
> detects that when trying to close the window and allows to kill the process.
> Usually "End process" is used for applications that do not have a window 
> (either because they don't have a GUI in the first place or because the 
> window has disappeared but the process is still there), in which case xkill 
> would not help anyway.
> Yes, there may be some situations where it's useful, but they are really 
> corner cases. Yes, very few people know it's there, but very few people miss 
> the function, either. I heve never wished there was a way to hard kill a 
> window without using the Close button in the windeco, and I have never met 
> one who did.
> 
> So we're complicating the GUI with a split button only to explain a 
> feature to users which the vast majority of them don't need in the first 
> place.
> 
> If I have missed the "killer usecase for xkill" which makes it necessary 
> to make it a lot more visible

Re: Review Request 123806: [klipper] Ignore empty / blank entries

[Patrick Eigensatz]

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



klipper/klipper.kcfg (line 24)


**Question:** Compiled plasma-workspace using *kdesrc-build 
--include-dependencies plasma-workspace*, applied patch and compiled again.
-> Build broken because g++ could not
locate *klippersettings.h* included in *historyitem.cpp*. Shouldn't this 
file be generated by *cmake*?
If I generate *klippersettings{.h,.cpp}* using *kconfig_compiler 
klipper.kcfg klippersettings.kcfgc* and run *kdesrc* again,
I get a build error:

CMakeFiles/testHistory.dir/__/historyitem.cpp.o: In function 
`KlipperSettings::allowWhitespaceEntries()':
/home/patrick/kdesrc/plasma-workspace/klipper/klippersettings.h:72: 
undefined reference to `KlipperSettings::self()'

Note: The whole file is full of *return self()->member*, I don't know why 
it fails on line 72 if it worked
the for last 20 members. (I didn't change anything, original 
kconfig_compiler output)


- Patrick Eigensatz


On Mai 21, 2015, 5 nachm., Patrick Eigensatz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123806/
> ---
> 
> (Updated Mai 21, 2015, 5 nachm.)
> 
> 
> Review request for kde-workspace, KDE Usability and Patrick Eigensatz.
> 
> 
> Bugs: 159267 and 192922
> https://bugs.kde.org/show_bug.cgi?id=159267
> https://bugs.kde.org/show_bug.cgi?id=192922
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> [PATCH] plasma-workspace: klipper: Fix #192922 Ignore blank entries
> 
> QString::isEmpty() is used to check if the string only consists of whitespace 
> characters. If it does, the creation of the HistoryStringItem fails.
> 
> 
> Diffs
> -
> 
>   klipper/generalconfig.ui f513e9c 
>   klipper/historyitem.cpp 36cbe61 
>   klipper/klipper.h 6952b11 
>   klipper/klipper.cpp 798b49f 
>   klipper/klipper.kcfg a03dd16 
> 
> Diff: https://git.reviewboard.kde.org/r/123806/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Patrick Eigensatz
> 
>

Re: Review Request 123806: [klipper] Ignore empty / blank entries

[Patrick Eigensatz]

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

(Updated Mai 21, 2015, 5 nachm.)


Review request for kde-workspace, KDE Usability and Patrick Eigensatz.


Changes
---

Fixed some C++ compiling issues. (nullptr needs to be static_casted)


Bugs: 159267 and 192922
https://bugs.kde.org/show_bug.cgi?id=159267
https://bugs.kde.org/show_bug.cgi?id=192922


Repository: plasma-workspace


Description
---

[PATCH] plasma-workspace: klipper: Fix #192922 Ignore blank entries

QString::isEmpty() is used to check if the string only consists of whitespace 
characters. If it does, the creation of the HistoryStringItem fails.


Diffs (updated)
-

  klipper/generalconfig.ui f513e9c 
  klipper/historyitem.cpp 36cbe61 
  klipper/klipper.h 6952b11 
  klipper/klipper.cpp 798b49f 
  klipper/klipper.kcfg a03dd16 

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


Testing
---


Thanks,

Patrick Eigensatz

Re: Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut

[Gregor Mi]

> On April 20, 2015, 10:36 p.m., Thomas Pfeiffer wrote:
> > What would likely be confusing is that the two button modes have different 
> > interaction flows: The "End Process" mode requires to first select a 
> > process and then press the button to work, whereas the "Kill specific 
> > window" mode requires to first press the button and then select the window 
> > to kill, and users have no easy way to understand how each one works and 
> > why they work differently.
> > 
> > The ellipsis in the label "End Process..." adds to that confusion. It 
> > indicates that further input is necessary before the action can take 
> > effect. While the button does open a dialog to confirm killing the selected 
> > process, ellipses are actually reserved for actions where a dialog asks for 
> > new information, such as the "Save As..." button, not for actions that 
> > require confirmation.
> > 
> > To avoid this confusion, it should be possible to also click "End 
> > Process..." even if no processes has been selected, whuch would then ask 
> > the user to select the process to kill. This could theoretically be done 
> > similarly to the "Kill specific window" function: Click the "End 
> > Process..." button and then click the process in the list that you want to 
> > end. Alternatively, if no process had been selected when "End Process..." 
> > is clicked, a dialog could be opened where the process to kill would be 
> > selected. Of course the current flow of ending a process could and should 
> > still work.
> 
> Gregor Mi wrote:
> Thanks for the feedback! The ellipsis in "End Process..." is the original 
> design. According to your explanation this was wrong in the first place. What 
> about removing the ellipses in both menu items so we will end up with "End 
> Process" and "Kill a specific window"?
> 
> Thomas Pfeiffer wrote:
> "Kill specific window" does always need additional input until it really 
> does something, doesn't it? As I understood it merely changes the cursor to 
> the kill cursor and then the user has to select the window to kill, right?
> 
> Gregor Mi wrote:
> Erm, right. To be exact, "Kill specific window..." only shows a message 
> box. But in the end - after the user presses the keyboard shortcut - the user 
> has to select a window. So this seems to be a special case. The intention 
> behind all this is to increase the discoverability of the hidden xkill 
> feature.
> 
> Gregor Mi wrote:
> > To avoid this confusion, it should be possible to also click "End 
> Process..." even if no processes has been selected, whuch would then ask the 
> user to select the process to kill.
> 
> If "End Process" is clicked with no processes selected, there will be a 
> message box which says that the user has to select one more more processes 
> first.
> 
> > This could theoretically be done similarly to the "Kill specific 
> window" function: Click the "End Process..." button and then click the 
> process in the list that you want to end.
> 
> I think "Kill specific windows" should be considered as the special case 
> here. Changing or extending the "End Process" workflow would introduce more 
> complexity to the code.
> 
> > Alternatively, if no process had been selected when "End Process..." is 
> clicked, a dialog could be opened where the process to kill would be 
> selected. Of course the current flow of ending a p>rocess could and should 
> still work.
> 
> This would be a topic for another RR.
> 
> Summary of final changes for this RR:
> 
> - I would change the "End Process..." to "End Process" (remove ellipsis). 
> Ok?
> - I am not sure if the ellipsis of "Kill specific window..." should be 
> removed or not.
> 
> Thomas Pfeiffer wrote:
> To be honest: The more I think about this feature, the less sense it 
> makes to me in general.
> What is XKill's usecase anyway? If an application works normally, one can 
> just quit it normally. If an application is frozen, KWin quite reliably 
> detects that when trying to close the window and allows to kill the process.
> Usually "End process" is used for applications that do not have a window 
> (either because they don't have a GUI in the first place or because the 
> window has disappeared but the process is still there), in which case xkill 
> would not help anyway.
> Yes, there may be some situations where it's useful, but they are really 
> corner cases. Yes, very few people know it's there, but very few people miss 
> the function, either. I heve never wished there was a way to hard kill a 
> window without using the Close button in the windeco, and I have never met 
> one who did.
> 
> So we're complicating the GUI with a split button only to explain a 
> feature to users which the vast majority of them don't need in the first 
> place.
> 
> If I have missed the "killer usecase for xkill" which makes it necessary 
> to make it a lot more visible