Re: Review Request 127488: Fix usage of std::isprint() function

[Aleix Pol Gonzalez]

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



Could we get a test for this?

- Aleix Pol Gonzalez


On March 24, 2016, 10:06 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127488/
> ---
> 
> (Updated March 24, 2016, 10:06 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 357341
> https://bugs.kde.org/show_bug.cgi?id=357341
> 
> 
> Repository: kcodecs
> 
> 
> Description
> ---
> 
> With the patch in review request https://git.reviewboard.kde.org/r/127486/ I 
> get another crash when loading the attached file in bug 
> https://bugs.kde.org/show_bug.cgi?id=360797
> 
> The issue is the call if isprint(int): Accoring to 
> http://en.cppreference.com/w/cpp/string/byte/isprint
> "the behavior is undefined if the value of ch is not representable as 
> unsigned char and is not equal to EOF."
> 
> In this case, casting the char to an int results "-31", which is clearly not 
> representable as *unsigned* char. Hence, we indeed hit the "undefined 
> behavior" and get a crash.
> 
> This patch therefore
> - casts the char to an int first
> - and then checks whether the int it non-negative
> - only then, isprint() is called.
> 
> 
> Diffs
> -
> 
>   src/probers/UnicodeGroupProber.cpp 1d86f08 
> 
> Diff: https://git.reviewboard.kde.org/r/127488/diff/
> 
> 
> Testing
> ---
> 
> Kate/Kwrite launch without crash.
> Unit tests still pass.
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

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

Re: Review Request 127486: Fix uninitialized int array in UnicodeGroupProber

[Aleix Pol Gonzalez]

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




src/probers/UnicodeGroupProber.cpp (line 78)


`(uint)` shouldn't be necessary, no? Otherwise:
`const uint weight_BOM(sqrt((double)aLen) + aLen / 10.0);`

Or even easier:
`const uint weight_BOM = aLen>>1 + aLen / 10;`



src/probers/UnicodeGroupProber.cpp (line 79)


This used to be `uint`.


- Aleix Pol Gonzalez


On March 24, 2016, 9:45 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127486/
> ---
> 
> (Updated March 24, 2016, 9:45 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 357341
> https://bugs.kde.org/show_bug.cgi?id=357341
> 
> 
> Repository: kcodecs
> 
> 
> Description
> ---
> 
> When loading the file from bug https://bugs.kde.org/show_bug.cgi?id=360797 
> with kwrite one gets a segfault. Valgrind tells us 'count' is uninitialized 
> (see line 83).
> 
> This patch
> - moves variables to local scopes
> - initializes the variable count with 5 times 0.
> 
> The backtrace in bug https://bugs.kde.org/show_bug.cgi?id=357341 is fixed 
> this way. However, there is a second crash still happening, which still needs 
> investigation.
> 
> 
> Diffs
> -
> 
>   src/probers/UnicodeGroupProber.cpp 1d86f08 
> 
> Diff: https://git.reviewboard.kde.org/r/127486/diff/
> 
> 
> Testing
> ---
> 
> - unit tests still pass
> - The encoding prober does not crash at the location anymore - it crashes a 
> bit later (another issue)
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

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

Re: Raising Qt requirement to Qt 5.4

[Albert Astals Cid]

El diumenge, 20 de març de 2016, a les 10:12:14 CET, David Faure va escriure:
> On Friday 18 March 2016 20:16:05 Ivan Čukić wrote:
> > Hi all,
> > 
> > IIRC, the last time we talked about this we decided (I'll try to find
> > the e-mail thread when I get back home, IIRC the conclusion was by
> > dfaure) on KF5 always support the two last versions of Qt. This was
> > decided so that we do not need to have threads like these every time
> > we want to increase the Qt version.
> 
> *Three* last versions.
> 
> Which is why we can drop 5.3 now that 5.6 is out
>  => we support 5.4, 5.5 and 5.6.

Is this documented in some wiki so we don't have to rely on people's memories 
or someone digging up the corresponding email thread?

Cheers,
  Albert

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

Re: Review Request 127488: Fix usage of std::isprint() function

[Dominik Haumann]

> On March 24, 2016, 9:11 p.m., Dominik Haumann wrote:
> > Btw: If you think QChar::isPrint() is a better choice [1], I'm happy to 
> > update the review request accordingly.
> > 
> > [1] http://doc.qt.io/qt-5/qchar.html#isPrint-1

Oh, and for a correct review, the isprint() comes from 
https://lxr.kde.org/source/frameworks/kcodecs/src/probers/ctype_test_p.h
This, however, just shows again that -31 is used as index in an array.


- Dominik


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


On March 24, 2016, 9:06 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127488/
> ---
> 
> (Updated March 24, 2016, 9:06 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 357341
> https://bugs.kde.org/show_bug.cgi?id=357341
> 
> 
> Repository: kcodecs
> 
> 
> Description
> ---
> 
> With the patch in review request https://git.reviewboard.kde.org/r/127486/ I 
> get another crash when loading the attached file in bug 
> https://bugs.kde.org/show_bug.cgi?id=360797
> 
> The issue is the call if isprint(int): Accoring to 
> http://en.cppreference.com/w/cpp/string/byte/isprint
> "the behavior is undefined if the value of ch is not representable as 
> unsigned char and is not equal to EOF."
> 
> In this case, casting the char to an int results "-31", which is clearly not 
> representable as *unsigned* char. Hence, we indeed hit the "undefined 
> behavior" and get a crash.
> 
> This patch therefore
> - casts the char to an int first
> - and then checks whether the int it non-negative
> - only then, isprint() is called.
> 
> 
> Diffs
> -
> 
>   src/probers/UnicodeGroupProber.cpp 1d86f08 
> 
> Diff: https://git.reviewboard.kde.org/r/127488/diff/
> 
> 
> Testing
> ---
> 
> Kate/Kwrite launch without crash.
> Unit tests still pass.
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

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

Re: Review Request 127488: Fix usage of std::isprint() function

[Dominik Haumann]

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



Btw: If you think QChar::isPrint() is a better choice [1], I'm happy to update 
the review request accordingly.

[1] http://doc.qt.io/qt-5/qchar.html#isPrint-1

- Dominik Haumann


On March 24, 2016, 9:06 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127488/
> ---
> 
> (Updated March 24, 2016, 9:06 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 357341
> https://bugs.kde.org/show_bug.cgi?id=357341
> 
> 
> Repository: kcodecs
> 
> 
> Description
> ---
> 
> With the patch in review request https://git.reviewboard.kde.org/r/127486/ I 
> get another crash when loading the attached file in bug 
> https://bugs.kde.org/show_bug.cgi?id=360797
> 
> The issue is the call if isprint(int): Accoring to 
> http://en.cppreference.com/w/cpp/string/byte/isprint
> "the behavior is undefined if the value of ch is not representable as 
> unsigned char and is not equal to EOF."
> 
> In this case, casting the char to an int results "-31", which is clearly not 
> representable as *unsigned* char. Hence, we indeed hit the "undefined 
> behavior" and get a crash.
> 
> This patch therefore
> - casts the char to an int first
> - and then checks whether the int it non-negative
> - only then, isprint() is called.
> 
> 
> Diffs
> -
> 
>   src/probers/UnicodeGroupProber.cpp 1d86f08 
> 
> Diff: https://git.reviewboard.kde.org/r/127488/diff/
> 
> 
> Testing
> ---
> 
> Kate/Kwrite launch without crash.
> Unit tests still pass.
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

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

Review Request 127488: Fix usage of std::isprint() function

[Dominik Haumann]

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

Review request for KDE Frameworks.


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


Repository: kcodecs


Description
---

With the patch in review request https://git.reviewboard.kde.org/r/127486/ I 
get another crash when loading the attached file in bug 
https://bugs.kde.org/show_bug.cgi?id=360797

The issue is the call if isprint(int): Accoring to 
http://en.cppreference.com/w/cpp/string/byte/isprint
"the behavior is undefined if the value of ch is not representable as unsigned 
char and is not equal to EOF."

In this case, casting the char to an int results "-31", which is clearly not 
representable as *unsigned* char. Hence, we indeed hit the "undefined behavior" 
and get a crash.

This patch therefore
- casts the char to an int first
- and then checks whether the int it non-negative
- only then, isprint() is called.


Diffs
-

  src/probers/UnicodeGroupProber.cpp 1d86f08 

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


Testing
---

Kate/Kwrite launch without crash.
Unit tests still pass.


Thanks,

Dominik Haumann

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

Review Request 127486: Fix uninitialized int array in UnicodeGroupProber

[Dominik Haumann]

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

Review request for KDE Frameworks.


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


Repository: kcodecs


Description
---

When loading the file from bug https://bugs.kde.org/show_bug.cgi?id=360797 with 
kwrite one gets a segfault. Valgrind tells us 'count' is uninitialized (see 
line 83).

This patch
- moves variables to local scopes
- initializes the variable count with 5 times 0.

The backtrace in bug https://bugs.kde.org/show_bug.cgi?id=357341 is fixed this 
way. However, there is a second crash still happening, which still needs 
investigation.


Diffs
-

  src/probers/UnicodeGroupProber.cpp 1d86f08 

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


Testing
---

- unit tests still pass
- The encoding prober does not crash at the location anymore - it crashes a bit 
later (another issue)


Thanks,

Dominik Haumann

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

Re: Review Request 127424: KCompletionBox popup gets full window decoration on Windows

[Dominik Haumann]

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



Given the feedback I got, I will submit tomorrow (Friday), independent on 
whether I have a ship-it or not.
The patch is tested both on Windows and Linux, and imo correct (the changes are 
rather trivial now).

If you have objections, please raise them by tomorrow, or better yet, do a 
review - thanks.

PS: I am totally aware that typically a ship-it is the way to get a patch 
accepted, but I have the feeling this patch is not getting much attention, 
despite me trying to get input for days.

- Dominik Haumann


On March 20, 2016, 12:06 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127424/
> ---
> 
> (Updated March 20, 2016, 12:06 p.m.)
> 
> 
> Review request for KDE Frameworks, kdewin, kwin, and Marco Martin.
> 
> 
> Repository: kcompletion
> 
> 
> Description
> ---
> 
> In https://git.reviewboard.kde.org/r/127191/ the KCompletionBox WindowFlags 
> were change from Qt::ToolTip to Qt::Window.
> 
> As consequence, the completion popup of the Kate command line gets a full 
> window decoration, which is obviously wrong, see attached screenshot.
> 
> Changing the type to Qt::Popup shows the proper popup, but key presses are 
> not forwarded, so typing is not possible, and additionally, on losing focus 
> the popup keeps staying open.
> Therefore, this patch reverts the type back to Qt::ToolTip.
> 
> Better fixes are of course welcome ;) Any ideas?
> 
> 
> Diffs
> -
> 
>   src/kcompletionbox.cpp 005aff8 
> 
> Diff: https://git.reviewboard.kde.org/r/127424/diff/
> 
> 
> Testing
> ---
> 
> Works on Windows as expected.
> 
> 
> File Attachments
> 
> 
> Completion Popup
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/03/19/7be64cad-6d95-46b8-9caa-41b41a135ca1__kate2015.png
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

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

Re: Review Request 127409: Make sure we're not forcing the window to be created prematurely

[Kai Uwe Broulik]

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




src/plasmaquick/dialog.cpp (lines 1000 - 1001)


Braces pls


- Kai Uwe Broulik


On März 17, 2016, 2:13 vorm., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127409/
> ---
> 
> (Updated März 17, 2016, 2:13 vorm.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Only update the properties that require a winId when visible, otherwise let 
> updateVisibility() take care about them.
> 
> 
> Diffs
> -
> 
>   src/plasmaquick/dialog.cpp 7bb63c5 
> 
> Diff: https://git.reviewboard.kde.org/r/127409/diff/
> 
> 
> Testing
> ---
> 
> I don't get platform creations being forced on gdb.
> 
> 
> 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 127462: Add support for XDG_*_HOME enviroment variables.

[Sandro Knauß]

> On März 24, 2016, 9:15 vorm., Jarosław Staniek wrote:
> > My thoughts mostly related to non-Plasma, non-XDG run environments.
> > 
> > KF5 is a great addition, in hierarchy often sitting aside of Qt, not a part 
> > of XDG-compliant desktop. As such we can't say too much about the 
> > underlying OS. It may be XDG-compliant but athis can be a decission at 
> > various levels: by application developer or on deployment. Things can of 
> > course also change after the binary is compiled.
> > 
> > It's hard to see any benefit that apps 'pretend' being XDG-compliant by 
> > setting vartiables this way on, say, MS Windows or Mac. Qt applications on 
> > these systems do not traditionally alter the OS' paradigm, they are.. just 
> > apps, as native as possible/reasonable. Example: there are portable apps 
> > (on an USB stick) - I believe we can benefit if we help making them. These 
> > apps choose not to integrate too much with what's found on the system, 
> > often the user account is shared or temporary.
> > 
> > Setting the variable makes KConfig closer to specific groups of desktop, 
> > which is a a small step back. People may be interested in KConfig lib as a 
> > general purpose utility in their apps.
> > I'd compare setting the variable this way to setting Windows' USERPROFILE 
> > variable on Linux systems, just because it's standard under the former and 
> > under Wine. it does not hurt but leaves bad taste.
> > 
> > So the behaviour at least shall be ifdef'd and the ifdef shall depend on OS 
> > for which we're building. But if the same binary should behave well in 
> > various environments, I am unsure about hardcoding the behaviour. Also I am 
> > not sure if this behaviour should be ON by default. I'd rather have it OFF 
> > if it's present at all.

Well I see it the other way around. Without support of XDG variables 
replacement, we need to use env varaible that are available on all systems 
f.ex. $HOME, so the only way to set the datalocaltion to write it hardcoded in 
the config to $HOME/.local/share. This is really ony true for linux for others 
OSes you want to have other diretories. So now we are able to use XDG_DATA_HOME 
to be used in kconfig and it will be replaced for linux to $HOME/.local/share, 
for windows to C:/Users//AppData/Local and so on.
We use QStandardsPaths for replacing so if the replacement is not correct for 
an OS this is a bug on Qt side, that should be fixed :D

And I can't see how this support can harm anyone - if the env is set this is 
prefered and only if it is not set it is replaced. No dependecy is added and 
applications are free to not use it at all. If you use XDG_*_HOME without this 
patch you will return an empty string for it:

i[$e] = $XDG_DATA_HOME/bla

-> will return /bla

with the patch you will get:
$HOME/.local/share/bla ( for linux)
C:/Users//AppData/Local/bla ( for windows)
[...]


- Sandro


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


On März 22, 2016, 3:23 nachm., Sandro Knauß wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127462/
> ---
> 
> (Updated März 22, 2016, 3:23 nachm.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> According to freedesktop specification XDG_*_HOME env varaible should be
> replaced, if they are not setted with default values.
> 
> https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
> 
> as qgetenv only calls getenv, so no path is traslated to it default values. 
> So we have to add this replacement manually. This would help to use 
> XDG_*_HOME more often in configfiles.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp e92197f3be57ead47b70ca5d040474e7a554c416 
>   src/core/kconfig.cpp 07fa6f552c61c52cc1dd64a1c5fb0e2f00873d50 
> 
> Diff: https://git.reviewboard.kde.org/r/127462/diff/
> 
> 
> Testing
> ---
> 
> Adding tests for XDG_*_HOME variables.
> 
> 
> Thanks,
> 
> Sandro Knauß
> 
>

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

Re: Review Request 127462: Add support for XDG_*_HOME enviroment variables.

[Sandro Knauß]

> On März 24, 2016, 8:16 vorm., David Faure wrote:
> > autotests/kconfigtest.cpp, line 536
> > 
> >
> > Why? On my system this wouldn't pass.
> > If you want to make sure it's empty, use qunsetenv.

Okay I mostly written this, to get a first impression if the patch will enter 
kconfig.
for sure we need a test with XDG env set to a value, and one where XDG is not 
set. To test both ways.


> On März 24, 2016, 8:16 vorm., David Faure wrote:
> > autotests/kconfigtest.cpp, line 538
> > 
> >
> > writableLocaion, not displayName.
> > 
> > This shows the limitations of a unittest that uses the same code in the 
> > code and in the test. If both are wrong, the test still passes :-)

Do you have a better solution for getting away from "a unittest that uses the 
same code" in this case?


- Sandro


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


On März 22, 2016, 3:23 nachm., Sandro Knauß wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127462/
> ---
> 
> (Updated März 22, 2016, 3:23 nachm.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> According to freedesktop specification XDG_*_HOME env varaible should be
> replaced, if they are not setted with default values.
> 
> https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
> 
> as qgetenv only calls getenv, so no path is traslated to it default values. 
> So we have to add this replacement manually. This would help to use 
> XDG_*_HOME more often in configfiles.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp e92197f3be57ead47b70ca5d040474e7a554c416 
>   src/core/kconfig.cpp 07fa6f552c61c52cc1dd64a1c5fb0e2f00873d50 
> 
> Diff: https://git.reviewboard.kde.org/r/127462/diff/
> 
> 
> Testing
> ---
> 
> Adding tests for XDG_*_HOME variables.
> 
> 
> Thanks,
> 
> Sandro Knauß
> 
>

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

Re: Review Request 127462: Add support for XDG_*_HOME enviroment variables.

[Jarosław Staniek]

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



My thoughts mostly related to non-Plasma, non-XDG run environments.

KF5 is a great addition, in hierarchy often sitting aside of Qt, not a part of 
XDG-compliant desktop. As such we can't say too much about the underlying OS. 
It may be XDG-compliant but athis can be a decission at various levels: by 
application developer or on deployment. Things can of course also change after 
the binary is compiled.

It's hard to see any benefit that apps 'pretend' being XDG-compliant by setting 
vartiables this way on, say, MS Windows or Mac. Qt applications on these 
systems do not traditionally alter the OS' paradigm, they are.. just apps, as 
native as possible/reasonable. Example: there are portable apps (on an USB 
stick) - I believe we can benefit if we help making them. These apps choose not 
to integrate too much with what's found on the system, often the user account 
is shared or temporary.

Setting the variable makes KConfig closer to specific groups of desktop, which 
is a a small step back. People may be interested in KConfig lib as a general 
purpose utility in their apps.
I'd compare setting the variable this way to setting Windows' USERPROFILE 
variable on Linux systems, just because it's standard under the former and 
under Wine. it does not hurt but leaves bad taste.

So the behaviour at least shall be ifdef'd and the ifdef shall depend on OS for 
which we're building. But if the same binary should behave well in various 
environments, I am unsure about hardcoding the behaviour. Also I am not sure if 
this behaviour should be ON by default. I'd rather have it OFF if it's present 
at all.

- Jarosław Staniek


On March 22, 2016, 4:23 p.m., Sandro Knauß wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127462/
> ---
> 
> (Updated March 22, 2016, 4:23 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> According to freedesktop specification XDG_*_HOME env varaible should be
> replaced, if they are not setted with default values.
> 
> https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
> 
> as qgetenv only calls getenv, so no path is traslated to it default values. 
> So we have to add this replacement manually. This would help to use 
> XDG_*_HOME more often in configfiles.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp e92197f3be57ead47b70ca5d040474e7a554c416 
>   src/core/kconfig.cpp 07fa6f552c61c52cc1dd64a1c5fb0e2f00873d50 
> 
> Diff: https://git.reviewboard.kde.org/r/127462/diff/
> 
> 
> Testing
> ---
> 
> Adding tests for XDG_*_HOME variables.
> 
> 
> Thanks,
> 
> Sandro Knauß
> 
>

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

Jenkins-kde-ci: kio master kf5-qt5 » Linux,gcc - Build # 2 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/2/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Thu, 24 Mar 2016 08:18:45 +
Build duration: 5 min 29 sec

CHANGE SET
Revision d6eadae8337e80f1bb8f06ce92cb4a77d67d00cb by David Faure: (GIT_SILENT 
remove C casts)
  change: edit src/widgets/kurlrequester.cpp
Revision 6cc690e03801a6fb1eba84ee27c2efdbad86704e by David Faure: ((autotests) 
KUrlCompletionTest: wait on the right KUrlCompletion)
  change: edit autotests/kurlcompletiontest.cpp
Revision 41359f9840973960295cb2ab80566b267cde68ac by David Faure: (move 
ECMQtDeclareLoggingCategory up so it's useful in all subdirs)
  change: edit CMakeLists.txt
  change: edit src/gui/CMakeLists.txt
Revision 15fd9b17eb2f4d686891d2364e698e40260f9131 by David Faure: (Fix many 
threading issues in KUrlCompletion)
  change: edit src/widgets/kurlcompletion.h
  change: edit autotests/kurlcompletiontest.cpp
  change: edit src/widgets/CMakeLists.txt
  change: edit autotests/CMakeLists.txt
  change: edit src/widgets/kurlcompletion.cpp


JUNIT RESULTS

Name: (root) Failed: 2 test(s), Passed: 48 test(s), Skipped: 0 test(s), Total: 
50 test(s)Failed: TestSuite.kiocore-kfileitemtestFailed: 
TestSuite.kiowidgets-kurifiltertest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 260/338 (77%)CLASSES 260/338 (77%)LINE 27023/50925 
(53%)CONDITIONAL 14764/22911 (64%)

By packages
  
autotests
FILES 65/65 (100%)CLASSES 65/65 (100%)LINE 7479/7768 
(96%)CONDITIONAL 4098/7514 (55%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 538/540 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 182/199 (91%)CONDITIONAL 
60/90 (67%)
src.core
FILES 95/117 (81%)CLASSES 95/117 (81%)LINE 7842/14146 
(55%)CONDITIONAL 4099/5688 (72%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 20/36 (56%)CLASSES 20/36 (56%)LINE 2281/7591 
(30%)CONDITIONAL 911/1403 (65%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 108/114 (95%)CONDITIONAL 
46/68 (68%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 430/844 (51%)CONDITIONAL 
313/459 (68%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1847/3852 
(48%)CONDITIONAL 1321/1954 (68%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 642/804 (80%)CONDITIONAL 
602/758 (79%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 684/1183 (58%)CONDITIONAL 
358/495 (72%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 690/768 (90%)CONDITIONAL 
433/820 (53%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/26 (54%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 359/385 (93%)CONDITIONAL 
102/126 (81%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 384/600 (64%)CONDITIONAL 
280/408 (69%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 280/285 (98%)CONDITIONAL 
144/254 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 32/35 (91%)CONDITIONAL 
42/52 (81%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 248/743 (33%)CONDITIONAL 
147/194 (76%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 20/27 (74%)CONDITIONAL 
14/18 (78%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 214/259 (83%)CONDITIONAL 
282/346 (82%)
src.widgets
FILES 29/62 (47%)CLASSES 29/62 (47%)LINE 2714/10663 
(25%)CONDITIONAL 1304/1912 (68%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kio master stable-kf5-qt5 » Linux,gcc - Build # 2 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/kio%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/2/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Thu, 24 Mar 2016 08:18:45 +
Build duration: 5 min 56 sec

CHANGE SET
Revision d6eadae8337e80f1bb8f06ce92cb4a77d67d00cb by David Faure: (GIT_SILENT 
remove C casts)
  change: edit src/widgets/kurlrequester.cpp
Revision 6cc690e03801a6fb1eba84ee27c2efdbad86704e by David Faure: ((autotests) 
KUrlCompletionTest: wait on the right KUrlCompletion)
  change: edit autotests/kurlcompletiontest.cpp
Revision 41359f9840973960295cb2ab80566b267cde68ac by David Faure: (move 
ECMQtDeclareLoggingCategory up so it's useful in all subdirs)
  change: edit src/gui/CMakeLists.txt
  change: edit CMakeLists.txt
Revision 15fd9b17eb2f4d686891d2364e698e40260f9131 by David Faure: (Fix many 
threading issues in KUrlCompletion)
  change: edit src/widgets/kurlcompletion.cpp
  change: edit autotests/CMakeLists.txt
  change: edit src/widgets/CMakeLists.txt
  change: edit autotests/kurlcompletiontest.cpp
  change: edit src/widgets/kurlcompletion.h


JUNIT RESULTS

Name: (root) Failed: 2 test(s), Passed: 48 test(s), Skipped: 0 test(s), Total: 
50 test(s)Failed: TestSuite.kiocore-kfileitemtestFailed: 
TestSuite.kiowidgets-kurifiltertest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 21/21 (100%)FILES 260/338 (77%)CLASSES 260/338 (77%)LINE 27020/50927 
(53%)CONDITIONAL 14767/22917 (64%)

By packages
  
autotests
FILES 65/65 (100%)CLASSES 65/65 (100%)LINE 7481/7770 
(96%)CONDITIONAL 4100/7514 (55%)
autotests.http
FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 538/540 
(100%)CONDITIONAL 200/336 (60%)
autotests.kcookiejar
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 182/199 (91%)CONDITIONAL 
60/90 (67%)
src.core
FILES 95/117 (81%)CLASSES 95/117 (81%)LINE 7835/14146 
(55%)CONDITIONAL 4097/5686 (72%)
src.core.kssl
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 
3/6 (50%)
src.filewidgets
FILES 20/36 (56%)CLASSES 20/36 (56%)LINE 2281/7591 
(30%)CONDITIONAL 911/1403 (65%)
src.gui
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 108/114 (95%)CONDITIONAL 
46/68 (68%)
src.ioslaves.file
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 430/844 (51%)CONDITIONAL 
313/459 (68%)
src.ioslaves.http
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1849/3852 
(48%)CONDITIONAL 1324/1962 (67%)
src.ioslaves.http.kcookiejar
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 642/804 (80%)CONDITIONAL 
602/758 (79%)
src.ioslaves.trash
FILES 7/9 (78%)CLASSES 7/9 (78%)LINE 684/1183 (58%)CONDITIONAL 
358/495 (72%)
src.ioslaves.trash.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 690/768 (90%)CONDITIONAL 
433/820 (53%)
src.kioslave
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/26 (54%)CONDITIONAL 
5/10 (50%)
src.kntlm
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 359/385 (93%)CONDITIONAL 
102/126 (81%)
src.kpasswdserver
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 384/600 (64%)CONDITIONAL 
280/408 (69%)
src.kpasswdserver.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 280/285 (98%)CONDITIONAL 
144/254 (57%)
src.urifilters.fixhost
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 32/35 (91%)CONDITIONAL 
42/52 (81%)
src.urifilters.ikws
FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 248/743 (33%)CONDITIONAL 
147/194 (76%)
src.urifilters.localdomain
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 20/27 (74%)CONDITIONAL 
14/18 (78%)
src.urifilters.shorturi
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 214/259 (83%)CONDITIONAL 
282/346 (82%)
src.widgets
FILES 29/62 (47%)CLASSES 29/62 (47%)LINE 2714/10663 
(25%)CONDITIONAL 1304/1912 (68%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127437: Fix many threading issues in KUrlCompletion

[David Faure]

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

(Updated March 24, 2016, 8:18 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 15fd9b17eb2f4d686891d2364e698e40260f9131 by David Faure 
to branch master.


Repository: kio


Description
---

* Major race with other threads due to using QDir::setCurrent()
* Race conditions on m_terminationRequested and m_matches
* Race with previous completion thread when its posted event arrives after 
cancelling
* Cancellation code spread out in many methods and never done fully correctly
* isRunning() was missing one of the two threads, making unit test fail in 
valgrind
* Fix the rarely-hit code path where the thread isn't finished after 200ms
   - the current search string was lost because finished() wasn't called
   - the matches were not used, in case of user-completion (AFAICS)
   - changing the search string while the thread was running could lead to the 
old search
 string still being used for completion
 (the misnamed finished() wasn't called, so KCompletion didn't get the new 
string)
  => added a variant of the unittest which doesn't wait for the thread initially

+ Simplify code using signal/slots rather than a custom event
+ Simplify code using enum rather than casting to/from int

Most of these bugs are from 2004 (e.g. commit ec83c408619e3) ;)

REVIEW: 127437


Diffs
-

  autotests/CMakeLists.txt 4dff0a24d31897a9641a70017bd87e33415cef14 
  autotests/kurlcompletiontest.cpp eef39ff17940fadcb9492e5e7092070c976310d4 
  src/widgets/CMakeLists.txt 87dac50b377f6ea4c3cd39e9afa37a4680aecf31 
  src/widgets/kurlcompletion.h 14fda22a0e08bcf2da30e19fed577c8bda64a4ca 
  src/widgets/kurlcompletion.cpp 1dc8f4898fb78d6f49e687462446007a10305f98 

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


Testing
---

Hit a crash in DirectoryListThread when playing with kopenwithtest.

kurlcompletiontest extended, kurlcompletion-nowait added, both pass, also in 
valgrind (different timings).


Thanks,

David Faure

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

Re: Review Request 127462: Add support for XDG_*_HOME enviroment variables.

[David Faure]

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



It's a bit of a stretch to say that the XDG base dir spec says that env vars in 
config files and desktop files should work even if they are not set :)
First, because support for env vars in desktop files ([$e]) is a KDE extension 
IIRC, and secondly because the base dir spec only gives default values for 
these locations, while [$e] was strictly supposed to be about env var expansion.
This being said, I see no harm in extending that to support the default values 
as if they were set in the env var. Just the commit log should be a bit more 
like "add a feature" than "the spec tells us to do this".


autotests/kconfigtest.cpp (line 511)


noeol was supposed to be the line with no EOL. Leave it as the last 
position.



autotests/kconfigtest.cpp (line 536)


Why? On my system this wouldn't pass.
If you want to make sure it's empty, use qunsetenv.



autotests/kconfigtest.cpp (line 538)


writableLocaion, not displayName.

This shows the limitations of a unittest that uses the same code in the 
code and in the test. If both are wrong, the test still passes :-)



src/core/kconfig.cpp (line 226)


displayName?? You want writableLocation here.


- David Faure


On March 22, 2016, 3:23 p.m., Sandro Knauß wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127462/
> ---
> 
> (Updated March 22, 2016, 3:23 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> According to freedesktop specification XDG_*_HOME env varaible should be
> replaced, if they are not setted with default values.
> 
> https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
> 
> as qgetenv only calls getenv, so no path is traslated to it default values. 
> So we have to add this replacement manually. This would help to use 
> XDG_*_HOME more often in configfiles.
> 
> 
> Diffs
> -
> 
>   autotests/kconfigtest.cpp e92197f3be57ead47b70ca5d040474e7a554c416 
>   src/core/kconfig.cpp 07fa6f552c61c52cc1dd64a1c5fb0e2f00873d50 
> 
> Diff: https://git.reviewboard.kde.org/r/127462/diff/
> 
> 
> Testing
> ---
> 
> Adding tests for XDG_*_HOME variables.
> 
> 
> Thanks,
> 
> Sandro Knauß
> 
>

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

Re: setting default widgetStyle (and ColorScheme)

[Martin Graesslin]

On Saturday, November 28, 2015 7:58:47 PM CET René J. V. Bertin wrote:
> Olivier Goffart wrote:
> > A pure KF5 appliation would anyway use the KDE platform theme plugin
> > provided by KDE Frameworks.  (in frameworkintegration)
> > (So in other words: that code should not be executed when kde frameworks
> > is
> > installed, in theory)
> 
> So... No, it doesn't. Not on OS X when using the default QPA plugin (Cocoa).
> From the looks of it that one lacks support for theme plugins. Or rather,
> it doesn't return the appropriate thing(s) from
> QGuiApplicationPrivate::platform_integration->themeNames().
> 
> Now the question is, should KF5 applications be able to use the KDE platform
> theme plugin provided by kf5-frameworkintegration if that framework is
> installed?

In my opinion: no. I even think the frameworkintegration framework should get 
removed from frameworks and moved into Plasma Workspace. Because that's what 
it is about: integrating Qt applications into plasma.

Cheers
Martin

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Policy for Dependencies

[Martin Graesslin]

On Tuesday, October 13, 2015 8:42:43 AM CEST Christoph Cullmann wrote:
> Hi,

thanks for raising this topic. I think it's very important that we have a 
general strategy for frameworks and not have thousands of micro-fixes in 
various frameworks.

> 1) "Normal" deployment like we do in on Linux => just installing it with all
> features if possible. 2) "Application Bundles/Installer" like we will have
> to do it on Win/Mac and 3rdparty Linux people will need to do.
> 
> I think the easiest solution is to make stuff optional. That will avoid ugly
> "if (WIN OR APPLE OR ANDROID)".. hacks in CMake and allow people to still
> build stuff with that deps on that operating systems if really wanted.

Given from the no-X11 fixes I think that we should avoid all if (WIN OR APPLE) 
as that:
a) is hard to maintain
b) doesn't scale
c) not testable for the devs working on Linux

Given that it should be feature based and we should make as much usage of the 
built in CMake features we have. I really like the approach we have now found 
for X11 on OSX: disable certain find modules at a global level.

I think that is something which could be applied for more things. Control 
through global ECM settings. This could if we don't want to have global 
changes also through convenient command switches:

e.g. cmake -DECM_BUILD_FOR_OSX_APPBUNDLE

which then implies e.g. no phonon and no dbus and ...

Cheers
Martin

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Jenkins and XDG_DATA_DIRS

[Martin Graesslin]

On Tuesday, March 22, 2016 6:46:50 PM CET Ben Cooksley wrote:
> On Mon, Mar 21, 2016 at 7:56 PM, Martin Graesslin  
wrote:
> > On Saturday, March 19, 2016 10:27:38 PM CET David Faure wrote:
> >> It appears that something regressed in the jenkins setup.
> >> 
> >> Almost all of the current failures in the "Frameworks kf5-qt5" group come
> >> from XDG_DATA_DIRS not being set up correctly anymore, I think.
> >> E.g. in
> >> https://build.kde.org/view/Frameworks%20kf5-qt5/job/kio%20master%20kf5-qt
> >> 5/
> >> 269/PLATFORM=Linux,compiler=gcc/console * KUriFilterTest and others fail
> >> to
> >> see the servicetype "KUriFilter/Plugin" which kio installs itself
> >> 
> >> The kio job says
> >> 
> >> export
> >> XDG_DATA_DIRS="/srv/jenkins/install/ubuntu/x86_64/g++/kf5-qt5/frameworks/
> >> kw
> >> idgetsaddons/inst/usr/share:/srv/jenkins/install/ubuntu/x86_64/g++/kf5-qt
> >> 5/f
> >> rameworks/kauth/inst/usr/share:/srv/jenkins/install/ubuntu/x86_64/g++/kf
> >> 5-qt
> >> 5/frameworks/sonnet/inst/usr/share:/srv/jenkins/install/ubuntu/x86_64/g+
> >> +/kf
> >> 5-qt5/frameworks/knotifications/inst/usr/share:/srv/jenkins/install/ubun
> >> tu/x
> >> 86_64/g++/kf5-qt5/kdesupport/extra-cmake-modules/inst/usr/share:/srv/jen
> >> kins
> >> /install/ubuntu/x86_64/g++/kf5-qt5/frameworks/kdoctools/inst/usr/share:/
> >> srv/
> >> jenkins/install/ubuntu/x86_64/g++/kf5-qt5/frameworks/ktextwidgets/inst/u
> >> sr/s
> >> hare:/srv/jenkins/install/ubuntu/x86_64/g++/kf5-qt5/frameworks/kconfigwi
> >> dget
> >> s/inst/usr/share:/srv/jenkins/install/ubuntu/x86_64/g++/kf5-qt5/framewor
> >> ks/k
> >> jobwidgets/inst/usr/share:/srv/jenkins/install/ubuntu/x86_64/g++/kf5-qt5
> >> /fra
> >> meworks/kcoreaddons/inst/usr/share:/srv/jenkins/install/ubuntu/x86_64/g+
> >> +/kf
> >> 5-qt5/frameworks/breeze-icons/inst/usr/share:/srv/jenkins/install/ubuntu
> >> /x86
> >> _64/g++/kf5-qt5/frameworks/kglobalaccel/inst/usr/share:/srv/jenkins/inst
> >> all/
> >> ubuntu/x86_64/g++/kf5-qt5/frameworks/kservice/inst/usr/share:/srv/jenkin
> >> s/in
> >> stall/ubuntu/x86_64/g++/kf5-qt5/frameworks/kwallet/inst/usr/share:/srv/j
> >> enki
> >> ns/install/ubuntu/x86_64/g++/kf5-qt5/kdesupport/phonon/phonon/inst/usr/s
> >> hare>> 
> >> :$XDG_DATA_DIRS:/usr/share:/usr/share"
> >> 
> >> This is missing the dir for kio itself.
> >> 
> >> Same kind of problem in kemoticonstest (which loads its own plugin),
> >> frameworkintegration (same).
> >> 
> >> Is this a voluntary change, as in, we should fix the tests to work
> >> without
> >> the need to make install first?
> > 
> > other frameworks affected by the CI-regression are:
> > * kwindowsystem
> > * kglobalaccel
> > 
> > both cannot find their own plugin so I assume it's the same problem. I
> > don't have the time to rework the tests to make them pass again, sorry
> > :-(
> From what i've been told, the necessary changes to the scripts should
> now have been made to correct this.
> A rebuild may be needed though.

I just run a build kwindowsystem and can confirm that it works again. It's back 
to green \o/

Thanks a lot

Martin

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel