Re: Review Request 123951: Change from GPL with KDE exception to LGPL to match the rest of the library

[Diane Trout]

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

(Updated June 2, 2015, 5:32 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and KDEPIM.


Changes
---

Submitted with commit 027f03a2d22d16f5e5abee6272573f1e1bbf7f00 by Diane Trout 
to branch master.


Repository: kpeople


Description
---

Change from GPL with KDE exception to LGPL to match the rest of the library.

Though since this is annoying legal stuff, and I changed David Edumndson's 
license block, I'm only going to push it this if he says "Ship It".


Diffs
-

  src/backends/allcontactsmonitor.h e02b3133590e15d644a1319f545d4b59df2909f0 
  src/backends/allcontactsmonitor.cpp bf3e6fc47f77d53277949d0e920acec2c3eef75a 

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


Testing
---

I only changed the license header so this shouldn't actually break anything.


Thanks,

Diane Trout

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

Re: Review Request 123929: If double-click is on empty area, then invoke back. (kfilewidgets)

[Thomas Pfeiffer]

> On May 29, 2015, 12:45 p.m., David Faure wrote:
> > I'm not sure this is a good idea, usability wise. Miss an icon by one 
> > pixel, and the effect is to go up, making the whole set of icons disappear?
> 
> Ashish Bansal wrote:
> Well about usability there's already discussion going on dolphin RR 
> 123658. Single click is the way to select a directory(atleast by default). 
> So, even if someone wants to select an directory and misses it, it won't go 
> up. Go up behaviour is only when one would use double click :)
> 
> David Faure wrote:
> Well, it would still be a huge problem for people using double-click mode 
> (e.g. my wife).
> 
> David Faure wrote:
> Maybe this feature should only be activated in single-click-mode.
> 
> Ashish Bansal wrote:
> That sounds good!
> 
> Arjun AK wrote:
> >Maybe this feature should only be activated in single-click-mode.
> 
> Are you sure thats a good idea? If i disable/enable single click mode, it 
> should only change that behaviour and not add/remove a completely unrelated 
> feature.
> 
> Martin Klapetek wrote:
> I agree with Arjun; plus if you change the behavior in one patch, the 
> other one must stay consistent (123658).
> 
> David Faure wrote:
> I don't agree that it's unrelated. If two features conflict, then we 
> shouldn't enable them together.
> 
> single-click mode = single-click to execute. double-click is not used.
> double-click mode = single-click to select, double-click to execute.
> 
> The only mode where double-click is currently unused is single-click mode.

This is a valid and correctly identified problem, but I do not agree with the 
proposed solution.
If there's a considerable chance that people may miss an icon they wanted to 
click, then we have to fix that, instead of working around the problem by 
changing the behavior of another feature depending on whether one selects by 
single or double click.

As far as I understood, this feature was meant to affect only truly empty 
areas, where there simply are no icons, not the space between two icons. That 
still leaves us with mis-clicks below the last row of icons, but with a big 
enough hit area, the chances for that should not be too big.

This is also something we could test, though: We could ask our beta testers (or 
better still, those using Kubuntu CI images) to test this feature out in both 
single- and double-click mode and report back problems, so we could refine the 
size of the hit areas until we can reduce mis-clicks to a low-enough rate. 
Mis-clicking is bad as it is, even if nothing happens.


- Thomas


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


On May 30, 2015, 12:42 p.m., Ashish Bansal wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123929/
> ---
> 
> (Updated May 30, 2015, 12:42 p.m.)
> 
> 
> Review request for Dolphin, KDE Frameworks, KDE Usability, David Faure, and 
> Martin Klapetek.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> If mouse double click is done on the empty area, then go to parent directory.
> 
> RR 123658 (dolphin)
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kdiroperator.h 6d601f3 
>   src/filewidgets/kdiroperator.cpp 4a7ef02 
> 
> Diff: https://git.reviewboard.kde.org/r/123929/diff/
> 
> 
> Testing
> ---
> 
> Works fine!
> 
> 
> Thanks,
> 
> Ashish Bansal
> 
>

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

