Re: I'm missing the favicons interface

[Martin Graesslin]

On Friday, February 19, 2016 11:04:21 PM CET David Faure wrote:
> On Tuesday 16 February 2016 22:32:18 Robby Stephenson wrote:
> > On Tue, Feb 16, 2016 at 5:00 PM, David Faure  wrote:
> > > I'm curious though, what's the use case for a large favicon? In KDE4 the
> > > only
> > > time I see this is when using Alt+Tab and with konqueror windows open
> > > (and
> > > that
> > > looks a bit weird because other apps have a much bigger icon than
> > > konq's,
> > > but
> > > that's an unrelated issue I guess). I guess your app has another use for
> > > large
> > > icons with a favicon, I'm just curious what it is, and whether the
> > > text-html icon
> > > as the main icon is the best icon to have. This is an opportunity to
> > > rethink this
> > > if we want to ;)
> > 
> > Tellico searches a number of API end points, including some like SRU or
> > z39.50 that could be any server at all. Many are also http servers and
> > provide a favicon. In the configuration dialog for those sources, I chose
> > to put a 64x64 icon next to all the specific data options and the favicon
> > overlay fits the bill. I agree - it's an odd case, and as for whether
> > text/html is the best icon, well, probably better than any other option.
> 
> OK.
> 
> I experimented a little bit, and realized that there are a few issues
> even in kdelibs4 this isn't ideal, in Alt+Tab everything has much bigger
> icons than konqueror's favicon... so an icon engine (and returning
> a QIcon from the job) would be good, but a bit tricky for konqueror's code
> (which uses QString for this in many places).
> 
> This also makes me wonder if KWindowSystem::setIcons should
> still exist, it's limited to two icons sizes while QIcon can handle a lot
> more --- Martin?

well yeah the API in KWindowSytem is not optimal. It would be better to have a 
variant to pass a QIcon.

The underlying NETWinInfo class can set icons in any size, though.

The better question is whether QWindow::setIcon works as expected. If that's 
the case I'd suggest to deprecate KWindowSystem::setIcons in favor of 
QWindow::setIcon.

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: Review Request 127117: Reverse the filtering logic

[Aleix Pol Gonzalez]

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



Looks good, can you get a test case in the PersonsProxyModelTest?

- Aleix Pol Gonzalez


On Feb. 19, 2016, 9:22 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127117/
> ---
> 
> (Updated Feb. 19, 2016, 9:22 p.m.)
> 
> 
> Review request for KDE Frameworks and KDEPIM.
> 
> 
> Repository: kpeople
> 
> 
> Description
> ---
> 
> For filtering multiple properties the logic needs to be OR
> rather than AND.
> 
> For example in Plasma Mobile, one can start a conversation
> either with a phonenumber contact or IM contact, so both
> properties need to be requested. But with AND matching it
> won't return anything (or only those contacts that have both
> and user merged them).
> 
> 
> Diffs
> -
> 
>   src/personssortfilterproxymodel.cpp b011cbd 
> 
> Diff: https://git.reviewboard.kde.org/r/127117/diff/
> 
> 
> Testing
> ---
> 
> I get both phonenumber and IM contacts in Plasma Mobile
> 
> 
> 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 127127: KOpenWithDialog: show mimetype name and comment in "Remember" checkbox text

[David Faure]

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

(Updated Feb. 22, 2016, 12:28 a.m.)


Review request for KDE Frameworks and KDE Usability.


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


Repository: kio


Description
---

BUG: 110146


Diffs
-

  src/widgets/kopenwithdialog.cpp 808af73ff332834e013bd3727fee54183eee538b 

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


Testing
---

kio/tests/kopenwithtest


File Attachments


screenshot
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/02/20/b3a0babf-43b5-4921-ab7a-899c011c115c__kopenwith-mime.png


Thanks,

David Faure

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

Re: RCC for icons - update: Re: Icons installed by apps

[Jaroslaw Staniek]

On 18 February 2016 at 18:25, Kåre Särs  wrote:

> On Thursday, February 18, 2016 02:51:58 PM René J.V. Bertin wrote:
> > On Thursday February 18 2016 13:55:18 Jaroslaw Staniek wrote:
> > >>> > since QIcon::fromTheme() apaprently isn't able to find app icons.
> >
> > Care to explain? QIcon::fromTheme() doesn't find anything "out of the
> box"
> > on OS X (and I presume on MS Windows), but that is only because no theme
> > search path is set on those platforms. When you add the standard XDG icon
> > repository to the icon search path on OS X, even "pure" Qt5 application
> > will start showing icons all over if you have an icon theme installed --
> > including in widgets that should not have icons according to the HIG.
> Also
> > on OS X, fromTheme() will only return the application icon (as in the
> icon
> > shown in the Finder) if the current theme defines that same icon for the
> > calling application, and the theme search path is set of course. In all
> > other cases it will not, because the application icon is not defined
> > through a theme on OS X (nor is it on MS Windows, I presume).
> > >> I think the solution with a packaged breeze icons resource working
> > >> out-of-the box could be a good (lightweight) addition for non-Plasma
> > >> (non-Linux?) users of KF5, to popularize KF5. They grab the icons
> package
> >
> > Popularise, with Breeze "art"work? O:-)
> > Anyway,  I don't think "grabbing an icon package" will work on OS X, not
> if
> > you want to create standalone app bundles which by definition contain
> > everything they need.
> > >> and icons just work without thousands of files, caching, etc. 'One in
> a
> > >> million' would of these users would be interested in theming.
> >
> > I'd up that estimate if we're still talking about Breeze icons here O:-)
> >
> > >> PS2: I have been beaten by situations such as KToolBar setting 0-size
> > >> icons by default.
> >
> > Partly this is because almost no KF5 code uses the fallback argument of
> > QIcon::fromTheme() explicitly, which means that the function returns an
> > empty icon if the search fails. In particular, statements like
> >
> > app->setWindowIcon(QIcon::fromTheme(programName))
> >
> > should read
> >
> > app->setWindowIcon(QIcon::fromTheme(programName, app->windowIcon()))
> >
> > >> This is in a Windows env where no themes are (properly) installed. If
> the
> > >> rcc-based solution is in use I would imagine that ideally all the KF5
> > >> code
> > >> detects this somehow and would not look for xdg standard themes
> through
> > >> the
> > >> classic KIconLoader but silently adapt so the rcc resource works
> great.
> > >> Just a dream.
> >
> > If your rcc resource corresponds to the resource mentioned in the
> > QIcon::fromTheme() documentation (I think that talks about "qrc") and if
> I
> > interpret that documentation correctly then yes, code using that function
> > will find icons from the rcc/qrc "builtin" resource over those in xdg
> > themes (if the XDG icon repository is even in the icon theme search
> path).
> > >>> What I don't know however is whether artists consider that these
> icons
> > >>> should be themeable...
> >
> > I think icon artists will consider that you should touch their icons (for
> > theming or anything else). They will probably also consider that their
> > icons are the "best" but they really should also consider it a right for
> > anyone to use other icons ;)
> >
>
> The breeze.rcc file way is actually how Christoph solved it in Kate for
> Windows and OSX. We create an .rcc file from the breeze icons and at
> start-up
> we search for the file in QStandardPaths::DataLocation and if the file is
> found we load it with  registerResource() and add ":/icons" to the
> themeSearchPaths. (kate.git/icons.h)
>
> Christoph also added ":/icons" to the search path in kicontheme so that
> xmlgui
> toolbars and friends also get the icons from the .rcc file.
>
> We get all the same icons as on Linux neatly in one file :)
>
> breesze's symlinks was a problem on windows, but I created a small
> application
> to replace the symlinked files with an alias pointing to the target file ->
> smaller .rcc file :)
>

