rjvbb created this revision.
rjvbb added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
rjvbb requested review of this revision.
REVISION SUMMARY
Sonnet has a hidden default client concept (hidden because the standard
configuration dialog doesn't expose the setting)
rjvbb updated this revision to Diff 29244.
rjvbb added a comment.
Add missing context (patch unchanged)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D11183?vs=29094&id=29244
REVISION DETAIL
https://phabricator.kde.org/D11183
AFFECTED FILES
src/core/loader.cpp
To: rjvbb, #fram
rjvbb added a reviewer: cullmann.
rjvbb set the repository for this revision to R246 Sonnet.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann
Cc: #frameworks, michaelh
rjvbb added reviewers: dfaure, mlaurent, vkrause.
rjvbb edited subscribers, added: kde-frameworks-devel; removed: Frameworks.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause
Cc: kde-frameworks-devel, michae
rjvbb added a comment.
Any objections if I commit this by the end of the week?
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause
Cc: kde-frameworks-devel, michaelh, ngraham
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
I like the idea.
INLINE COMMENTS
> loader.cpp:101
> +// if it does it will be contained in lClients.
> +QVectorIterator itr1(lClients);
> +bool unknown = t
rjvbb updated this revision to Diff 30617.
rjvbb added a comment.
Modified as requested.
(Please double-check, it works AFAICT but it's the first time I use this
construct.)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D11183?vs=29244&id=30617
REVISION DETAIL
https://phabricat
rjvbb set the repository for this revision to R246 Sonnet.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause
Cc: kde-frameworks-devel, michaelh, ngraham
rjvbb marked an inline comment as done.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause
Cc: kde-frameworks-devel, michaelh, ngraham
mwolff added inline comments.
INLINE COMMENTS
> loader.cpp:103
> +// if it does it will be an element of lClients.
> +bool unknown = std::find_if(lClients.constBegin(),
> lClients.constEnd(), [backend] (const Client *client) {
> +return client->name() == backend;
cullmann added inline comments.
INLINE COMMENTS
> mwolff wrote in loader.cpp:103
> nope, that's wrong. find_if returns an iterator - in this case a Client*
> probably that gets converted to bool?
Isn't there a == lClients.constEnd() behind that does that?
REPOSITORY
R246 Sonnet
REVISION DET
mwolff added inline comments.
INLINE COMMENTS
> cullmann wrote in loader.cpp:103
> Isn't there a == lClients.constEnd() behind that does that?
ah, then this should be simplified by using `std::any_of` instead
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rj
rjvbb added a comment.
> ah, then this should be simplified by using `std::any_of` instead
I was asked to use std::find_if, and (spicey detail) referred to the KDevelop
code for examples how to use it :)
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To:
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
I suggested find_if because I didn't think about any_of, which is even better
indeed :-)
If I suggest something and Milian suggests something else then Milian is
right :-)
INLI
rjvbb marked 4 inline comments as done.
rjvbb added a comment.
> If I suggest something and Milian suggests something else then Milian is
right :-)
Unconditionally? :)
INLINE COMMENTS
> dfaure wrote in loader.cpp:114
> can this ever happen now that you set it if it was empty? If it can
rjvbb updated this revision to Diff 30657.
rjvbb marked an inline comment as done.
rjvbb added a comment.
next iteration
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D11183?vs=30617&id=30657
REVISION DETAIL
https://phabricator.kde.org/D11183
AFFECTED FILES
src/core/loader.cpp
rjvbb set the repository for this revision to R246 Sonnet.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause
Cc: mwolff, kde-frameworks-devel, michaelh, ngraham
rjvbb updated this revision to Diff 31594.
rjvbb added a comment.
Unnecessary rebump, pardon, rebase.
What's holding this up?
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D11183?vs=30657&id=31594
REVISION DETAIL
https://phabricator.kde.org/D11183
AFFECTED FILES
src/core/l
rjvbb set the repository for this revision to R246 Sonnet.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause
Cc: mwolff, kde-frameworks-devel, michaelh, ngraham, bruns
dfaure accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R246 Sonnet
REVISION DETAIL
https://phabricator.kde.org/D11183
To: rjvbb, #frameworks, cullmann, dfaure, mlaurent, vkrause
Cc: mwolff, kde-frameworks-devel, michaelh, ngraham, bruns
This revision was automatically updated to reflect the committed changes.
Closed by commit R246:e63a234d55d9: Don't impose using the default client
(authored by rjvbb).
REPOSITORY
R246 Sonnet
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D11183?vs=31594&id=31608
REVISION DETAIL
ht
This introduces a subtle change in behaviour so I'd appreciate some feedback!
22 matches
Mail list logo