Re: Review Request: Use platform palette and fonts when running on other desktop environments

[Thomas Lübking]

> On July 2, 2011, 9:49 p.m., Oswald Buddenhagen wrote:
> > hmm. but now things are still done twice in a kde session, no?
> > what was wrong with the suggestion to notify qt that it should update 
> > "stuff"?
> 
> Aurélien Gâteau wrote:
> createApplicationPalette() is indeed called twice when running on a KDE 
> session, but it is not a regression introduced by this change so I think it 
> is outside of the scope for now. I tried not doing anything in 
> kdisplaySetPalette() and call qt_x11_apply_settings_in_all_apps() from the 
> kcm as Olivier suggested, but that didn't work: the palette change was not 
> propagated to the running application.
> 
> What worries me right now is that the text area of KWrite does not get 
> updated at runtime. I thought it was due to the widget being custom, but it 
> correctly updates itself without the patch.
> 
> Aurélien Gâteau wrote:
> Finally found time to do more testing. It turns out the behavior of 
> KWrite text area is the same with or without the patch so it's not a 
> regression. Therefore, I think the patch should go in.

Sh*t - i forgot that I wanted to comment on that: kate keeps own color schemes 
for the text area, they're completely unrelated to he rest of the system.
(since you need to configure syntax highlightning and don't want that to run 
into a conflict with the system palette de toujours)

So yes, that's not a regression for sure, sorry.


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101805/#review4333
---


On July 2, 2011, 9:19 p.m., Aurélien Gâteau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101805/
> ---
> 
> (Updated July 2, 2011, 9:19 p.m.)
> 
> 
> Review request for kdelibs and Olivier Goffart.
> 
> 
> Summary
> ---
> 
> When a KDE application is running on GNOME it looks odd right now because it 
> does not use the GNOME palette and fonts, contrary to Qt-only applications. 
> Attached patch fixes this by relying on the platform plugin to set the 
> correct palette and fonts if we are not running in a full KDE session.
> 
> Patch was suggested by Olivier Goffart.
> 
> 
> Diffs
> -
> 
>   kdeui/kernel/kglobalsettings.cpp 1a497c7 
> 
> Diff: http://git.reviewboard.kde.org/r/101805/diff
> 
> 
> Testing
> ---
> 
> # On KDE
> - Run kwrite on KDE => KDE palette and fonts
> - Change palette and fonts from System Settings => kwrite updates itself 
> correctly
> 
> # On GNOME
> - Run kwrite on GNOME => GNOME palette and fonts
> - Change palette and fonts from GNOME Tweak Tool => palette gets applied, 
> font does not for now
> 
> 
> Thanks,
> 
> Aurélien
> 
>

Re: Review Request: Only include nepomuk directories if nepomuk is available

[Laszlo Papp]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101949/#review4755
---

Ship it!


The compilation issue was not personally talkative enough when I tried to build 
kdelibs without sdo installed (it has not been packaged for this architecture 
that time). I have tested it more times in scratchbox for Harmattan, N9 and it 
works just fine without shared-desktop-ontologies installed. 

- Laszlo


On July 14, 2011, 2:40 p.m., Bjoern Ricks wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101949/
> ---
> 
> (Updated July 14, 2011, 2:40 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> ---
> 
> kdelibs build fails if sdo and/or soprano aren't available
> 
> 
> Diffs
> -
> 
>   kparts/CMakeLists.txt db76613 
> 
> Diff: http://git.reviewboard.kde.org/r/101949/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bjoern
> 
>

Re: Review Request: Use platform palette and fonts when running on other desktop environments

[Dominik Haumann]

> On July 2, 2011, 9:49 p.m., Oswald Buddenhagen wrote:
> > hmm. but now things are still done twice in a kde session, no?
> > what was wrong with the suggestion to notify qt that it should update 
> > "stuff"?
> 
> Aurélien Gâteau wrote:
> createApplicationPalette() is indeed called twice when running on a KDE 
> session, but it is not a regression introduced by this change so I think it 
> is outside of the scope for now. I tried not doing anything in 
> kdisplaySetPalette() and call qt_x11_apply_settings_in_all_apps() from the 
> kcm as Olivier suggested, but that didn't work: the palette change was not 
> propagated to the running application.
> 
> What worries me right now is that the text area of KWrite does not get 
> updated at runtime. I thought it was due to the widget being custom, but it 
> correctly updates itself without the patch.
> 
> Aurélien Gâteau wrote:
> Finally found time to do more testing. It turns out the behavior of 
> KWrite text area is the same with or without the patch so it's not a 
> regression. Therefore, I think the patch should go in.
> 
> Thomas Lübking wrote:
> Sh*t - i forgot that I wanted to comment on that: kate keeps own color 
> schemes for the text area, they're completely unrelated to he rest of the 
> system.
> (since you need to configure syntax highlightning and don't want that to 
> run into a conflict with the system palette de toujours)
> 
> So yes, that's not a regression for sure, sorry.

With regard to kwrite: It uses the system colors as long as they were never 
changed. Changed once, these system settings are overwritten. Hence, this is 
very likely a KatePart issue.


- Dominik


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101805/#review4333
---


On July 2, 2011, 9:19 p.m., Aurélien Gâteau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101805/
> ---
> 
> (Updated July 2, 2011, 9:19 p.m.)
> 
> 
> Review request for kdelibs and Olivier Goffart.
> 
> 
> Summary
> ---
> 
> When a KDE application is running on GNOME it looks odd right now because it 
> does not use the GNOME palette and fonts, contrary to Qt-only applications. 
> Attached patch fixes this by relying on the platform plugin to set the 
> correct palette and fonts if we are not running in a full KDE session.
> 
> Patch was suggested by Olivier Goffart.
> 
> 
> Diffs
> -
> 
>   kdeui/kernel/kglobalsettings.cpp 1a497c7 
> 
> Diff: http://git.reviewboard.kde.org/r/101805/diff
> 
> 
> Testing
> ---
> 
> # On KDE
> - Run kwrite on KDE => KDE palette and fonts
> - Change palette and fonts from System Settings => kwrite updates itself 
> correctly
> 
> # On GNOME
> - Run kwrite on GNOME => GNOME palette and fonts
> - Change palette and fonts from GNOME Tweak Tool => palette gets applied, 
> font does not for now
> 
> 
> Thanks,
> 
> Aurélien
> 
>

Re: Review Request: Make mouse cursor size configurable

[Lukas Sommer]

> On July 12, 2011, 8:56 a.m., Christoph Feck wrote:
> > kcontrol/input/xcursor/xcursortheme.cpp, line 73
> > 
> >
> > Do you mean the sizes are always > 0, or do you mean the number of 
> > entries in the list is > 0 ("never empty")?
> > 
> > Do we need some logic to prevent it saying "Available sizes:" when only 
> > one size is available? But I am not sure if i18n can handle that.

It means "never empty".

This could be a solution to the plural problem:

// The translation is aware of plural forms. i18ncp uses the first 
integer argument to
// distinguish plural forms. The first and the second argument are 
QString. So we use
// sizeList.size() as third argument to provide proper support for 
plural forms. This 
// works, although it is never referenced with %3 in the string itself. 
Although we
// provide english strings only for "1 item" and for "more than 1 
item", but i18ncp
// will silently expand to as many plural forms as necesarry in the 
target language.
m_description = i18ncp(
  "@info/plain This is the description of the cursor themes. The first 
argument is the "
"original description that comes from the index.theme file. The 
second argument is "
"the list of available sizes, which is never empty. This string is 
localized with "
"support for plural forms: You can make different singular/plural 
translations "
"depending on the number (!) of list items in the second argument.",
  "%1 (Size: %2)"/* only 1 item in sizeListString  */
  "%1 (Available sizes: %2)" /* more than 1 item in sizeListString */
  ).arg(m_description).arg(sizeliststring).arg(sizeList.size());

I'll test this after holiday.


- Lukas


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101701/#review4633
---


On June 20, 2011, 10:04 a.m., Lukas Sommer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101701/
> ---
> 
> (Updated June 20, 2011, 10:04 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, kdelibs, and Christoph Feck.
> 
> 
> Summary
> ---
> 
> X11 mouse cursor themes can contain cursors in multiple sizes, making them 
> pseudo-scalable.
> 
> It is yet possible in KDE to configure manually the mouse cursor size 
> (editing kcminput.rc). However, the GUI of the corresponding KControl module 
> didn't provide support to change this. This patch add support for changing 
> the mouse cursor size to the GUI.
> 
> This are mostly GUI related changes. The underlying data structure 
> XCursorTheme did yet provide support for choosing different sizes and only 
> needed some adjustments.
> 
> 
> This addresses bug 90444.
> http://bugs.kde.org/show_bug.cgi?id=90444
> 
> 
> Diffs
> -
> 
>   kcontrol/input/xcursor/xcursortheme.cpp a987487 
>   kcontrol/input/xcursor/themepage.ui 2e38054 
>   kcontrol/input/xcursor/xcursortheme.h b474086 
>   kcontrol/input/xcursor/themepage.h 902148f 
>   kcontrol/input/xcursor/themepage.cpp 24b9efe 
>   kcontrol/input/xcursor/previewwidget.h f4d2c4e 
>   kcontrol/input/xcursor/previewwidget.cpp 3c264fc 
>   kcontrol/input/xcursor/cursortheme.h 8f7834b 
>   kcontrol/input/xcursor/legacytheme.h 23c9d5f 
> 
> Diff: http://git.reviewboard.kde.org/r/101701/diff
> 
> 
> Testing
> ---
> 
> Tested locally. Works fine for me. Also when using non-standard font DPI 
> values.
> 
> 
> Screenshots
> ---
> 
> 
>   http://git.reviewboard.kde.org/r/101701/s/188/
> 
>   http://git.reviewboard.kde.org/r/101701/s/189/
> 
> 
> Thanks,
> 
> Lukas
> 
>

cpp includes in http ioslave

[Rolf Eike Beer]

When one looks at kioslave/http/http.cpp it finds things like this:

//string parsing helpers and HeaderTokenizer implementation
#include "parsinghelpers.cpp"
//authentication handlers
#include "httpauthentication.cpp"

Which happens because e.g. parsinghelpers.cpp only consist of static 
functions. httpauthentication.cpp on the other hand consists of normal class 
implementations. I don't really see a point in keeping at least this file 
included as it's currently is. I wanted to write some testcases for this and 
found out it's hard to use at all as several headers are missing because it 
relies on http.cpp including them. In httpauthentication.h you can also find 
things like this:

#ifndef HTTP_H_ // if we're included from http.cpp all necessary headers are 
already included
#include 
#include 
#include 
#include 
#endif

Would anyone object if I clean this up so the cpp files are all compiled on 
their own and the parsinghelper stuff is put into a namespace (to avoid 
pollution the global one) so these files can be just added in the 
CMakeLists.txt to some project (like the testcases)?

I also would like to move the kioslave/tests/ directory to 
kioslave/http/tests. Currently all tests in there are http tests anyway. The 
rationale behind this is that the authentication test would need to duplicate 
all that gss include/link stuff from the http/CMakeLists.txt. I don't want to 
duplicate this as it would also needed to kept in sync if anyone goes and 
touches this. So putting the tests into the http directory would allow the 
tests to simply use the same settings that kio_http uses.

Eike

signature.asc
Description: This is a digitally signed message part.

CDash of kdelibs

[Rolf Eike Beer]

When I look at the kdelibs test results at 
http://my.cdash.org/index.php?project=kdelibs I see the following things that 
IMHO need to be changed:

-the limit of 3000 warnings is reached, do we need to increase the limit?

-the moc files are counted in the coverage results even if CTestConfig.cmake 
contains

set(CTEST_CUSTOM_COVERAGE_EXCLUDE ".moc$" "moc_" "ui_")

So something is wrong there. This also adds to the number of warnings as the 
first to warning messages are "no relevant classes found" ones which should be 
suppressed.

IIRC those lines need to be moved to the CTestCustom.cmake to take effect. 
Alex?

-I think that all testcases should also be removed from the coverage results, 
i.e. those list should be extended by "/tests/".

Opinions?

Eike

signature.asc
Description: This is a digitally signed message part.

Re: CDash of kdelibs

[Alexander Neundorf]

On Sunday 17 July 2011, Rolf Eike Beer wrote:
> When I look at the kdelibs test results at
> http://my.cdash.org/index.php?project=kdelibs I see the following things
> that IMHO need to be changed:
> 
> -the limit of 3000 warnings is reached, do we need to increase the limit?

I don't think having more then 3000 warnings there is useful.
From my own experience the display of warnings starts to be useful when it's 
close to zero, let's say less than 20 or so.
Then you start to see whether new warnings appeared, and can check if that 
happens.
With such a huge number as we have now this doesn't make a lot of sense in my 
experience.
So, we should try to get that number down.
Either by fixing them, or by disabling the wanrings, or by putting the ones we 
are not interested in the the exception regex for ignored warnings.

> -the moc files are counted in the coverage results even if
> CTestConfig.cmake contains
> 
> set(CTEST_CUSTOM_COVERAGE_EXCLUDE ".moc$" "moc_" "ui_")
> 
> So something is wrong there. This also adds to the number of warnings as
> the first to warning messages are "no relevant classes found" ones which
> should be suppressed.
> 
> IIRC those lines need to be moved to the CTestCustom.cmake to take effect.
> Alex?

I think so.

> 
> -I think that all testcases should also be removed from the coverage
> results, i.e. those list should be extended by "/tests/".

Shouldn't the coverage for the testcases itself be around 100%, so they don't 
cause fixing work.
Beside that I don't have an opinion whether they should be included or not.

Alex

Re: Review Request: Make mouse cursor size configurable

[Chusslove Illich]

> On July 12, 2011, 8:56 a.m., Christoph Feck wrote:
> > kcontrol/input/xcursor/xcursortheme.cpp, line 73
> > 
> >
> > Do you mean the sizes are always > 0, or do you mean the number of 
> > entries in the list is > 0 ("never empty")?
> > 
> > Do we need some logic to prevent it saying "Available sizes:" when only 
> > one size is available? But I am not sure if i18n can handle that.
> 
> Lukas Sommer wrote:
> It means "never empty".
> 
> This could be a solution to the plural problem:
> 
> // The translation is aware of plural forms. i18ncp uses the 
> first integer argument to
> // distinguish plural forms. The first and the second argument 
> are QString. So we use
> // sizeList.size() as third argument to provide proper support 
> for plural forms. This 
> // works, although it is never referenced with %3 in the string 
> itself. Although we
> // provide english strings only for "1 item" and for "more than 1 
> item", but i18ncp
> // will silently expand to as many plural forms as necesarry in 
> the target language.
> m_description = i18ncp(
>   "@info/plain This is the description of the cursor themes. The 
> first argument is the "
> "original description that comes from the index.theme file. 
> The second argument is "
> "the list of available sizes, which is never empty. This 
> string is localized with "
> "support for plural forms: You can make different 
> singular/plural translations "
> "depending on the number (!) of list items in the second 
> argument.",
>   "%1 (Size: %2)"/* only 1 item in sizeListString 
>  */
>   "%1 (Available sizes: %2)" /* more than 1 item in 
> sizeListString */
>   ).arg(m_description).arg(sizeliststring).arg(sizeList.size());
> 
> I'll test this after holiday.

i18ncp() should not be used for clever tricks. The plural string should
always be the direct pluralization of the singular string.

I think it would be just fine to leave it at the original version, with "...
Available sizes: ..." even if there is only one size. Also no need to
mention in the context that list of sizes is never empty.

Otherwise, use two separate messages with ordinary if-selection.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101701/#review4633
---


On June 20, 2011, 10:04 a.m., Lukas Sommer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101701/
> ---
> 
> (Updated June 20, 2011, 10:04 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, kdelibs, and Christoph Feck.
> 
> 
> Summary
> ---
> 
> X11 mouse cursor themes can contain cursors in multiple sizes, making them 
> pseudo-scalable.
> 
> It is yet possible in KDE to configure manually the mouse cursor size 
> (editing kcminput.rc). However, the GUI of the corresponding KControl module 
> didn't provide support to change this. This patch add support for changing 
> the mouse cursor size to the GUI.
> 
> This are mostly GUI related changes. The underlying data structure 
> XCursorTheme did yet provide support for choosing different sizes and only 
> needed some adjustments.
> 
> 
> This addresses bug 90444.
> http://bugs.kde.org/show_bug.cgi?id=90444
> 
> 
> Diffs
> -
> 
>   kcontrol/input/xcursor/xcursortheme.cpp a987487 
>   kcontrol/input/xcursor/themepage.ui 2e38054 
>   kcontrol/input/xcursor/xcursortheme.h b474086 
>   kcontrol/input/xcursor/themepage.h 902148f 
>   kcontrol/input/xcursor/themepage.cpp 24b9efe 
>   kcontrol/input/xcursor/previewwidget.h f4d2c4e 
>   kcontrol/input/xcursor/previewwidget.cpp 3c264fc 
>   kcontrol/input/xcursor/cursortheme.h 8f7834b 
>   kcontrol/input/xcursor/legacytheme.h 23c9d5f 
> 
> Diff: http://git.reviewboard.kde.org/r/101701/diff
> 
> 
> Testing
> ---
> 
> Tested locally. Works fine for me. Also when using non-standard font DPI 
> values.
> 
> 
> Screenshots
> ---
> 
> 
>   http://git.reviewboard.kde.org/r/101701/s/188/
> 
>   http://git.reviewboard.kde.org/r/101701/s/189/
> 
> 
> Thanks,
> 
> Lukas
> 
>

Re: CDash of kdelibs

[Rolf Eike Beer]

Am Sonntag 17 Juli 2011, 12:58:35 schrieben Sie:
> On Sunday 17 July 2011, Rolf Eike Beer wrote:

> > -the moc files are counted in the coverage results even if
> > CTestConfig.cmake contains
> > 
> > set(CTEST_CUSTOM_COVERAGE_EXCLUDE ".moc$" "moc_" "ui_")
> > 
> > So something is wrong there. This also adds to the number of warnings as
> > the first to warning messages are "no relevant classes found" ones which
> > should be suppressed.
> > 
> > IIRC those lines need to be moved to the CTestCustom.cmake to take
> > effect. Alex?
> 
> I think so.

Ok, pushed that into master. If that shows any effect in dashboard tomorrow 
I'll do that also for 4.[67].

> > -I think that all testcases should also be removed from the coverage
> > results, i.e. those list should be extended by "/tests/".
> 
> Shouldn't the coverage for the testcases itself be around 100%, so they
> don't  cause fixing work.
> Beside that I don't have an opinion whether they should be included or not.

The overall coverage is computed with the testcases. So a coverage of 50% may 
actually mean 35% of relevant code are tested and the rest is testcases.

Eike


signature.asc
Description: This is a digitally signed message part.

Re: cpp includes in http ioslave

[Dawit A]

On Sun, Jul 17, 2011 at 6:38 AM, Rolf Eike Beer
 wrote:
> When one looks at kioslave/http/http.cpp it finds things like this:
>
> //string parsing helpers and HeaderTokenizer implementation
> #include "parsinghelpers.cpp"
> //authentication handlers
> #include "httpauthentication.cpp"
>
> Which happens because e.g. parsinghelpers.cpp only consist of static
> functions. httpauthentication.cpp on the other hand consists of normal class
> implementations. I don't really see a point in keeping at least this file
> included as it's currently is. I wanted to write some testcases for this and
> found out it's hard to use at all as several headers are missing because it
> relies on http.cpp including them. In httpauthentication.h you can also find
> things like this:

> #ifndef HTTP_H_ // if we're included from http.cpp all necessary headers are
> already included
> #include 
> #include 
> #include 
> #include 
> #endif
>
> Would anyone object if I clean this up so the cpp files are all compiled on
> their own and the parsinghelper stuff is put into a namespace (to avoid
> pollution the global one) so these files can be just added in the
> CMakeLists.txt to some project (like the testcases)?

No objection to the cleanup, but please do not do it until I commit my
changes. I haved added a new unit test for http authentication after
fixing the test cases that were failing at
http://greenbytes.de/tech/tc/httpauth/. The unit test is actually more
through than that and exercises many parts of http authentication
including calculation of digest and basic authentications. I hope you
did not also do the same thing.

> I also would like to move the kioslave/tests/ directory to
> kioslave/http/tests. Currently all tests in there are http tests anyway. The
> rationale behind this is that the authentication test would need to duplicate
> all that gss include/link stuff from the http/CMakeLists.txt. I don't want to
> duplicate this as it would also needed to kept in sync if anyone goes and
> touches this. So putting the tests into the http directory would allow the
> tests to simply use the same settings that kio_http uses.

This makes sense to me too, but please wait until I commit my changes.

Regards,
Dawit A.

Re: cpp includes in http ioslave

[Rolf Eike Beer]

Am Sonntag, 17. Juli 2011, 13:14:39 schrieb Dawit A:
> On Sun, Jul 17, 2011 at 6:38 AM, Rolf Eike Beer
> 
>  wrote:
> > When one looks at kioslave/http/http.cpp it finds things like this:
> > 
> > //string parsing helpers and HeaderTokenizer implementation
> > #include "parsinghelpers.cpp"
> > //authentication handlers
> > #include "httpauthentication.cpp"
> > 
> > Which happens because e.g. parsinghelpers.cpp only consist of static
> > functions. httpauthentication.cpp on the other hand consists of normal
> > class implementations. I don't really see a point in keeping at least
> > this file included as it's currently is. I wanted to write some
> > testcases for this and found out it's hard to use at all as several
> > headers are missing because it relies on http.cpp including them. In
> > httpauthentication.h you can also find things like this:
> > 
> > #ifndef HTTP_H_ // if we're included from http.cpp all necessary headers
> > are already included
> > #include 
> > #include 
> > #include 
> > #include 
> > #endif
> > 
> > Would anyone object if I clean this up so the cpp files are all compiled
> > on their own and the parsinghelper stuff is put into a namespace (to
> > avoid pollution the global one) so these files can be just added in the
> > CMakeLists.txt to some project (like the testcases)?
> 
> No objection to the cleanup, but please do not do it until I commit my
> changes. I haved added a new unit test for http authentication after
> fixing the test cases that were failing at
> http://greenbytes.de/tech/tc/httpauth/. The unit test is actually more
> through than that and exercises many parts of http authentication
> including calculation of digest and basic authentications. I hope you
> did not also do the same thing.

Well, if I'm faster those issues would be fixed, too ;) I think we should both 
push our branches to the server and compare what we have done.

Eike

signature.asc
Description: This is a digitally signed message part.

Re: cpp includes in http ioslave

[Rolf Eike Beer]

Am Sonntag, 17. Juli 2011, 20:28:44 schrieb Rolf Eike Beer:
> Am Sonntag, 17. Juli 2011, 13:14:39 schrieb Dawit A:

> > No objection to the cleanup, but please do not do it until I commit my
> > changes. I haved added a new unit test for http authentication after
> > fixing the test cases that were failing at
> > http://greenbytes.de/tech/tc/httpauth/. The unit test is actually more
> > through than that and exercises many parts of http authentication
> > including calculation of digest and basic authentications. I hope you
> > did not also do the same thing.
> 
> Well, if I'm faster those issues would be fixed, too ;) I think we should
> both push our branches to the server and compare what we have done.

I've pushed my stuff to dakon/http-auth branch. Please have a look at this one 
and tell me what you think. Feel free to merge those things you find usable and 
push to master.

Eike

signature.asc
Description: This is a digitally signed message part.

Re: Review Request: Make mouse cursor size configurable

[Lukas Sommer]

> On July 12, 2011, 8:56 a.m., Christoph Feck wrote:
> > kcontrol/input/xcursor/xcursortheme.cpp, line 73
> > 
> >
> > Do you mean the sizes are always > 0, or do you mean the number of 
> > entries in the list is > 0 ("never empty")?
> > 
> > Do we need some logic to prevent it saying "Available sizes:" when only 
> > one size is available? But I am not sure if i18n can handle that.
> 
> Lukas Sommer wrote:
> It means "never empty".
> 
> This could be a solution to the plural problem:
> 
> // The translation is aware of plural forms. i18ncp uses the 
> first integer argument to
> // distinguish plural forms. The first and the second argument 
> are QString. So we use
> // sizeList.size() as third argument to provide proper support 
> for plural forms. This 
> // works, although it is never referenced with %3 in the string 
> itself. Although we
> // provide english strings only for "1 item" and for "more than 1 
> item", but i18ncp
> // will silently expand to as many plural forms as necesarry in 
> the target language.
> m_description = i18ncp(
>   "@info/plain This is the description of the cursor themes. The 
> first argument is the "
> "original description that comes from the index.theme file. 
> The second argument is "
> "the list of available sizes, which is never empty. This 
> string is localized with "
> "support for plural forms: You can make different 
> singular/plural translations "
> "depending on the number (!) of list items in the second 
> argument.",
>   "%1 (Size: %2)"/* only 1 item in sizeListString 
>  */
>   "%1 (Available sizes: %2)" /* more than 1 item in 
> sizeListString */
>   ).arg(m_description).arg(sizeliststring).arg(sizeList.size());
> 
> I'll test this after holiday.
> 
> Chusslove Illich wrote:
> i18ncp() should not be used for clever tricks. The plural string should
> always be the direct pluralization of the singular string.
> 
> I think it would be just fine to leave it at the original version, with 
> "...
> Available sizes: ..." even if there is only one size. Also no need to
> mention in the context that list of sizes is never empty.
> 
> Otherwise, use two separate messages with ordinary if-selection.
>

What's about merging both approaches?

int sizeListSize = sizeList.size();
QString sizeListString = QString::number(sizeList.takeFirst());
while (!sizeList.isEmpty())
{
sizeListString.append(", ");
sizeListString.append(QString::number(sizeList.takeFirst()));
};
if (sizeList.size() == 1)
m_description = i18ncp(
"@info/plain This is the description of the cursor themes. The 
first argument is "
"the original description that comes from the index.theme 
file. The second "
"argument is the size (in pixel). Example: 
'OriginalDescription (Size: 24)'",
"%1 (Size: %2)").arg(m_description).arg(sizeListString);
else // sizeList.size() > 1
/* The translation is aware of plural forms. i18ncp uses the first 
integer argument to
   distinguish plural forms. The first and the second argument are 
QString. So we use
   sizeList.size() as third argument to provide proper support for 
plural forms. This 
   works, although it is never referenced with %3 in the string 
itself. Although we
   provide english strings only for "1 item" and for "more than 1 
item", but i18ncp
   will silently expand to as many plural forms as necesarry in the 
target language. */
m_description = i18ncp(
"@info/plain This is the description of the cursor themes. The 
first argument is "
"the original description that comes from the index.theme 
file. The second "
"argument is the list of available sizes (in pixel). This 
string is localized "
"with support for plural forms: You can make different 
singular/plural "
"translations depending on the number (!) of list items in 
the second "
"argument. Example: 'OriginalDescription (Available sizes: 
24, 36, 48)' has "
"the plural form for 3 because the list has 3 items.",
"%1 (Available size: %2)"  /* only 1 item in sizeListString - 
will never be used */
"%1 (Available sizes: %2)" /* more than 1 item in 
sizeListString */
).arg(m_description).arg(sizeListString).arg(sizeListSize);

This way we have a good translation for only 1 size: (Size: 24). In this case 
the user can't do an

Re: cpp includes in http ioslave

[Rolf Eike Beer]

Am Sonntag, 17. Juli 2011, 20:58:09 schrieb Rolf Eike Beer:
> Am Sonntag, 17. Juli 2011, 20:28:44 schrieb Rolf Eike Beer:
> > Am Sonntag, 17. Juli 2011, 13:14:39 schrieb Dawit A:
> > > No objection to the cleanup, but please do not do it until I commit
> > > my
> > > changes. I haved added a new unit test for http authentication after
> > > fixing the test cases that were failing at
> > > http://greenbytes.de/tech/tc/httpauth/. The unit test is actually
> > > more
> > > through than that and exercises many parts of http authentication
> > > including calculation of digest and basic authentications. I hope
> > > you
> > > did not also do the same thing.
> > 
> > Well, if I'm faster those issues would be fixed, too ;) I think we
> > should
> > both push our branches to the server and compare what we have done.
> 
> I've pushed my stuff to dakon/http-auth branch. Please have a look at this
> one and tell me what you think. Feel free to merge those things you find
> usable and push to master.

Oh, nice, my git setup for this repository is incomplete. Everything I pushed 
had a wrong email address. If you want to take some patches from there then 
please change my email address to the one used in this mail.

Eike

signature.asc
Description: This is a digitally signed message part.

Re: RFC: replacing MacroLogFeature.cmake with FeatureSummary.cmake

[Alexander Neundorf]

Ok, there is now a branch FeatureSummaryImprovements on the cmake stage:
http://cmake.org/gitweb?p=stage/cmake.git;a=shortlog;h=refs/heads/FeatureSummaryImprovements

It should have everything discussed here:
* every package can have multiple PURPOSEs
* every package has a dependency TYPE: RUNTIME < OPTIONAL < RECOMMENDED < 
REQUIRED

* optionally, feature_summary() can abort if a REQUIRED package has not been 
found


Usage is like this:

find_package(LibXml2)
# the following one ideally directly inside LibXml2:
set_package_properties(LibXml2 PROPERTIES
   DESCRIPTION "XML processing library."
   URL "http://xmlsoft.org/";)

# and in the using project
set_package_properties(LibXml2 PROPERTIES
PURPOSE "Required for exporting spreadsheets to odt format in kspread"
TYPE RECOMMENDED
)


# and in some other part of the project:
set_package_properties(LibXml2 PROPERTIES
PURPOSE "Required for importing html files in kword"
TYPE OPTIONAL )

# or all in one call:
find_package(DBus)
set_package_properties(DBus PROPERTIES 
DESCRIPTION "A desktop IPC bus"
URL "http://dbus.freedesktop.org>"
TYPE RUNTIME
PURPOSE "Required to disable the screensaver via kpresenter")


The call to feature_summary() equivalent to what we have now is:
feature_summary(WHAT ALL   FATAL_ON_MISSING_REQUIRED_PACKAGES)


Please give it a try, let me know about issues you find or whether it's just 
nice.
Also, I'm happy about patches which improve the documentation or the output 
format (or anything else).

Alex

Re: Review Request: Make mouse cursor size configurable

[Chusslove Illich]

> On July 12, 2011, 8:56 a.m., Christoph Feck wrote:
> > kcontrol/input/xcursor/xcursortheme.cpp, line 73
> > 
> >
> > Do you mean the sizes are always > 0, or do you mean the number of 
> > entries in the list is > 0 ("never empty")?
> > 
> > Do we need some logic to prevent it saying "Available sizes:" when only 
> > one size is available? But I am not sure if i18n can handle that.
> 
> Lukas Sommer wrote:
> It means "never empty".
> 
> This could be a solution to the plural problem:
> 
> // The translation is aware of plural forms. i18ncp uses the 
> first integer argument to
> // distinguish plural forms. The first and the second argument 
> are QString. So we use
> // sizeList.size() as third argument to provide proper support 
> for plural forms. This 
> // works, although it is never referenced with %3 in the string 
> itself. Although we
> // provide english strings only for "1 item" and for "more than 1 
> item", but i18ncp
> // will silently expand to as many plural forms as necesarry in 
> the target language.
> m_description = i18ncp(
>   "@info/plain This is the description of the cursor themes. The 
> first argument is the "
> "original description that comes from the index.theme file. 
> The second argument is "
> "the list of available sizes, which is never empty. This 
> string is localized with "
> "support for plural forms: You can make different 
> singular/plural translations "
> "depending on the number (!) of list items in the second 
> argument.",
>   "%1 (Size: %2)"/* only 1 item in sizeListString 
>  */
>   "%1 (Available sizes: %2)" /* more than 1 item in 
> sizeListString */
>   ).arg(m_description).arg(sizeliststring).arg(sizeList.size());
> 
> I'll test this after holiday.
> 
> Chusslove Illich wrote:
> i18ncp() should not be used for clever tricks. The plural string should
> always be the direct pluralization of the singular string.
> 
> I think it would be just fine to leave it at the original version, with 
> "...
> Available sizes: ..." even if there is only one size. Also no need to
> mention in the context that list of sizes is never empty.
> 
> Otherwise, use two separate messages with ordinary if-selection.
>
> 
> Lukas Sommer wrote:
> What's about merging both approaches?
> 
> int sizeListSize = sizeList.size();
> QString sizeListString = QString::number(sizeList.takeFirst());
> while (!sizeList.isEmpty())
> {
> sizeListString.append(", ");
> sizeListString.append(QString::number(sizeList.takeFirst()));
> };
> if (sizeList.size() == 1)
> m_description = i18ncp(
> "@info/plain This is the description of the cursor 
> themes. The first argument is "
> "the original description that comes from the 
> index.theme file. The second "
> "argument is the size (in pixel). Example: 
> 'OriginalDescription (Size: 24)'",
> "%1 (Size: %2)").arg(m_description).arg(sizeListString);
> else // sizeList.size() > 1
> /* The translation is aware of plural forms. i18ncp uses the 
> first integer argument to
>distinguish plural forms. The first and the second 
> argument are QString. So we use
>sizeList.size() as third argument to provide proper 
> support for plural forms. This 
>works, although it is never referenced with %3 in the 
> string itself. Although we
>provide english strings only for "1 item" and for "more 
> than 1 item", but i18ncp
>will silently expand to as many plural forms as necesarry 
> in the target language. */
> m_description = i18ncp(
> "@info/plain This is the description of the cursor 
> themes. The first argument is "
> "the original description that comes from the 
> index.theme file. The second "
> "argument is the list of available sizes (in pixel). 
> This string is localized "
> "with support for plural forms: You can make 
> different singular/plural "
> "translations depending on the number (!) of list 
> items in the second "
> "argument. Example: 'OriginalDescription (Available 
> sizes: 24, 36, 48)' has "
> "the plural form for 3 because the list has 3 items.",
> "%1 (Available size: %2)"  /* only 1 item in 
> sizeListString - will never be used */
>  

Re: Review Request: Use platform palette and fonts when running on other desktop environments

[Aurélien Gâteau]

> On July 2, 2011, 9:49 p.m., Oswald Buddenhagen wrote:
> > hmm. but now things are still done twice in a kde session, no?
> > what was wrong with the suggestion to notify qt that it should update 
> > "stuff"?
> 
> Aurélien Gâteau wrote:
> createApplicationPalette() is indeed called twice when running on a KDE 
> session, but it is not a regression introduced by this change so I think it 
> is outside of the scope for now. I tried not doing anything in 
> kdisplaySetPalette() and call qt_x11_apply_settings_in_all_apps() from the 
> kcm as Olivier suggested, but that didn't work: the palette change was not 
> propagated to the running application.
> 
> What worries me right now is that the text area of KWrite does not get 
> updated at runtime. I thought it was due to the widget being custom, but it 
> correctly updates itself without the patch.
> 
> Aurélien Gâteau wrote:
> Finally found time to do more testing. It turns out the behavior of 
> KWrite text area is the same with or without the patch so it's not a 
> regression. Therefore, I think the patch should go in.
> 
> Thomas Lübking wrote:
> Sh*t - i forgot that I wanted to comment on that: kate keeps own color 
> schemes for the text area, they're completely unrelated to he rest of the 
> system.
> (since you need to configure syntax highlightning and don't want that to 
> run into a conflict with the system palette de toujours)
> 
> So yes, that's not a regression for sure, sorry.
> 
> Dominik Haumann wrote:
> With regard to kwrite: It uses the system colors as long as they were 
> never changed. Changed once, these system settings are overwritten. Hence, 
> this is very likely a KatePart issue.

Oh. Thanks Thomas and Dominik, it suddenly makes more sense! If there is no 
other objection I'd like to merge this patch this week. Anyone against that?


- Aurélien


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101805/#review4333
---


On July 2, 2011, 9:19 p.m., Aurélien Gâteau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101805/
> ---
> 
> (Updated July 2, 2011, 9:19 p.m.)
> 
> 
> Review request for kdelibs and Olivier Goffart.
> 
> 
> Summary
> ---
> 
> When a KDE application is running on GNOME it looks odd right now because it 
> does not use the GNOME palette and fonts, contrary to Qt-only applications. 
> Attached patch fixes this by relying on the platform plugin to set the 
> correct palette and fonts if we are not running in a full KDE session.
> 
> Patch was suggested by Olivier Goffart.
> 
> 
> Diffs
> -
> 
>   kdeui/kernel/kglobalsettings.cpp 1a497c7 
> 
> Diff: http://git.reviewboard.kde.org/r/101805/diff
> 
> 
> Testing
> ---
> 
> # On KDE
> - Run kwrite on KDE => KDE palette and fonts
> - Change palette and fonts from System Settings => kwrite updates itself 
> correctly
> 
> # On GNOME
> - Run kwrite on GNOME => GNOME palette and fonts
> - Change palette and fonts from GNOME Tweak Tool => palette gets applied, 
> font does not for now
> 
> 
> Thanks,
> 
> Aurélien
> 
>

Re: RFC: replacing MacroLogFeature.cmake with FeatureSummary.cmake

[David Jarvie]

On Wednesday 13 July 2011 20:16:58 Alexander Neundorf wrote:
> Hi,
> 
> in KDE we have the file MacroLogFeature.cmake which we use to print a summary 
> of found/missing packages at the end of a cmake run.
> I added similar functionality later on in 2007 to cmake in the form of 
> FeatureSummary.cmake:
> http://www.cmake.org/cmake/help/cmake-2-8-docs.html#module:FeatureSummary
> 
> Among others due to source compatibility requirements we are still using 
> MacroLogFeature.cmake in KDE, but I'd like to get rid of this and instead use 
> FeatureSummary.cmake from the not yet existing cmake 2.8.6, which means we 
> have some time to improve it so it does what we need.
> The plan is to get this into cmake 2.8.6, so once we have the binary break 
> (and minor source compat. break) with Qt5 and KDE frameworks, to make the 
> switch here too.
> 
> -
> What is missing in FeatureSummary.cmake compared to MacroLogFeature.cmake:
> -
> 
> * the REQUIRED keyword, to make the cmake run error out at the end if one of 
> the packages marked with this keyword have not been found
> 
> -
> What has FeatureSummary.cmake that MacroLogFeature.cmake does not have:
> -
> * more flexible output, to a file, to stdout
> * should be faster, since it stores the results in a cmake property and not 
> in 
> a file (cmake properties did not exist back then when I write 
> MacroLogFeature.cmake initially)
> * it can separate between "packages" and "features", i.e. you can also add 
> feature documentation to the log at the end, e.g. for enabled or disabled 
> options
> * it's in cmake, so no extra stuff for us to maintain
> 
> 
> -
> What I'd like to have additionally in FeatureSummary.cmake
> -
> 
> * the ability to report "runtime" dependencies, i.e. packages which have been 
> checked for at build time, but which are actually not used at build time, but 
> only at runtime. So the build will be successful if they are missing, but the 
> result will not run properly
> 
> * the ability to set not only a description of a package, but also one or 
> more 
> usages/purposes.
> 
> How I imagine this is the following, e.g. for FindLibXml2.cmake:
> 
> inside FindLibXml2.cmake:
> find_path(...)
> find_library(...)
> ...
> set_package_info(LibXml2 "XML processing library." "http://xmlsoft.org/";)
> 
> 
> and then in a project using LibXml2, let's assume koffice:
> 
> find_package(LibXml2)
> 
> set_package_properties(LibXml2 PROPERTIES
> PURPOSE "Required for exporting spreadsheets to odt format in kspread")
> 
> 
> and in some other part of the project:
> set_package_properties(LibXml2 PROPERTIES
> PURPOSE "Required for importing html files in kword")
> 
> 
> find_package(DBus)
> set_package_properties(DBus PROPERTIES TYPE RUNTIME
> PURPOSE "Required to disable the screensaver via kpresenter")
> 
> -
> 
> which should then produce something like:
> 
> -- Found packages:
> LibXml2, XML processing library., 
> * Required for exporting spreadsheets to odt format in kspread
> * Required for importing html files in kword
> ...
>  -- Missing RUNTIME packages:
>DBus, A desktop IPC bus, 
>  * Required to disable the screensaver via kpresenter
> 
> 
> 
> What do you think of this ?
> More wishes ?
> Should it do it in a different way ?

Something else which would be useful would be to output the actual file names 
(library files, header files etc.) which were checked for but not found when 
looking for the missing packages. Trying to determine which distro package to 
install to satisfy the dependency isn't always straightforward, since distro 
package names aren't always the same as the cmake package names. In these 
cases, I quite often have to find the cmake module used to locate the package, 
and then look through the cmake module to work out the files it's looking for, 
in order to then be able to do a package search for my distro to find which 
package to install. 

-- 
David Jarvie.
KDE developer.
KAlarm author -- http://www.astrojar.org.uk/kalarm

Re: CDash of kdelibs

[Rolf Eike Beer]

Am Sonntag, 17. Juli 2011, 12:47:36 schrieb Rolf Eike Beer:
> When I look at the kdelibs test results at
> http://my.cdash.org/index.php?project=kdelibs I see the following things
> that IMHO need to be changed:
> 
> -the limit of 3000 warnings is reached, do we need to increase the limit?

We don't, we are now down to 2422 warnings by fixing problem #2.

> -the moc files are counted in the coverage results even if CTestConfig.cmake
> contains
> 
> set(CTEST_CUSTOM_COVERAGE_EXCLUDE ".moc$" "moc_" "ui_")
> 
> So something is wrong there. This also adds to the number of warnings as the
> first to warning messages are "no relevant classes found" ones which should
> be suppressed.
> 
> IIRC those lines need to be moved to the CTestCustom.cmake to take effect.
> Alex?

This is fixed now, I backported this into 4.7 in case anyone starts doing 
builds there sometime.

> -I think that all testcases should also be removed from the coverage
> results, i.e. those list should be extended by "/tests/".

For this I would like to hear some more opinions.

Eike

signature.asc
Description: This is a digitally signed message part.