Thanks for reminding this, ​​Kåre​!
I went this way and started with manual creation of rcc on Windows.
Then I've added some cmake code for creation of an rcc for app icons
(Kexi).
I think the code would be usable to the same for breeze_icons.rcc and have
it in breeze-icons.git, and maybe by default for non-XDG platforms. So
people can build the fresh file very easily. Then a more flexible loading
code could be put into the kiconthemes, and we're mostly done. Things
should work with qmake too, so the list of projects potentially interested
-- grows.

Now I have two rcc files, registering app's icons rcc first, then the
global breeze one, what gives name overriding as expected. Not sure I will
want to override eventually, it's not good idea but nice to have as
emergency (I needed it once).

I'll be 

Re: Review Request 126991: Try multiple authentication methods in case of failures

[Krzysztof Nowicki]

> On Feb. 21, 2016, 7:07 p.m., Andreas Hartmetz wrote:
> > m_triedPasswords doesn't really tell the truth if the application didn't 
> > supply a password, right? It is not really a number of tried passwords, it 
> > is the state of a little state machine. In such cases, it is better to make 
> > it an enum: enum TriedCredentials { NoCredentials = 0, JobCredentials, 
> > CachedCredentials, UserInputCredentials };
> > ... where the distinction of Cached and UserInput is currently unnecessary 
> > but more consistent, a nice piece of self-documentation and might be 
> > helpful in the future.
> > (Note: "credentials" is better than "passwords" because it's a combination 
> > of username and password and i some special cases something different, like 
> > with NTLM and Kerberos)
> > Additionally, it looks like m_triedPasswords will carry over failed 
> > attempts from proxy authentication to web server authentication if both 
> > happen in the same KIO get() job [which may produce several HTTP GETs]. I'm 
> > not completely sure though because it's been a while since I worked on that 
> > code. Did you consider the problem?

You're right in both cases (field naming & proxy auth). Thanks for spotting 
this. I'll update the patch to fix the issues you've indicated.


- Krzysztof


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