Re: Review Request 123975: Fix crash in NotifyByExecute when no widget has been set

[Martin Klapetek]

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

(Updated June 1, 2015, 1:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit a999bc1c4087fbc5288873193af86b4f739116e3 by Martin 
Klapetek to branch master.


Bugs: 348510
https://bugs.kde.org/show_bug.cgi?id=348510


Repository: knotifications


Description
---

Additionally, it makes the debug output a bit more useful.


Diffs
-

  src/knotification.cpp 790ff3f 
  src/notifybyexecute.cpp b5d437a 

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


Testing
---


Thanks,

Martin Klapetek

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

Re: Review Request 123974: Add more symbols to convert-klineedit.pl

[Jarosław Staniek]

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

(Updated June 1, 2015, 1:10 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Aleix Pol Gonzalez and Laurent Montel.


Changes
---

Submitted with commit 64789d05c9b27a3ae2031738e516ec2d31ab39e4 by Jaroslaw 
Staniek to branch master.


Repository: kde-dev-scripts


Description
---

Add a property names/getters/signals to convert-klineedit.pl.


Diffs
-

  kf5/convert-klineedit.pl 33044d8e7812c8fcadedccc77dc6a200f8247bd1 

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


Testing
---


Thanks,

Jarosław Staniek

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

Re: Review Request 123975: Fix crash in NotifyByExecute when no widget has been set

[David Edmundson]

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

Ship it!


Ship It!

- David Edmundson


On June 1, 2015, 12:49 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123975/
> ---
> 
> (Updated June 1, 2015, 12:49 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 348510
> https://bugs.kde.org/show_bug.cgi?id=348510
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> Additionally, it makes the debug output a bit more useful.
> 
> 
> Diffs
> -
> 
>   src/knotification.cpp 790ff3f 
>   src/notifybyexecute.cpp b5d437a 
> 
> Diff: https://git.reviewboard.kde.org/r/123975/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 123974: Add more symbols to convert-klineedit.pl

[Laurent Montel]

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

Ship it!


Seems good for me :)
Ship it

- Laurent Montel


On juin 1, 2015, 12:27 après-midi, Jarosław Staniek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123974/
> ---
> 
> (Updated juin 1, 2015, 12:27 après-midi)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Laurent Montel.
> 
> 
> Repository: kde-dev-scripts
> 
> 
> Description
> ---
> 
> Add a property names/getters/signals to convert-klineedit.pl.
> 
> 
> Diffs
> -
> 
>   kf5/convert-klineedit.pl 33044d8e7812c8fcadedccc77dc6a200f8247bd1 
> 
> Diff: https://git.reviewboard.kde.org/r/123974/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarosław Staniek
> 
>

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

Re: Review Request 123975: Fix crash in NotifyByExecute when no widget has been set

[Martin Klapetek]

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

(Updated June 1, 2015, 2:49 p.m.)


Review request for KDE Frameworks.


Bugs: 348510
https://bugs.kde.org/show_bug.cgi?id=348510


Repository: knotifications


Description
---

Additionally, it makes the debug output a bit more useful.


Diffs (updated)
-

  src/knotification.cpp 790ff3f 
  src/notifybyexecute.cpp b5d437a 

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


Testing
---


Thanks,

Martin Klapetek

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

Re: Review Request 123975: Fix crash in NotifyByExecute when no widget has been set

[Martin Klapetek]

> On June 1, 2015, 2:40 p.m., David Edmundson wrote:
> > src/notifybyexecute.cpp, line 56
> > 
> >
> > should this have an 
> > 
> > else subst.insert("w", 0); ?
> > 
> > otherwise it won't be substituted out and the receiving process will 
> > get an unepected letter.

Good point, I'll fix.


- Martin


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


On June 1, 2015, 1:51 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123975/
> ---
> 
> (Updated June 1, 2015, 1:51 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 348510
> https://bugs.kde.org/show_bug.cgi?id=348510
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> Additionally, it makes the debug output a bit more useful.
> 
> 
> Diffs
> -
> 
>   src/knotification.cpp 790ff3f 
>   src/notifybyexecute.cpp b5d437a 
> 
> Diff: https://git.reviewboard.kde.org/r/123975/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 123975: Fix crash in NotifyByExecute when no widget has been set

[David Edmundson]

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



src/notifybyexecute.cpp (line 54)


should this have an 

else subst.insert("w", 0); ?

otherwise it won't be substituted out and the receiving process will get an 
unepected letter.


- David Edmundson


On June 1, 2015, 11:51 a.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123975/
> ---
> 
> (Updated June 1, 2015, 11:51 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 348510
> https://bugs.kde.org/show_bug.cgi?id=348510
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> Additionally, it makes the debug output a bit more useful.
> 
> 
> Diffs
> -
> 
>   src/knotification.cpp 790ff3f 
>   src/notifybyexecute.cpp b5d437a 
> 
> Diff: https://git.reviewboard.kde.org/r/123975/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Review Request 123975: Fix crash in NotifyByExecute when no widget has been set

[Martin Klapetek]

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

Review request for KDE Frameworks.


Bugs: 348510
https://bugs.kde.org/show_bug.cgi?id=348510


Repository: knotifications


Description
---

Additionally, it makes the debug output a bit more useful.


Diffs
-

  src/knotification.cpp 790ff3f 
  src/notifybyexecute.cpp b5d437a 

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


Testing
---


Thanks,

Martin Klapetek

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

Jenkins-kde-ci: ktexteditor master stable-kf5-qt5 » OSX,clang - Build # 8 - Failure!

[no-reply]

 
 Check console output at 
https://build.kde.org/job/ktexteditor%20master%20stable-kf5-qt5/PLATFORM=OSX,compiler=clang/8/
 to view the results. 
 
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 123974: Add more symbols to convert-klineedit.pl

[Jarosław Staniek]

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

Review request for KDE Frameworks.


Repository: kde-dev-scripts


Description
---

Add a property names/getters/signals to convert-klineedit.pl.


Diffs
-

  kf5/convert-klineedit.pl 33044d8e7812c8fcadedccc77dc6a200f8247bd1 

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


Testing
---


Thanks,

Jarosław Staniek

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

Re: Review Request 123929: If double-click is on empty area, then invoke back. (kfilewidgets)

[David Faure]

> On May 29, 2015, 12:45 p.m., David Faure wrote:
> > I'm not sure this is a good idea, usability wise. Miss an icon by one 
> > pixel, and the effect is to go up, making the whole set of icons disappear?
> 
> Ashish Bansal wrote:
> Well about usability there's already discussion going on dolphin RR 
> 123658. Single click is the way to select a directory(atleast by default). 
> So, even if someone wants to select an directory and misses it, it won't go 
> up. Go up behaviour is only when one would use double click :)
> 
> David Faure wrote:
> Well, it would still be a huge problem for people using double-click mode 
> (e.g. my wife).
> 
> David Faure wrote:
> Maybe this feature should only be activated in single-click-mode.
> 
> Ashish Bansal wrote:
> That sounds good!
> 
> Arjun AK wrote:
> >Maybe this feature should only be activated in single-click-mode.
> 
> Are you sure thats a good idea? If i disable/enable single click mode, it 
> should only change that behaviour and not add/remove a completely unrelated 
> feature.
> 
> Martin Klapetek wrote:
> I agree with Arjun; plus if you change the behavior in one patch, the 
> other one must stay consistent (123658).

I don't agree that it's unrelated. If two features conflict, then we shouldn't 
enable them together.

single-click mode = single-click to execute. double-click is not used.
double-click mode = single-click to select, double-click to execute.

The only mode where double-click is currently unused is single-click mode.


- David


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


On May 30, 2015, 12:42 p.m., Ashish Bansal wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123929/
> ---
> 
> (Updated May 30, 2015, 12:42 p.m.)
> 
> 
> Review request for Dolphin, KDE Frameworks, KDE Usability, David Faure, and 
> Martin Klapetek.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> If mouse double click is done on the empty area, then go to parent directory.
> 
> RR 123658 (dolphin)
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kdiroperator.h 6d601f3 
>   src/filewidgets/kdiroperator.cpp 4a7ef02 
> 
> Diff: https://git.reviewboard.kde.org/r/123929/diff/
> 
> 
> Testing
> ---
> 
> Works fine!
> 
> 
> Thanks,
> 
> Ashish Bansal
> 
>

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

Re: Review Request 123929: If double-click is on empty area, then invoke back. (kfilewidgets)

[Martin Klapetek]

> On May 29, 2015, 2:45 p.m., David Faure wrote:
> > I'm not sure this is a good idea, usability wise. Miss an icon by one 
> > pixel, and the effect is to go up, making the whole set of icons disappear?
> 
> Ashish Bansal wrote:
> Well about usability there's already discussion going on dolphin RR 
> 123658. Single click is the way to select a directory(atleast by default). 
> So, even if someone wants to select an directory and misses it, it won't go 
> up. Go up behaviour is only when one would use double click :)
> 
> David Faure wrote:
> Well, it would still be a huge problem for people using double-click mode 
> (e.g. my wife).
> 
> David Faure wrote:
> Maybe this feature should only be activated in single-click-mode.
> 
> Ashish Bansal wrote:
> That sounds good!
> 
> Arjun AK wrote:
> >Maybe this feature should only be activated in single-click-mode.
> 
> Are you sure thats a good idea? If i disable/enable single click mode, it 
> should only change that behaviour and not add/remove a completely unrelated 
> feature.

I agree with Arjun; plus if you change the behavior in one patch, the other one 
must stay consistent (123658).


- Martin


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


On May 30, 2015, 2:42 p.m., Ashish Bansal wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123929/
> ---
> 
> (Updated May 30, 2015, 2:42 p.m.)
> 
> 
> Review request for Dolphin, KDE Frameworks, KDE Usability, David Faure, and 
> Martin Klapetek.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> If mouse double click is done on the empty area, then go to parent directory.
> 
> RR 123658 (dolphin)
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kdiroperator.h 6d601f3 
>   src/filewidgets/kdiroperator.cpp 4a7ef02 
> 
> Diff: https://git.reviewboard.kde.org/r/123929/diff/
> 
> 
> Testing
> ---
> 
> Works fine!
> 
> 
> Thanks,
> 
> Ashish Bansal
> 
>

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

Re: [knewstuff] src/kmoretools: Use pimpl for KMoreToolsMenuFactory

[David Faure]

On Monday 01 June 2015 10:22:19 Gregor Mi wrote:
> Am 01.06.2015 09:48 schrieb David Faure:
> > On Thursday 21 May 2015 14:41:27 Gregor Mi wrote:
> >>  private:
> >> -// Note that this object must live long enough in case the user
> >> opens
> >> -// the "Configure..." dialog
> >> -KMoreTools* m_kmt = nullptr;
> >> -
> >> -QMenu* m_menu = nullptr;
> >> +KMoreToolsMenuFactoryPrivate* d;
> >> 
> >>  };
> > 
> > Err, are you changing the size of a public class after it got released?
> > That would be BIC.
> 
> This should be reverted then. I'll do it. What would be good procedure
> to introduce this kind of change?

If "this kind of" means a BIC, there isn't, you can't make any BICs ;)

However you could replace *one* of the two pointers with a d pointer, thereby 
preserving the object size.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

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

Re: [knewstuff] src/kmoretools: Use pimpl for KMoreToolsMenuFactory

[David Faure]

On Thursday 21 May 2015 14:41:27 Gregor Mi wrote:
>  private:
> -// Note that this object must live long enough in case the user opens
> -// the "Configure..." dialog
> -KMoreTools* m_kmt = nullptr;
> -
> -QMenu* m_menu = nullptr;
> +KMoreToolsMenuFactoryPrivate* d;
>  };
>  

Err, are you changing the size of a public class after it got released?
That would be BIC.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

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