On Feb. 9, 2016, 10:53 p.m., Krzysztof Nowicki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126991/
> ---
> 
> (Updated Feb. 9, 2016, 10:53 p.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When authenticating agains a server offering multiple authentication methods 
> make sure to attempt other methods in case the best one fails.
> 
> This also fixes a connection close issue in the middle of an NTLM 
> authentication dialog due to clearing the password.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.h 621b2c7a957b9bc9cc14ff13ed3c3a72dec38190 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/126991/diff/
> 
> 
> Testing
> ---
> 
> I have performed testing on an IIS 7.5 server which offered 3 authentication 
> options: Negotiate, NTLM and Basic. Since I have Kerberos configured the 
> original code would only try Negotiate and because it failed it would retry 
> it endlessly. With this patch authentication correctly falls back to NTLM or 
> Basic (if NTLM fails too).
> 
> 
> Thanks,
> 
> Krzysztof Nowicki
> 
>

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

Jenkins-kde-ci: kcoreaddons master stable-kf5-qt5 » Linux,gcc - Build # 113 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/113/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 21 Feb 2016 19:27:32 +
Build duration: 2 min 37 sec

CHANGE SET
Revision 8ed22237eb362c4af2fcb09c081a6831028c2caa by David Faure: (silence gcc)
  change: edit src/lib/io/kdirwatch.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 21 test(s), Skipped: 0 test(s), Total: 
21 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 10/10 (100%)FILES 76/90 (84%)CLASSES 76/90 (84%)LINE 5551/7709 
(72%)CONDITIONAL 12041/22407 (54%)

By packages
  
autotests
FILES 31/38 (82%)CLASSES 31/38 (82%)LINE 2384/2488 
(96%)CONDITIONAL 7273/14397 (51%)
src.desktoptojson
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 84/106 (79%)CONDITIONAL 
214/417 (51%)
src.lib
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 236/498 (47%)CONDITIONAL 
204/383 (53%)
src.lib.caching
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 352/764 (46%)CONDITIONAL 
202/384 (53%)
src.lib.io
FILES 9/10 (90%)CLASSES 9/10 (90%)LINE 740/1225 
(60%)CONDITIONAL 896/1470 (61%)
src.lib.jobs
FILES 5/7 (71%)CLASSES 5/7 (71%)LINE 155/303 (51%)CONDITIONAL 
55/92 (60%)
src.lib.plugin
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 629/726 (87%)CONDITIONAL 
1195/2045 (58%)
src.lib.randomness
FILES 1/2 (50%)CLASSES 1/2 (50%)LINE 21/98 (21%)CONDITIONAL 
22/36 (61%)
src.lib.text
FILES 5/8 (63%)CLASSES 5/8 (63%)LINE 312/729 (43%)CONDITIONAL 
1133/1833 (62%)
src.lib.util
FILES 10/10 (100%)CLASSES 10/10 (100%)LINE 638/772 
(83%)CONDITIONAL 847/1350 (63%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kcoreaddons master stable-kf5-qt5 » Linux,gcc - Build # 113 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/113/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 21 Feb 2016 19:27:32 +
Build duration: 2 min 37 sec

CHANGE SET
Revision 8ed22237eb362c4af2fcb09c081a6831028c2caa by David Faure: (silence gcc)
  change: edit src/lib/io/kdirwatch.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 21 test(s), Skipped: 0 test(s), Total: 
21 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 10/10 (100%)FILES 76/90 (84%)CLASSES 76/90 (84%)LINE 5551/7709 
(72%)CONDITIONAL 12041/22407 (54%)

By packages
  
autotests
FILES 31/38 (82%)CLASSES 31/38 (82%)LINE 2384/2488 
(96%)CONDITIONAL 7273/14397 (51%)
src.desktoptojson
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 84/106 (79%)CONDITIONAL 
214/417 (51%)
src.lib
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 236/498 (47%)CONDITIONAL 
204/383 (53%)
src.lib.caching
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 352/764 (46%)CONDITIONAL 
202/384 (53%)
src.lib.io
FILES 9/10 (90%)CLASSES 9/10 (90%)LINE 740/1225 
(60%)CONDITIONAL 896/1470 (61%)
src.lib.jobs
FILES 5/7 (71%)CLASSES 5/7 (71%)LINE 155/303 (51%)CONDITIONAL 
55/92 (60%)
src.lib.plugin
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 629/726 (87%)CONDITIONAL 
1195/2045 (58%)
src.lib.randomness
FILES 1/2 (50%)CLASSES 1/2 (50%)LINE 21/98 (21%)CONDITIONAL 
22/36 (61%)
src.lib.text
FILES 5/8 (63%)CLASSES 5/8 (63%)LINE 312/729 (43%)CONDITIONAL 
1133/1833 (62%)
src.lib.util
FILES 10/10 (100%)CLASSES 10/10 (100%)LINE 638/772 
(83%)CONDITIONAL 847/1350 (63%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kcoreaddons master kf5-qt5 » Linux,gcc - Build # 118 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/118/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 21 Feb 2016 19:27:32 +
Build duration: 2 min 5 sec

CHANGE SET
Revision 8ed22237eb362c4af2fcb09c081a6831028c2caa by David Faure: (silence gcc)
  change: edit src/lib/io/kdirwatch.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 21 test(s), Skipped: 0 test(s), Total: 
21 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 10/10 (100%)FILES 76/90 (84%)CLASSES 76/90 (84%)LINE 5551/7711 
(72%)CONDITIONAL 11576/21479 (54%)

By packages
  
autotests
FILES 31/38 (82%)CLASSES 31/38 (82%)LINE 2384/2488 
(96%)CONDITIONAL 7038/13927 (51%)
src.desktoptojson
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 84/108 (78%)CONDITIONAL 
195/380 (51%)
src.lib
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 236/498 (47%)CONDITIONAL 
142/259 (55%)
src.lib.caching
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 352/764 (46%)CONDITIONAL 
200/380 (53%)
src.lib.io
FILES 9/10 (90%)CLASSES 9/10 (90%)LINE 740/1225 
(60%)CONDITIONAL 867/1412 (61%)
src.lib.jobs
FILES 5/7 (71%)CLASSES 5/7 (71%)LINE 155/303 (51%)CONDITIONAL 
55/92 (60%)
src.lib.plugin
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 629/726 (87%)CONDITIONAL 
1144/1944 (59%)
src.lib.randomness
FILES 1/2 (50%)CLASSES 1/2 (50%)LINE 21/98 (21%)CONDITIONAL 
20/32 (63%)
src.lib.text
FILES 5/8 (63%)CLASSES 5/8 (63%)LINE 312/729 (43%)CONDITIONAL 
1099/1765 (62%)
src.lib.util
FILES 10/10 (100%)CLASSES 10/10 (100%)LINE 638/772 
(83%)CONDITIONAL 816/1288 (63%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kcoreaddons master kf5-qt5 » Linux,gcc - Build # 118 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/118/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 21 Feb 2016 19:27:32 +
Build duration: 2 min 5 sec

CHANGE SET
Revision 8ed22237eb362c4af2fcb09c081a6831028c2caa by David Faure: (silence gcc)
  change: edit src/lib/io/kdirwatch.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 21 test(s), Skipped: 0 test(s), Total: 
21 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 10/10 (100%)FILES 76/90 (84%)CLASSES 76/90 (84%)LINE 5551/7711 
(72%)CONDITIONAL 11576/21479 (54%)

By packages
  
autotests
FILES 31/38 (82%)CLASSES 31/38 (82%)LINE 2384/2488 
(96%)CONDITIONAL 7038/13927 (51%)
src.desktoptojson
FILES 3/3 (100%)CLASSES 3/3 (100%)LINE 84/108 (78%)CONDITIONAL 
195/380 (51%)
src.lib
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 236/498 (47%)CONDITIONAL 
142/259 (55%)
src.lib.caching
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 352/764 (46%)CONDITIONAL 
200/380 (53%)
src.lib.io
FILES 9/10 (90%)CLASSES 9/10 (90%)LINE 740/1225 
(60%)CONDITIONAL 867/1412 (61%)
src.lib.jobs
FILES 5/7 (71%)CLASSES 5/7 (71%)LINE 155/303 (51%)CONDITIONAL 
55/92 (60%)
src.lib.plugin
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 629/726 (87%)CONDITIONAL 
1144/1944 (59%)
src.lib.randomness
FILES 1/2 (50%)CLASSES 1/2 (50%)LINE 21/98 (21%)CONDITIONAL 
20/32 (63%)
src.lib.text
FILES 5/8 (63%)CLASSES 5/8 (63%)LINE 312/729 (43%)CONDITIONAL 
1099/1765 (62%)
src.lib.util
FILES 10/10 (100%)CLASSES 10/10 (100%)LINE 638/772 
(83%)CONDITIONAL 816/1288 (63%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127131: kcoreaddons: Fix many clang warnings

[David Faure]

> On Feb. 21, 2016, 6:49 p.m., Michael Pyne wrote:
> > Jenkins has reported a build failure 
> > (https://build.kde.org/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/117/console),
> >  looks like we might have dueling compiler errors:
> > 
> > 18:07:28 
> > /home/jenkins/builds/kcoreaddons/kf5-qt5/src/lib/io/kdirwatch.cpp:137:1: 
> > error: control reaches end of non-void function [-Werror=return-type]
> > 
> > Seems that the GCC used on CI doesn't recognize that every possible error 
> > case is handled in the switch (or alternately, it assumes that someone 
> > might try to shoehorn an out-of-range enumeration value into the function). 
> > GCC 5.3 works fine, which is what I'd used for testing. But it might be 
> > easier to revert the switch/case portions of the patch, perhaps add warning 
> > suppressions instead?

Thanks for the heads up. I added a return statement after the switch, should 
keep both compilers happy.


- David


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


On Feb. 21, 2016, 5:39 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127131/
> ---
> 
> (Updated Feb. 21, 2016, 5:39 p.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> (C casts, global functions without static, unused default statements...)
> 
> This detected a small bug in kdirwatch, where fall-through was unintended
> (I know, because I wrote that code).
> 
> 
> Diffs
> -
> 
>   autotests/kjobtest.cpp aedde39f35ec7bafeb13ffd33d8eb871f60b29eb 
>   autotests/kprocesstest.cpp 2ac756c973b12b895af6fa34ff5077d2d064af8b 
>   src/desktoptojson/main.cpp 45bdcba8f5e7ab395f52ebf8d31ccde826e77434 
>   src/lib/caching/kshareddatacache.cpp 
> 0c1c61cc7f70969748d102c8313fadc73cef55a5 
>   src/lib/io/kautosavefile.cpp e540630179b17b18b7dce5480969aab89d45be34 
>   src/lib/io/kdirwatch.cpp 4b900228c74c609d26e52542bd42e124c6d1d39b 
>   src/lib/io/kdirwatch_p.h af74ff15869b6f9c16bb14f81b6c488dd082de19 
>   src/lib/io/kfilesystemtype.cpp 3526de8ab710f31dabad09026b9b993785658033 
>   src/lib/io/kmessage.h 590a1fe961a220bb2e879b35b4936bf9a1cdfdbb 
>   src/lib/io/kprocess.cpp 2bd8aee1293adf3b5fe549fa2cff675160a5ea96 
>   src/lib/jobs/kjob.cpp 1ad2c2858ee7ed2167a740a10fb7b185d8833288 
>   src/lib/kaboutdata.cpp 3193808797c53586bea94768dc81867e35a8a535 
>   src/lib/plugin/desktopfileparser.cpp 
> 1122af8ba3640a7f06819063157069b652a4f904 
>   src/lib/plugin/desktopfileparser_p.h 
> f19be800e5af2cc120a8ad4f9561e92dbf9fd2c7 
>   src/lib/plugin/kpluginloader.cpp 530e7b865022b680a473fceceb7d659196e9420d 
>   src/lib/randomness/krandom.cpp 93b917d23c2ab8ec7a10277ea8bd49d6f9a61fd7 
>   src/lib/randomness/krandomsequence.cpp 
> 01490bfac5e5fda0ba509fb791bdd1e791f90cbd 
>   src/lib/text/kmacroexpander_unix.cpp 
> f773291ffe961c1a01235eb6eaf79488461eac59 
>   src/lib/text/ktexttohtml.cpp 0fe2a2d69de1d42341cb3ad50c236c21c0aa1f06 
>   src/lib/text/ktexttohtmlemoticonsinterface.h 
> 8fcd30f6c93424aeb103243a0e080798a56fdcc1 
>   src/lib/util/kformatprivate.cpp 9af072dcf3068d8056582be046e38bab48a548a5 
>   src/lib/util/kshell.cpp 0f2e1d46e0761320b871949da594a9b60e2feaf5 
>   src/lib/util/kuser_unix.cpp 01120f6928e04682ca11c0a0822047b43c2f047b 
> 
> Diff: https://git.reviewboard.kde.org/r/127131/diff/
> 
> 
> Testing
> ---
> 
> Compiled with clang; ran autotests.
> 
> 
> 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 127131: kcoreaddons: Fix many clang warnings

[Michael Pyne]

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



Jenkins has reported a build failure 
(https://build.kde.org/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/117/console),
 looks like we might have dueling compiler errors:

18:07:28 
/home/jenkins/builds/kcoreaddons/kf5-qt5/src/lib/io/kdirwatch.cpp:137:1: error: 
control reaches end of non-void function [-Werror=return-type]

Seems that the GCC used on CI doesn't recognize that every possible error case 
is handled in the switch (or alternately, it assumes that someone might try to 
shoehorn an out-of-range enumeration value into the function). GCC 5.3 works 
fine, which is what I'd used for testing. But it might be easier to revert the 
switch/case portions of the patch, perhaps add warning suppressions instead?

- Michael Pyne


On Feb. 21, 2016, 5:39 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127131/
> ---
> 
> (Updated Feb. 21, 2016, 5:39 p.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> (C casts, global functions without static, unused default statements...)
> 
> This detected a small bug in kdirwatch, where fall-through was unintended
> (I know, because I wrote that code).
> 
> 
> Diffs
> -
> 
>   autotests/kjobtest.cpp aedde39f35ec7bafeb13ffd33d8eb871f60b29eb 
>   autotests/kprocesstest.cpp 2ac756c973b12b895af6fa34ff5077d2d064af8b 
>   src/desktoptojson/main.cpp 45bdcba8f5e7ab395f52ebf8d31ccde826e77434 
>   src/lib/caching/kshareddatacache.cpp 
> 0c1c61cc7f70969748d102c8313fadc73cef55a5 
>   src/lib/io/kautosavefile.cpp e540630179b17b18b7dce5480969aab89d45be34 
>   src/lib/io/kdirwatch.cpp 4b900228c74c609d26e52542bd42e124c6d1d39b 
>   src/lib/io/kdirwatch_p.h af74ff15869b6f9c16bb14f81b6c488dd082de19 
>   src/lib/io/kfilesystemtype.cpp 3526de8ab710f31dabad09026b9b993785658033 
>   src/lib/io/kmessage.h 590a1fe961a220bb2e879b35b4936bf9a1cdfdbb 
>   src/lib/io/kprocess.cpp 2bd8aee1293adf3b5fe549fa2cff675160a5ea96 
>   src/lib/jobs/kjob.cpp 1ad2c2858ee7ed2167a740a10fb7b185d8833288 
>   src/lib/kaboutdata.cpp 3193808797c53586bea94768dc81867e35a8a535 
>   src/lib/plugin/desktopfileparser.cpp 
> 1122af8ba3640a7f06819063157069b652a4f904 
>   src/lib/plugin/desktopfileparser_p.h 
> f19be800e5af2cc120a8ad4f9561e92dbf9fd2c7 
>   src/lib/plugin/kpluginloader.cpp 530e7b865022b680a473fceceb7d659196e9420d 
>   src/lib/randomness/krandom.cpp 93b917d23c2ab8ec7a10277ea8bd49d6f9a61fd7 
>   src/lib/randomness/krandomsequence.cpp 
> 01490bfac5e5fda0ba509fb791bdd1e791f90cbd 
>   src/lib/text/kmacroexpander_unix.cpp 
> f773291ffe961c1a01235eb6eaf79488461eac59 
>   src/lib/text/ktexttohtml.cpp 0fe2a2d69de1d42341cb3ad50c236c21c0aa1f06 
>   src/lib/text/ktexttohtmlemoticonsinterface.h 
> 8fcd30f6c93424aeb103243a0e080798a56fdcc1 
>   src/lib/util/kformatprivate.cpp 9af072dcf3068d8056582be046e38bab48a548a5 
>   src/lib/util/kshell.cpp 0f2e1d46e0761320b871949da594a9b60e2feaf5 
>   src/lib/util/kuser_unix.cpp 01120f6928e04682ca11c0a0822047b43c2f047b 
> 
> Diff: https://git.reviewboard.kde.org/r/127131/diff/
> 
> 
> Testing
> ---
> 
> Compiled with clang; ran autotests.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Jenkins-kde-ci: kcoreaddons master kf5-qt5 » Linux,gcc - Build # 117 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/117/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 21 Feb 2016 17:40:21 +
Build duration: 58 sec

CHANGE SET
Revision 12eb1ab1224bb7a10df3520d9a2617b5032fe31c by David Faure: (Fix many 
clang warnings)
  change: edit src/lib/plugin/kpluginloader.cpp
  change: edit src/lib/io/kdirwatch.cpp
  change: edit autotests/kjobtest.cpp
  change: edit src/lib/jobs/kjob.cpp
  change: edit src/lib/plugin/desktopfileparser_p.h
  change: edit src/lib/text/ktexttohtmlemoticonsinterface.h
  change: edit src/lib/plugin/desktopfileparser.cpp
  change: edit src/lib/io/kdirwatch_p.h
  change: edit src/lib/text/kmacroexpander_unix.cpp
  change: edit src/lib/io/kmessage.h
  change: edit src/lib/util/kshell.cpp
  change: edit src/lib/randomness/krandomsequence.cpp
  change: edit src/lib/util/kuser_unix.cpp
  change: edit src/lib/io/kfilesystemtype.cpp
  change: edit src/lib/util/kformatprivate.cpp
  change: edit src/lib/caching/kshareddatacache.cpp
  change: edit src/lib/randomness/krandom.cpp
  change: edit src/lib/kaboutdata.cpp
  change: edit src/lib/text/ktexttohtml.cpp
  change: edit autotests/kprocesstest.cpp
  change: edit src/lib/io/kautosavefile.cpp
  change: edit src/desktoptojson/main.cpp
  change: edit src/lib/io/kprocess.cpp
Revision 3b06113b4fa7baca92fe02c6ba93180b48519408 by David Faure: (autotest: 
fix wrong QUrl construction from local path; fix typo)
  change: edit src/lib/io/kautosavefile.cpp
  change: edit autotests/kautosavefiletest.cpp
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126991: Try multiple authentication methods in case of failures

[Andreas Hartmetz]

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



m_triedPasswords doesn't really tell the truth if the application didn't supply 
a password, right? It is not really a number of tried passwords, it is the 
state of a little state machine. In such cases, it is better to make it an 
enum: enum TriedCredentials { NoCredentials = 0, JobCredentials, 
CachedCredentials, UserInputCredentials };
... where the distinction of Cached and UserInput is currently unnecessary but 
more consistent, a nice piece of self-documentation and might be helpful in the 
future.
(Note: "credentials" is better than "passwords" because it's a combination of 
username and password and i some special cases something different, like with 
NTLM and Kerberos)
Additionally, it looks like m_triedPasswords will carry over failed attempts 
from proxy authentication to web server authentication if both happen in the 
same KIO get() job [which may produce several HTTP GETs]. I'm not completely 
sure though because it's been a while since I worked on that code. Did you 
consider the problem?


src/ioslaves/http/http.cpp (line 5445)


} else {



src/ioslaves/http/http.cpp (line 5476)


Please break this line, it's extremely long with the addition.


- Andreas Hartmetz


On Feb. 9, 2016, 9:53 p.m., Krzysztof Nowicki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126991/
> ---
> 
> (Updated Feb. 9, 2016, 9:53 p.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When authenticating agains a server offering multiple authentication methods 
> make sure to attempt other methods in case the best one fails.
> 
> This also fixes a connection close issue in the middle of an NTLM 
> authentication dialog due to clearing the password.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.h 621b2c7a957b9bc9cc14ff13ed3c3a72dec38190 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/126991/diff/
> 
> 
> Testing
> ---
> 
> I have performed testing on an IIS 7.5 server which offered 3 authentication 
> options: Negotiate, NTLM and Basic. Since I have Kerberos configured the 
> original code would only try Negotiate and because it failed it would retry 
> it endlessly. With this patch authentication correctly falls back to NTLM or 
> Basic (if NTLM fails too).
> 
> 
> Thanks,
> 
> Krzysztof Nowicki
> 
>

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

Jenkins-kde-ci: kcoreaddons master stable-kf5-qt5 » Linux,gcc - Build # 112 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kcoreaddons%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/112/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Sun, 21 Feb 2016 17:40:21 +
Build duration: 50 sec

CHANGE SET
Revision 12eb1ab1224bb7a10df3520d9a2617b5032fe31c by David Faure: (Fix many 
clang warnings)
  change: edit src/lib/plugin/desktopfileparser.cpp
  change: edit src/lib/io/kdirwatch_p.h
  change: edit src/lib/io/kdirwatch.cpp
  change: edit src/lib/util/kuser_unix.cpp
  change: edit src/lib/plugin/desktopfileparser_p.h
  change: edit src/lib/text/ktexttohtml.cpp
  change: edit src/lib/text/kmacroexpander_unix.cpp
  change: edit src/lib/io/kmessage.h
  change: edit src/lib/util/kshell.cpp
  change: edit src/lib/io/kprocess.cpp
  change: edit src/lib/randomness/krandomsequence.cpp
  change: edit autotests/kprocesstest.cpp
  change: edit autotests/kjobtest.cpp
  change: edit src/desktoptojson/main.cpp
  change: edit src/lib/io/kautosavefile.cpp
  change: edit src/lib/kaboutdata.cpp
  change: edit src/lib/util/kformatprivate.cpp
  change: edit src/lib/jobs/kjob.cpp
  change: edit src/lib/caching/kshareddatacache.cpp
  change: edit src/lib/text/ktexttohtmlemoticonsinterface.h
  change: edit src/lib/io/kfilesystemtype.cpp
  change: edit src/lib/randomness/krandom.cpp
  change: edit src/lib/plugin/kpluginloader.cpp
Revision 3b06113b4fa7baca92fe02c6ba93180b48519408 by David Faure: (autotest: 
fix wrong QUrl construction from local path; fix typo)
  change: edit src/lib/io/kautosavefile.cpp
  change: edit autotests/kautosavefiletest.cpp
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127033: Add /../share/hunspell/ to dictionary search path

[David Faure]

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




src/plugins/hunspell/hunspelldict.cpp (line 40)


The QLatin1String here looks wrong, this should be QFile::decodeName().



src/plugins/hunspell/hunspelldict.cpp (line 42)


if (!QFileInfo::exists(dic)) is faster



src/plugins/hunspell/hunspelldict.cpp (line 44)


And then this could be the same as line 40, both would then be:

dic = QFile::decodeName(dirPath) + lang + QStringLiteral(".dic");



src/plugins/hunspell/hunspelldict.cpp (line 55)


I've seen this before maybe it should be a function ;)


- David Faure


On Feb. 10, 2016, 6:45 p.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127033/
> ---
> 
> (Updated Feb. 10, 2016, 6:45 p.m.)
> 
> 
> Review request for KDE Frameworks, Laurent Montel and Martin Tobias Holmedahl 
> Sandsmark.
> 
> 
> Repository: sonnet
> 
> 
> Description
> ---
> 
> The default search path for hunspell dictionaries is /usr/share/hunspell/, 
> but that is not a very common directory on windows ;)
> 
> For Windows and Mac there are some extra search paths defined in side 
> #ifdefs, but they are hardcoded to the LibreOffice dictionary directories. 
> Unfortunately the extra Windows path points to a 32bit LibreOffice 5 on a 64 
> bit Windows directory. This patch adds /../share/hunspell/ to the 
> search path in case the default is not found. 
> 
> If wanted I can add a CMake define for specifying the path relative to the 
> application directory.
> 
> 
> Diffs
> -
> 
>   src/core/guesslanguage.cpp e8deb90 
>   src/plugins/hunspell/hunspellclient.cpp 3da7219 
>   src/plugins/hunspell/hunspelldict.cpp bc5c314 
> 
> Diff: https://git.reviewboard.kde.org/r/127033/diff/
> 
> 
> Testing
> ---
> 
> Compiled Sonnet and Kate on Windows and got spelling to work :)
> 
> It is now possible to add dictionaries to a relocatable  dir>/../share/hunspell/ and get spelling to work without depending on 
> LibreOffice installation on Windows.
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

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

Re: Review Request 127131: kcoreaddons: Fix many clang warnings

[David Faure]

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

(Updated Feb. 21, 2016, 5:39 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Michael Pyne.


Changes
---

Submitted with commit 12eb1ab1224bb7a10df3520d9a2617b5032fe31c by David Faure 
to branch master.


Repository: kcoreaddons


Description
---

(C casts, global functions without static, unused default statements...)

This detected a small bug in kdirwatch, where fall-through was unintended
(I know, because I wrote that code).


Diffs
-

  autotests/kjobtest.cpp aedde39f35ec7bafeb13ffd33d8eb871f60b29eb 
  autotests/kprocesstest.cpp 2ac756c973b12b895af6fa34ff5077d2d064af8b 
  src/desktoptojson/main.cpp 45bdcba8f5e7ab395f52ebf8d31ccde826e77434 
  src/lib/caching/kshareddatacache.cpp 0c1c61cc7f70969748d102c8313fadc73cef55a5 
  src/lib/io/kautosavefile.cpp e540630179b17b18b7dce5480969aab89d45be34 
  src/lib/io/kdirwatch.cpp 4b900228c74c609d26e52542bd42e124c6d1d39b 
  src/lib/io/kdirwatch_p.h af74ff15869b6f9c16bb14f81b6c488dd082de19 
  src/lib/io/kfilesystemtype.cpp 3526de8ab710f31dabad09026b9b993785658033 
  src/lib/io/kmessage.h 590a1fe961a220bb2e879b35b4936bf9a1cdfdbb 
  src/lib/io/kprocess.cpp 2bd8aee1293adf3b5fe549fa2cff675160a5ea96 
  src/lib/jobs/kjob.cpp 1ad2c2858ee7ed2167a740a10fb7b185d8833288 
  src/lib/kaboutdata.cpp 3193808797c53586bea94768dc81867e35a8a535 
  src/lib/plugin/desktopfileparser.cpp 1122af8ba3640a7f06819063157069b652a4f904 
  src/lib/plugin/desktopfileparser_p.h f19be800e5af2cc120a8ad4f9561e92dbf9fd2c7 
  src/lib/plugin/kpluginloader.cpp 530e7b865022b680a473fceceb7d659196e9420d 
  src/lib/randomness/krandom.cpp 93b917d23c2ab8ec7a10277ea8bd49d6f9a61fd7 
  src/lib/randomness/krandomsequence.cpp 
01490bfac5e5fda0ba509fb791bdd1e791f90cbd 
  src/lib/text/kmacroexpander_unix.cpp f773291ffe961c1a01235eb6eaf79488461eac59 
  src/lib/text/ktexttohtml.cpp 0fe2a2d69de1d42341cb3ad50c236c21c0aa1f06 
  src/lib/text/ktexttohtmlemoticonsinterface.h 
8fcd30f6c93424aeb103243a0e080798a56fdcc1 
  src/lib/util/kformatprivate.cpp 9af072dcf3068d8056582be046e38bab48a548a5 
  src/lib/util/kshell.cpp 0f2e1d46e0761320b871949da594a9b60e2feaf5 
  src/lib/util/kuser_unix.cpp 01120f6928e04682ca11c0a0822047b43c2f047b 

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


Testing
---

Compiled with clang; ran autotests.


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 127131: kcoreaddons: Fix many clang warnings

[Michael Pyne]

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


Fix it, then Ship it!




Looked good to me (so marking as +1) though I do have a suggested improvement.


src/lib/jobs/kjob.cpp (line 302)


Since you're here anyways, probably best to redo the right-hand side to 
something like `100.0 * processedAmount / totalAmount`.

The cast to float is incomplete since the compiler still has to upcast to 
double (for 100.0, which should be 100.0f to be treated as a float), and moving 
the 100.0 to the beginning will force the rest of the expression to be treated 
as a double anyways.


- Michael Pyne


On Feb. 21, 2016, 10:30 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127131/
> ---
> 
> (Updated Feb. 21, 2016, 10:30 a.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> (C casts, global functions without static, unused default statements...)
> 
> This detected a small bug in kdirwatch, where fall-through was unintended
> (I know, because I wrote that code).
> 
> 
> Diffs
> -
> 
>   autotests/kjobtest.cpp aedde39f35ec7bafeb13ffd33d8eb871f60b29eb 
>   autotests/kprocesstest.cpp 2ac756c973b12b895af6fa34ff5077d2d064af8b 
>   src/desktoptojson/main.cpp 45bdcba8f5e7ab395f52ebf8d31ccde826e77434 
>   src/lib/caching/kshareddatacache.cpp 
> 0c1c61cc7f70969748d102c8313fadc73cef55a5 
>   src/lib/io/kautosavefile.cpp e540630179b17b18b7dce5480969aab89d45be34 
>   src/lib/io/kdirwatch.cpp 4b900228c74c609d26e52542bd42e124c6d1d39b 
>   src/lib/io/kdirwatch_p.h af74ff15869b6f9c16bb14f81b6c488dd082de19 
>   src/lib/io/kfilesystemtype.cpp 3526de8ab710f31dabad09026b9b993785658033 
>   src/lib/io/kmessage.h 590a1fe961a220bb2e879b35b4936bf9a1cdfdbb 
>   src/lib/io/kprocess.cpp 2bd8aee1293adf3b5fe549fa2cff675160a5ea96 
>   src/lib/jobs/kjob.cpp 1ad2c2858ee7ed2167a740a10fb7b185d8833288 
>   src/lib/kaboutdata.cpp 3193808797c53586bea94768dc81867e35a8a535 
>   src/lib/plugin/desktopfileparser.cpp 
> 1122af8ba3640a7f06819063157069b652a4f904 
>   src/lib/plugin/desktopfileparser_p.h 
> f19be800e5af2cc120a8ad4f9561e92dbf9fd2c7 
>   src/lib/plugin/kpluginloader.cpp 530e7b865022b680a473fceceb7d659196e9420d 
>   src/lib/randomness/krandom.cpp 93b917d23c2ab8ec7a10277ea8bd49d6f9a61fd7 
>   src/lib/randomness/krandomsequence.cpp 
> 01490bfac5e5fda0ba509fb791bdd1e791f90cbd 
>   src/lib/text/kmacroexpander_unix.cpp 
> f773291ffe961c1a01235eb6eaf79488461eac59 
>   src/lib/text/ktexttohtml.cpp 0fe2a2d69de1d42341cb3ad50c236c21c0aa1f06 
>   src/lib/text/ktexttohtmlemoticonsinterface.h 
> 8fcd30f6c93424aeb103243a0e080798a56fdcc1 
>   src/lib/util/kformatprivate.cpp 9af072dcf3068d8056582be046e38bab48a548a5 
>   src/lib/util/kshell.cpp 0f2e1d46e0761320b871949da594a9b60e2feaf5 
>   src/lib/util/kuser_unix.cpp 01120f6928e04682ca11c0a0822047b43c2f047b 
> 
> Diff: https://git.reviewboard.kde.org/r/127131/diff/
> 
> 
> Testing
> ---
> 
> Compiled with clang; ran autotests.
> 
> 
> 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 127131: kcoreaddons: Fix many clang warnings

[David Edmundson]

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


Ship it!




Ship It!

- David Edmundson


On Feb. 21, 2016, 10:30 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127131/
> ---
> 
> (Updated Feb. 21, 2016, 10:30 a.m.)
> 
> 
> Review request for KDE Frameworks and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> (C casts, global functions without static, unused default statements...)
> 
> This detected a small bug in kdirwatch, where fall-through was unintended
> (I know, because I wrote that code).
> 
> 
> Diffs
> -
> 
>   autotests/kjobtest.cpp aedde39f35ec7bafeb13ffd33d8eb871f60b29eb 
>   autotests/kprocesstest.cpp 2ac756c973b12b895af6fa34ff5077d2d064af8b 
>   src/desktoptojson/main.cpp 45bdcba8f5e7ab395f52ebf8d31ccde826e77434 
>   src/lib/caching/kshareddatacache.cpp 
> 0c1c61cc7f70969748d102c8313fadc73cef55a5 
>   src/lib/io/kautosavefile.cpp e540630179b17b18b7dce5480969aab89d45be34 
>   src/lib/io/kdirwatch.cpp 4b900228c74c609d26e52542bd42e124c6d1d39b 
>   src/lib/io/kdirwatch_p.h af74ff15869b6f9c16bb14f81b6c488dd082de19 
>   src/lib/io/kfilesystemtype.cpp 3526de8ab710f31dabad09026b9b993785658033 
>   src/lib/io/kmessage.h 590a1fe961a220bb2e879b35b4936bf9a1cdfdbb 
>   src/lib/io/kprocess.cpp 2bd8aee1293adf3b5fe549fa2cff675160a5ea96 
>   src/lib/jobs/kjob.cpp 1ad2c2858ee7ed2167a740a10fb7b185d8833288 
>   src/lib/kaboutdata.cpp 3193808797c53586bea94768dc81867e35a8a535 
>   src/lib/plugin/desktopfileparser.cpp 
> 1122af8ba3640a7f06819063157069b652a4f904 
>   src/lib/plugin/desktopfileparser_p.h 
> f19be800e5af2cc120a8ad4f9561e92dbf9fd2c7 
>   src/lib/plugin/kpluginloader.cpp 530e7b865022b680a473fceceb7d659196e9420d 
>   src/lib/randomness/krandom.cpp 93b917d23c2ab8ec7a10277ea8bd49d6f9a61fd7 
>   src/lib/randomness/krandomsequence.cpp 
> 01490bfac5e5fda0ba509fb791bdd1e791f90cbd 
>   src/lib/text/kmacroexpander_unix.cpp 
> f773291ffe961c1a01235eb6eaf79488461eac59 
>   src/lib/text/ktexttohtml.cpp 0fe2a2d69de1d42341cb3ad50c236c21c0aa1f06 
>   src/lib/text/ktexttohtmlemoticonsinterface.h 
> 8fcd30f6c93424aeb103243a0e080798a56fdcc1 
>   src/lib/util/kformatprivate.cpp 9af072dcf3068d8056582be046e38bab48a548a5 
>   src/lib/util/kshell.cpp 0f2e1d46e0761320b871949da594a9b60e2feaf5 
>   src/lib/util/kuser_unix.cpp 01120f6928e04682ca11c0a0822047b43c2f047b 
> 
> Diff: https://git.reviewboard.kde.org/r/127131/diff/
> 
> 
> Testing
> ---
> 
> Compiled with clang; ran autotests.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Review Request 127131: kcoreaddons: Fix many clang warnings

[David Faure]

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

Review request for KDE Frameworks and Michael Pyne.


Repository: kcoreaddons


Description
---

(C casts, global functions without static, unused default statements...)

This detected a small bug in kdirwatch, where fall-through was unintended
(I know, because I wrote that code).


Diffs
-

  autotests/kjobtest.cpp aedde39f35ec7bafeb13ffd33d8eb871f60b29eb 
  autotests/kprocesstest.cpp 2ac756c973b12b895af6fa34ff5077d2d064af8b 
  src/desktoptojson/main.cpp 45bdcba8f5e7ab395f52ebf8d31ccde826e77434 
  src/lib/caching/kshareddatacache.cpp 0c1c61cc7f70969748d102c8313fadc73cef55a5 
  src/lib/io/kautosavefile.cpp e540630179b17b18b7dce5480969aab89d45be34 
  src/lib/io/kdirwatch.cpp 4b900228c74c609d26e52542bd42e124c6d1d39b 
  src/lib/io/kdirwatch_p.h af74ff15869b6f9c16bb14f81b6c488dd082de19 
  src/lib/io/kfilesystemtype.cpp 3526de8ab710f31dabad09026b9b993785658033 
  src/lib/io/kmessage.h 590a1fe961a220bb2e879b35b4936bf9a1cdfdbb 
  src/lib/io/kprocess.cpp 2bd8aee1293adf3b5fe549fa2cff675160a5ea96 
  src/lib/jobs/kjob.cpp 1ad2c2858ee7ed2167a740a10fb7b185d8833288 
  src/lib/kaboutdata.cpp 3193808797c53586bea94768dc81867e35a8a535 
  src/lib/plugin/desktopfileparser.cpp 1122af8ba3640a7f06819063157069b652a4f904 
  src/lib/plugin/desktopfileparser_p.h f19be800e5af2cc120a8ad4f9561e92dbf9fd2c7 
  src/lib/plugin/kpluginloader.cpp 530e7b865022b680a473fceceb7d659196e9420d 
  src/lib/randomness/krandom.cpp 93b917d23c2ab8ec7a10277ea8bd49d6f9a61fd7 
  src/lib/randomness/krandomsequence.cpp 
01490bfac5e5fda0ba509fb791bdd1e791f90cbd 
  src/lib/text/kmacroexpander_unix.cpp f773291ffe961c1a01235eb6eaf79488461eac59 
  src/lib/text/ktexttohtml.cpp 0fe2a2d69de1d42341cb3ad50c236c21c0aa1f06 
  src/lib/text/ktexttohtmlemoticonsinterface.h 
8fcd30f6c93424aeb103243a0e080798a56fdcc1 
  src/lib/util/kformatprivate.cpp 9af072dcf3068d8056582be046e38bab48a548a5 
  src/lib/util/kshell.cpp 0f2e1d46e0761320b871949da594a9b60e2feaf5 
  src/lib/util/kuser_unix.cpp 01120f6928e04682ca11c0a0822047b43c2f047b 

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


Testing
---

Compiled with clang; ran autotests.


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 126774: Fix many warnings presented by clazy

[David Faure]

> On Feb. 21, 2016, 4:52 a.m., Nick Shaforostoff wrote:
> > src/core/kprotocolmanager.cpp, line 940
> > 
> >
> > please revert back to QLatin1String, as there is a special overload for 
> > it. if you really want to reduce number of [re]allocs then change it to one 
> > big assignment:
> > 
> > info += ... + ... + ... + ...;
> > 
> > and make sure QT_USE_STRINGBUILDER is defined

Yes there is an overload, but unlike startsWith/endsWith where it's just a 
comparison, here a conversion to 16 bit will have to happen, in order to 
append. So a 16 bit literal seems better.


- David


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


On Feb. 20, 2016, 8:41 p.m., Russell Greene wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126774/
> ---
> 
> (Updated Feb. 20, 2016, 8:41 p.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Fix many warnings presented by clazy
> 
> I ran the clazy static anaylizer on the codebase and fixed many of the errors.
> 
> Many of these were using QStringLiteral instead of QString, which removes an 
> allocation, including Q_OBJECT macros, adding const ref instad of const 
> lvalue, stuff like that.
> 
> 
> Diffs
> -
> 
>   autotests/http/httpheaderdispositiontest.cpp ac41656 
>   autotests/http/httpheadertokenizetest.cpp e57be09 
>   autotests/jobtest.cpp dfbfba9 
>   autotests/kacltest.cpp 2b88906 
>   autotests/kcookiejar/kcookiejartest.cpp dab6987 
>   autotests/kfileitemtest.cpp 49d11e1 
>   autotests/kfileplacesmodeltest.cpp effc95b 
>   autotests/klocalsocketservertest.cpp 9b7f246 
>   autotests/klocalsockettest.cpp 0d831f2 
>   autotests/kmountpointtest.cpp f6eccd8 
>   autotests/knewfilemenutest.cpp 1b11f15 
>   autotests/kprotocolinfotest.cpp 812f7f7 
>   autotests/ktcpsockettest.cpp 640d871 
>   autotests/kurlnavigatortest.cpp 1b295c3 
>   autotests/udsentry_benchmark.cpp 16c8b7e 
>   src/core/dataprotocol_p.h 784226b 
>   src/core/global.cpp eaa4264 
>   src/core/job_error.cpp 7943cb3 
>   src/core/kprotocolinfofactory.cpp 1329b6b 
>   src/core/kprotocolmanager.cpp bd42c9e 
>   src/core/ksambasharedata.cpp b63f518 
>   src/core/ksslcertificatemanager.cpp 07feda4 
>   src/core/restorejob.cpp ee5cca9 
>   src/core/sessiondata.cpp d21ab21 
>   src/core/simplejob.cpp 440fa62 
>   src/core/slave.cpp 5ae4d97 
>   src/core/slavebase.cpp a9bf648 
>   src/core/statjob.cpp e55e3b4 
>   src/core/storedtransferjob.cpp 3e86cb9 
>   src/core/tcpslavebase.cpp 2bd1c0f 
>   src/filewidgets/defaults-kfile.h 825a297 
>   src/filewidgets/defaultviewadapter_p.h 79381e4 
>   src/filewidgets/kdiroperator.cpp 9c2b6be 
>   src/filewidgets/kfilecopytomenu.cpp dab5db6 
>   src/filewidgets/kfileplacesitem.cpp 1748188 
>   src/filewidgets/kfileplacesmodel.cpp e9d37a9 
>   src/filewidgets/kfileplacesview.cpp 4121191 
>   src/filewidgets/knewfilemenu.cpp b249898 
>   src/filewidgets/kurlnavigator.cpp 64d2a6d 
>   src/ioslaves/ftp/ftp.cpp 7477a6a 
>   src/ioslaves/http/http.cpp 76da711 
>   src/ioslaves/http/kcookiejar/kcookiejar.cpp c06882a 
>   src/ioslaves/http/kcookiejar/kcookieserver.cpp 3e7dd8c 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/ktrash.cpp 298a201 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
>   src/kcms/kio/kcookiespolicyselectiondlg.cpp fcd0763 
>   src/kcms/kio/kproxydlg.cpp fe2583e 
>   src/kcms/kio/main.cpp 387cf89 
>   src/kcms/kio/useragentdlg.cpp a6560b5 
>   src/kcms/kio/useragentinfo.cpp 1459fcc 
>   src/kcms/kio/useragentselectordlg.cpp 3952761 
>   src/kioexec/main.cpp 6f4540f 
>   src/kntlm/kntlm.cpp ed6f388 
>   src/kpac/script.cpp 9b126ca 
>   src/kpasswdserver/kpasswdserver.cpp 7a53300 
>   src/kssld/kssld.cpp fcc3bed 
>   src/protocoltojson/main.cpp 05b9364 
>   src/urifilters/ikws/kuriikwsfiltereng.h c8fbcd2 
>   src/urifilters/ikws/searchprovider.cpp 8652997 
>   src/urifilters/ikws/searchproviderdlg.cpp 4effe08 
>   src/urifilters/localdomain/localdomainurifilter.cpp 4ebd1c0 
>   src/urifilters/shorturi/kshorturifilter.cpp c9024b0 
>   src/widgets/accessmanager.cpp 68f4a27 
>   src/widgets/accessmanagerreply_p.cpp 2ade6fd 
>   src/widgets/delegateanimationhandler.cpp 8e08a0c 
>   src/widgets/fileundomanager.cpp c9790ab 
>   src/widgets/kbuildsycocaprogressdialog.cpp 9ee0644 
>   src/widgets/kfileitemactions.cpp 4343da7 
>   src/widgets/kopenwithdialog.cpp 808af73 
>   src/widgets/kpropertiesdialog.cpp a4fdeb9 
>   src/widgets/krun.cpp af48473 
>   

Re: Review Request 126774: Fix many warnings presented by clazy

[David Faure]

> On Feb. 21, 2016, 4:52 a.m., Nick Shaforostoff wrote:
> > src/core/kprotocolinfofactory.cpp, line 114
> > 
> >
> > please use QLatin1String here: there is a special overload for it

Yes I already fixed the startsWith/endsWith cases before pushing the patch ;)


> On Feb. 21, 2016, 4:52 a.m., Nick Shaforostoff wrote:
> > src/core/kprotocolmanager.cpp, line 838
> > 
> >
> > please revert back to QLatin1String, as there is a special overload for 
> > it. if you really want to reduce number of [re]allocs then change it to one 
> > big assignment:
> > 
> > d->useragent = ... + ... + ... + ...;
> > 
> > and make sure QT_USE_STRINGBUILDER is defined

Isn't the string builder on by default these days?


- David


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


On Feb. 20, 2016, 8:41 p.m., Russell Greene wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126774/
> ---
> 
> (Updated Feb. 20, 2016, 8:41 p.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Fix many warnings presented by clazy
> 
> I ran the clazy static anaylizer on the codebase and fixed many of the errors.
> 
> Many of these were using QStringLiteral instead of QString, which removes an 
> allocation, including Q_OBJECT macros, adding const ref instad of const 
> lvalue, stuff like that.
> 
> 
> Diffs
> -
> 
>   autotests/http/httpheaderdispositiontest.cpp ac41656 
>   autotests/http/httpheadertokenizetest.cpp e57be09 
>   autotests/jobtest.cpp dfbfba9 
>   autotests/kacltest.cpp 2b88906 
>   autotests/kcookiejar/kcookiejartest.cpp dab6987 
>   autotests/kfileitemtest.cpp 49d11e1 
>   autotests/kfileplacesmodeltest.cpp effc95b 
>   autotests/klocalsocketservertest.cpp 9b7f246 
>   autotests/klocalsockettest.cpp 0d831f2 
>   autotests/kmountpointtest.cpp f6eccd8 
>   autotests/knewfilemenutest.cpp 1b11f15 
>   autotests/kprotocolinfotest.cpp 812f7f7 
>   autotests/ktcpsockettest.cpp 640d871 
>   autotests/kurlnavigatortest.cpp 1b295c3 
>   autotests/udsentry_benchmark.cpp 16c8b7e 
>   src/core/dataprotocol_p.h 784226b 
>   src/core/global.cpp eaa4264 
>   src/core/job_error.cpp 7943cb3 
>   src/core/kprotocolinfofactory.cpp 1329b6b 
>   src/core/kprotocolmanager.cpp bd42c9e 
>   src/core/ksambasharedata.cpp b63f518 
>   src/core/ksslcertificatemanager.cpp 07feda4 
>   src/core/restorejob.cpp ee5cca9 
>   src/core/sessiondata.cpp d21ab21 
>   src/core/simplejob.cpp 440fa62 
>   src/core/slave.cpp 5ae4d97 
>   src/core/slavebase.cpp a9bf648 
>   src/core/statjob.cpp e55e3b4 
>   src/core/storedtransferjob.cpp 3e86cb9 
>   src/core/tcpslavebase.cpp 2bd1c0f 
>   src/filewidgets/defaults-kfile.h 825a297 
>   src/filewidgets/defaultviewadapter_p.h 79381e4 
>   src/filewidgets/kdiroperator.cpp 9c2b6be 
>   src/filewidgets/kfilecopytomenu.cpp dab5db6 
>   src/filewidgets/kfileplacesitem.cpp 1748188 
>   src/filewidgets/kfileplacesmodel.cpp e9d37a9 
>   src/filewidgets/kfileplacesview.cpp 4121191 
>   src/filewidgets/knewfilemenu.cpp b249898 
>   src/filewidgets/kurlnavigator.cpp 64d2a6d 
>   src/ioslaves/ftp/ftp.cpp 7477a6a 
>   src/ioslaves/http/http.cpp 76da711 
>   src/ioslaves/http/kcookiejar/kcookiejar.cpp c06882a 
>   src/ioslaves/http/kcookiejar/kcookieserver.cpp 3e7dd8c 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/ktrash.cpp 298a201 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
>   src/kcms/kio/kcookiespolicyselectiondlg.cpp fcd0763 
>   src/kcms/kio/kproxydlg.cpp fe2583e 
>   src/kcms/kio/main.cpp 387cf89 
>   src/kcms/kio/useragentdlg.cpp a6560b5 
>   src/kcms/kio/useragentinfo.cpp 1459fcc 
>   src/kcms/kio/useragentselectordlg.cpp 3952761 
>   src/kioexec/main.cpp 6f4540f 
>   src/kntlm/kntlm.cpp ed6f388 
>   src/kpac/script.cpp 9b126ca 
>   src/kpasswdserver/kpasswdserver.cpp 7a53300 
>   src/kssld/kssld.cpp fcc3bed 
>   src/protocoltojson/main.cpp 05b9364 
>   src/urifilters/ikws/kuriikwsfiltereng.h c8fbcd2 
>   src/urifilters/ikws/searchprovider.cpp 8652997 
>   src/urifilters/ikws/searchproviderdlg.cpp 4effe08 
>   src/urifilters/localdomain/localdomainurifilter.cpp 4ebd1c0 
>   src/urifilters/shorturi/kshorturifilter.cpp c9024b0 
>   src/widgets/accessmanager.cpp 68f4a27 
>   src/widgets/accessmanagerreply_p.cpp 2ade6fd 
>   src/widgets/delegateanimationhandler.cpp 8e08a0c 
>   src/widgets/fileundomanager.cpp c9790ab 
>   

Re: Proposal for moving KColorScheme from KConfigWidgets to KGuiAddons

[Martin Graesslin]

On Saturday, February 20, 2016 11:22:48 AM CET Andreas Cord-Landwehr wrote:
> Hi, I want to propose to move the KColorScheme class from the KConfigWidgets
> famework to the KGuiAddons framework.
> 
> Reasons:
> * that would reduce dependencies of KIconThemes and make it
>   - a Tier 2 framework (currently Tier 3) and
>   - make it easier to use KIconThemes on Android
> * KGuiAddons seems to be the much more logical place for this class in my
> opinion
> 
> My Questions:
> a) Are there other opinions about this move?
> b) What about KColorSchemeManager? I would propose to keep it at
> KConfigWidgets, though.

KColorSchemeManager is only in KConfigWidgets because KColorScheme is in 
KConfigWidgets. Thus I would suggest to move it as well if possible.

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