D7316: Avoid sending data offers from an invalid source.

[David Edmundson]

davidedmundson updated this revision to Diff 18213.
davidedmundson added a comment.
Restricted Application edited projects, added Plasma; removed Plasma on Wayland.


  Split into two lines

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7316?vs=18162=18213

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7316

AFFECTED FILES
  autotests/client/test_wayland_seat.cpp
  src/server/datadevice_interface.cpp

To: davidedmundson, #plasma
Cc: graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas

D7316: Avoid sending data offers from an invalid source.

[David Edmundson]

davidedmundson marked an inline comment as done.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D7316

To: davidedmundson, #plasma
Cc: graesslin, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas

D7293: Deprecate KStandardAction::SaveOptions

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Indeed I have never seen "Save Settings" anywhere, and git log says it was 
added in 2000, with a bunch of other standard actions, looks like "just in 
case" to me.

REPOSITORY
  R265 KConfigWidgets

BRANCH
  deprecate-saveoptions

REVISION DETAIL
  https://phabricator.kde.org/D7293

To: elvisangelaccio, #frameworks, dfaure
Cc: dfaure

D7328: Don't enter test subdirectories if BUILD_TESTING=OFF

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7328

To: asturmlechner, #frameworks, dfaure

Re: Running applications and unittests without "make install"

[David Faure]

On mardi 15 août 2017 18:36:39 CEST Elvis Angelaccio wrote:
> On martedì 15 agosto 2017 17:17:16 CEST, David Faure wrote:
> > Hi everyone,
> > 
> > The documentation for how to do that is now up at
> > 
> > https://blogs.kde.org/2017/08/15/running-applications-and-unittests-withou
> > t-make-install
> > 
> > I have ported/fixed all frameworks, except for kirigami, and
> > there are still pending
> > merge requests for kpackage.
> > 
> > I'm wondering if we could change the CI scripts so that they
> > run the tests before doing make install?
> > For frameworks only, for now.
> 
> Thanks for working on this. Is there a plan to put this documentation on
> some wiki page as well?

Right, I can definitely do some copy/pasting, that's in my area of expertise 
:-)

https://community.kde.org/Guidelines_and_HOWTOs/Making_apps_run_uninstalled

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

D6829: Add ability to use the new kauth helper in file ioslave

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> global.h:265
>  /**
> + * Specifies privilege file operation status.
> + */

@since 5.38

> global.h:267
> + */
> +enum  PrivilegeFileOpStatus {
> +OperationNotAllowed,

I suggest naming it PrivilegedOperationStatus

(Better avoid abbreviations, and "File" is kind of obvious in KIO.)

REVISION DETAIL
  https://phabricator.kde.org/D6829

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks

D6197: Add kauth helper to file ioslave

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> filehelper.cpp:46
> +// Set it to 0.
> +errno = 0;
> +

Well, this proves exactly my earlier point: we should only test errno *when* a 
libc function fails.

If this code is still checking errno after success somewhere, then *that* it 
what should be fixed. And once it's fixed, there's no need to reset errno here.

(BTW strerror(11) is EAGAIN, "Resource temporarily unavailable", rather 
frequent for non-blocking sockets, which is probably what triggered the slave 
to wake up in the first place.)

So, where is this code checking for errno even after success? In the caller of 
this method? It's hard to review all these separate review requests because I 
never have a global overview or the ability to search across the whole codebase 
-- but at the same time, everything in a single merge request would kill this 
slow webbrowser... [QtWebEngine compiled in debug mode] :-)

REVISION DETAIL
  https://phabricator.kde.org/D6197

To: chinmoyr, elvisangelaccio, #frameworks, dfaure

D6829: Add ability to use the new kauth helper in file ioslave

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> global.h:270
> +OperationAllowed,
> +OperationCancelled
> +};

Canceled and cancelled as both correct English (maybe one is UK and one is US, 
I don't know).

In KIO we used Canceled everywhere, though. I would prefer if this stayed 
consistent with the rest of KIO..

> slavebase.h:944
> + */
> +int isPrivilegeOperationAllowed();
> +

If it returns an int, it can't really be named "isSomething" anymore, which is 
for boolean methods.

"Is this allowed?" "2" doesn't really work.

Can't this return an enum type rather than an int?
Also, the method should be const...

> file_p.h:24
>  
> +#define EUSERCANCELLED 255
> +

#define is for 1990, these days we have a proper C++ language where 
preprocessor hacks are less and less needed.

An enum value is probably the cleanest way here (to avoid the whole issue of 
"in which .cpp file to implement it", if it was an actual int variable).

The naming EFOO is very libc-like, I wouldn't use this here.
In fact, why not use KIO::ERR_USER_CANCELED?  (note that it has a value of 1, 
don't use 1 for something else ;)

> file_unix.cpp:666
> +if (fileOpStatus == KIO::OperationCancelled) {
> +errno = EUSERCANCELED;
> +}

Who's going to read that value? The caller? Abusing errno to ship a return 
value via global state reads horrible to me we're not a libc function

REVISION DETAIL
  https://phabricator.kde.org/D6829

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks

D7337: Port rest of scripting API to QJSValue-based solution

[Allan Sandfeld Jensen]

carewolf accepted this revision.
carewolf added a comment.
This revision is now accepted and ready to land.


  Looks good to me

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D7337

To: dhaumann, carewolf, cullmann
Cc: kwrite-devel, #frameworks

D7337: Port rest of scripting API to QJSValue-based solution

[Dominik Haumann]

dhaumann created this revision.
Restricted Application added subscribers: Frameworks, kwrite-devel.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  This completes the scripting API for QJSValue-based Cursor and Range 
functions. The changes are rather simple, sometimes code is just moved around 
to make the header file more readable.

TEST PLAN
  make test succeeds.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D7337

AFFECTED FILES
  src/script/katescriptdocument.cpp
  src/script/katescriptdocument.h
  src/script/katescriptview.cpp
  src/script/katescriptview.h

To: dhaumann, carewolf, cullmann
Cc: kwrite-devel, #frameworks

D6830: Make use of kauth helper in copy method of file ioslave

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 18208.
chinmoyr added a comment.


  - Added method to check if filesystem supports chmod, chown and utime.
  - Added check for EUSERCANCELLED error.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6830?vs=18015=18208

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6830

AFFECTED FILES
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp

To: chinmoyr, dfaure, #frameworks
Cc: elvisangelaccio, #frameworks

D6829: Add ability to use the new kauth helper in file ioslave

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 18206.
chinmoyr added a comment.


  - Fix spelling

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6829?vs=18205=18206

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6829

AFFECTED FILES
  src/core/global.h
  src/core/slavebase.cpp
  src/core/slavebase.h
  src/core/slaveinterface.cpp
  src/core/slaveinterface.h
  src/ioslaves/file/file.h
  src/ioslaves/file/file_p.h
  src/ioslaves/file/file_unix.cpp
  src/ioslaves/file/file_win.cpp

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks

D6829: Add ability to use the new kauth helper in file ioslave

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 18205.
chinmoyr added a comment.


  - Fix spelling

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6829?vs=18204=18205

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6829

AFFECTED FILES
  src/core/global.h
  src/core/slavebase.cpp
  src/core/slavebase.h
  src/core/slaveinterface.cpp
  src/core/slaveinterface.h
  src/ioslaves/file/file.h
  src/ioslaves/file/file_p.h
  src/ioslaves/file/file_unix.cpp
  src/ioslaves/file/file_win.cpp

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks

D6829: Add ability to use the new kauth helper in file ioslave

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 18204.
chinmoyr added a comment.


  - Added enum to denote file operation state
  - Added new error code

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6829?vs=18014=18204

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6829

AFFECTED FILES
  src/core/global.h
  src/core/slavebase.cpp
  src/core/slavebase.h
  src/core/slaveinterface.cpp
  src/core/slaveinterface.h
  src/ioslaves/file/file.h
  src/ioslaves/file/file_p.h
  src/ioslaves/file/file_unix.cpp
  src/ioslaves/file/file_win.cpp

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks

Re: github repos

[Albert Astals Cid]

El dimarts, 15 d’agost de 2017, a les 17:14:27 CEST, David Faure va escriure:
> Hi Albert,
> 
> I just added myself as watcher to the frameworks I maintain:
> 
> karchive
> kcrash
> kdbusaddons
> kded
> kinit
> kio
> kparts
> kservice
> kxmlgui
> 
> Can you remove the auto-reply on merge requests for these repositories?

Done.

> 
> Thanks.

D7316: Avoid sending data offers from an invalid source.

[Martin Flöser]

graesslin added inline comments.

INLINE COMMENTS

> datadevice_interface.cpp:206
> +if (!otherSelection) {
> +return sendClearSelection();
> +}

please use a dedicated line for the return here.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D7316

To: davidedmundson, #plasma
Cc: graesslin, plasma-devel, #frameworks, leezu, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, 
hein, lukas

D6197: Add kauth helper to file ioslave

[Chinmoy Ranjan Pradhan]

chinmoyr added inline comments.

INLINE COMMENTS

> dfaure wrote in filehelper.cpp:44
> should be unnecessary now, with the above suggested change

In my system errno is set to 11. It seems like the case where function call 
succeeds but errno is set. But then it happens right after control reaches this 
method so I can't figure out which call might have set the error. I can't say 
if its unique to my system or not but there it is. So I think its best to set 
errno to 0  just to be on safe side.

REVISION DETAIL
  https://phabricator.kde.org/D6197

To: chinmoyr, elvisangelaccio, #frameworks, dfaure

D6197: Add kauth helper to file ioslave

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 18201.
chinmoyr added a comment.


  - fixed minor issues

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6197?vs=18012=18201

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D6197

AFFECTED FILES
  src/ioslaves/file/CMakeLists.txt
  src/ioslaves/file/file_p.h
  src/ioslaves/file/kauth/CMakeLists.txt
  src/ioslaves/file/kauth/file.actions
  src/ioslaves/file/kauth/filehelper.cpp
  src/ioslaves/file/kauth/filehelper.h

To: chinmoyr, elvisangelaccio, #frameworks, dfaure

Re: Running applications and unittests without "make install"

[Elvis Angelaccio]

On martedì 15 agosto 2017 17:17:16 CEST, David Faure wrote:

Hi everyone,

The documentation for how to do that is now up at
 
https://blogs.kde.org/2017/08/15/running-applications-and-unittests-without-make-install


I have ported/fixed all frameworks, except for kirigami, and 
there are still pending

merge requests for kpackage.

I'm wondering if we could change the CI scripts so that they 
run the tests before doing make install?

For frameworks only, for now.



Thanks for working on this. Is there a plan to put this documentation on 
some wiki page as well?


Cheers,
Elvis

D7230: Save up a bunch of stat() calls on application start

[Andreas Sturmlechner]

asturmlechner accepted this revision.
asturmlechner added a comment.


  Briefly runtime tested it as well with and without translations, no issues.

REPOSITORY
  R263 KXmlGui

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7230

To: apol, #frameworks, dfaure, asturmlechner
Cc: ltoscano, aacid, dfaure, elvisangelaccio, broulik

D7297: Add Pony highlighting

[jonathan poelen]

jpoelen updated this revision to Diff 18196.
jpoelen added a comment.


  Pony: Support for attributes and best behavior for function definitions. Add 
license and removed autotests/foldingtest.pony

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7297?vs=18194=18196

BRANCH
  pony

REVISION DETAIL
  https://phabricator.kde.org/D7297

AFFECTED FILES
  autotests/folding/highlight.pony.fold
  autotests/html/highlight.pony.html
  autotests/input/highlight.pony
  autotests/reference/highlight.pony.ref
  data/syntax/pony.xml

To: jpoelen, #framework_syntax_hightlighting
Cc: dhaumann, #frameworks, cullmann, vkrause

D7297: Add Pony highlighting

[jonathan poelen]

jpoelen updated this revision to Diff 18194.
jpoelen added a comment.
Restricted Application added a project: Frameworks.


  - Pony: Support for attributes and best behavior for function definitions. 
Add license and removed autotests/foldingtest.pony

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7297?vs=18110=18194

BRANCH
  pony

REVISION DETAIL
  https://phabricator.kde.org/D7297

AFFECTED FILES
  CMakeLists.txt
  KF5SyntaxHighlightingConfig.cmake.in
  autotests/folding/Doxyfile.example.fold
  autotests/folding/craftenv.ps1.fold
  autotests/folding/highlight.cmake.fold
  autotests/folding/highlight.cpp.fold
  autotests/folding/highlight.hs.fold
  autotests/folding/highlight.js.fold
  autotests/folding/highlight.php.fold
  autotests/folding/highlight.pl.fold
  autotests/folding/highlight.pony.fold
  autotests/folding/highlight.yang.fold
  autotests/folding/light52_tb.vhdl.fold
  autotests/folding/test.bash.fold
  autotests/folding/test.htm.fold
  autotests/folding/test.py.fold
  autotests/folding/test.zsh.fold
  autotests/folding/test_syntax.sql.fold
  autotests/foldingtest.cpp
  autotests/html/Doxyfile.example.html
  autotests/html/craftenv.ps1.html
  autotests/html/highlight.cmake.html
  autotests/html/highlight.cpp.html
  autotests/html/highlight.dox.html
  autotests/html/highlight.hs.html
  autotests/html/highlight.js.html
  autotests/html/highlight.lex.html
  autotests/html/highlight.php.html
  autotests/html/highlight.pl.html
  autotests/html/highlight.pony.html
  autotests/html/highlight.yang.html
  autotests/html/light52_tb.vhdl.html
  autotests/html/test.bash.html
  autotests/html/test.c.html
  autotests/html/test.css.html
  autotests/html/test.htm.html
  autotests/html/test.py.html
  autotests/html/test.zsh.html
  autotests/html/test_syntax.sql.html
  autotests/htmlhighlighter_test.cpp
  autotests/input/Doxyfile.example
  autotests/input/craftenv.ps1
  autotests/input/highlight.cmake
  autotests/input/highlight.cpp
  autotests/input/highlight.hs
  autotests/input/highlight.js
  autotests/input/highlight.php
  autotests/input/highlight.pl
  autotests/input/highlight.pony
  autotests/input/highlight.yang
  autotests/input/light52_tb.vhdl
  autotests/input/test.bash
  autotests/input/test.htm
  autotests/input/test.py
  autotests/input/test.zsh
  autotests/reference/Doxyfile.example.ref
  autotests/reference/craftenv.ps1.ref
  autotests/reference/highlight.cmake.ref
  autotests/reference/highlight.cpp.ref
  autotests/reference/highlight.dox.ref
  autotests/reference/highlight.hs.ref
  autotests/reference/highlight.js.ref
  autotests/reference/highlight.lex.ref
  autotests/reference/highlight.php.ref
  autotests/reference/highlight.pl.ref
  autotests/reference/highlight.pony.ref
  autotests/reference/highlight.sh.ref
  autotests/reference/highlight.yang.ref
  autotests/reference/light52_tb.vhdl.ref
  autotests/reference/test.bash.ref
  autotests/reference/test.c.ref
  autotests/reference/test.css.ref
  autotests/reference/test.htm.ref
  autotests/reference/test.py.ref
  autotests/reference/test.zsh.ref
  autotests/reference/test_syntax.sql.ref
  autotests/syntaxrepository_test.cpp
  autotests/testhighlighter.cpp
  autotests/theme_test.cpp
  data/CMakeLists.txt
  data/syntax-data.qrc.in
  data/syntax/4dos.xml
  data/syntax/abap.xml
  data/syntax/actionscript.xml
  data/syntax/ada.xml
  data/syntax/agda.xml
  data/syntax/ahdl.xml
  data/syntax/ahk.xml
  data/syntax/alert.xml
  data/syntax/alert_indent.xml
  data/syntax/ample.xml
  data/syntax/ansforth94.xml
  data/syntax/ansic89.xml
  data/syntax/asm-avr.xml
  data/syntax/asn1.xml
  data/syntax/awk.xml
  data/syntax/bash.xml
  data/syntax/bitbake.xml
  data/syntax/bmethod.xml
  data/syntax/boo.xml
  data/syntax/c.xml
  data/syntax/carto-css.xml
  data/syntax/ccss.xml
  data/syntax/cgis.xml
  data/syntax/changelog.xml
  data/syntax/chicken.xml
  data/syntax/cisco.xml
  data/syntax/clipper.xml
  data/syntax/clojure.xml
  data/syntax/cmake.xml
  data/syntax/coldfusion.xml
  data/syntax/commonlisp.xml
  data/syntax/component-pascal.xml
  data/syntax/cpp.xml
  data/syntax/crk.xml
  data/syntax/cs.xml
  data/syntax/css.xml
  data/syntax/cubescript.xml
  data/syntax/cue.xml
  data/syntax/curry.xml
  data/syntax/d.xml
  data/syntax/dockerfile.xml
  data/syntax/dosbat.xml
  data/syntax/dot.xml
  data/syntax/doxyfile.xml
  data/syntax/doxygen.xml
  data/syntax/doxygenlua.xml
  data/syntax/e.xml
  data/syntax/eiffel.xml
  data/syntax/erlang.xml
  data/syntax/euphoria.xml
  data/syntax/ferite.xml
  data/syntax/fgl-4gl.xml
  data/syntax/fgl-per.xml
  data/syntax/fortran.xml
  data/syntax/freebasic.xml
  data/syntax/fsharp.xml
  data/syntax/fstab.xml
  data/syntax/gap.xml
  data/syntax/gcc.xml
  data/syntax/gdl.xml
  data/syntax/gettext.xml
  data/syntax/gnuplot.xml
  data/syntax/grammar.xml
  data/syntax/groovy.xml
  data/syntax/haml.xml
  data/syntax/haskell.xml
  data/syntax/haxe.xml
  data/syntax/html.xml
  

Running applications and unittests without "make install"

[David Faure]

Hi everyone,

The documentation for how to do that is now up at
 
https://blogs.kde.org/2017/08/15/running-applications-and-unittests-without-make-install

I have ported/fixed all frameworks, except for kirigami, and there are still 
pending
merge requests for kpackage.

I'm wondering if we could change the CI scripts so that they run the tests 
before doing make install?
For frameworks only, for now.

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

D7330: Don't enter test subdirectories if BUILD_TESTING=OFF

[Andreas Sturmlechner]

asturmlechner created this revision.
Restricted Application added a project: Frameworks.

REPOSITORY
  R293 Baloo

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7330

AFFECTED FILES
  CMakeLists.txt
  src/file/extractor/CMakeLists.txt

To: asturmlechner, #frameworks

github repos

[David Faure]

Hi Albert,

I just added myself as watcher to the frameworks I maintain:

karchive
kcrash
kdbusaddons
kded
kinit
kio
kparts
kservice
kxmlgui

Can you remove the auto-reply on merge requests for these repositories?

Thanks.

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

D6234: KGlobalAccel: port to KKeyServer's new method symXModXToKeyQt, to fix numpad keys

[David Faure]

This revision was automatically updated to reflect the committed changes.
Closed by commit R268:2c20ddff034e: KGlobalAccel: port to KKeyServer's new 
method symXModXToKeyQt, to fix numpad… (authored by dfaure).

REPOSITORY
  R268 KGlobalAccel

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6234?vs=15488=18191

REVISION DETAIL
  https://phabricator.kde.org/D6234

AFFECTED FILES
  src/runtime/plugins/xcb/kglobalaccel_x11.cpp

To: dfaure, graesslin
Cc: #frameworks

D7239: Drop unused dependency

[Heiko Becker]

heikobecker added a reviewer: Frameworks.

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D7239

To: heikobecker, #plasma, #frameworks
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart, lukas

D7328: Don't enter test subdirectories if BUILD_TESTING=OFF

[Andreas Sturmlechner]

asturmlechner created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  The proposed change to ECMAddTest was reverted because it led to failures
  with subsequent calls depending on then nonexisting test targets. Instead,
  just make the test subdirectories conditional on BUILD_TESTING.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7328

AFFECTED FILES
  CMakeLists.txt
  src/ioslaves/trash/CMakeLists.txt
  src/ioslaves/trash/tests/CMakeLists.txt
  src/kpasswdserver/CMakeLists.txt

To: asturmlechner, #frameworks

D7230: Save up a bunch of stat() calls on application start

[Andreas Sturmlechner]

asturmlechner added a comment.


  Sounds fine.

REPOSITORY
  R263 KXmlGui

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7230

To: apol, #frameworks, dfaure, asturmlechner
Cc: ltoscano, aacid, dfaure, elvisangelaccio, broulik

D7304: Do not leak symbols of pimpl classes, protect with Q_DECL_HIDDEN

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R127 KWayland

BRANCH
  davidedmundson/xdgv6

REVISION DETAIL
  https://phabricator.kde.org/D7304

To: davidedmundson, #plasma, dfaure
Cc: plasma-devel, #frameworks, leezu, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas

D7249: Return high-resolution line edit clear icon

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> davidedmundson wrote in kstyle.cpp:419
> just QIcon::fromTheme(directionalThemeName, "edit-clear")

You mean `QIcon::fromTheme(directionalThemeName, 
QIcon::fromTheme("edit-clear"));`?

REPOSITORY
  R252 Framework Integration

REVISION DETAIL
  https://phabricator.kde.org/D7249

To: broulik, kde-frameworks-devel, hpereiradacosta, davidedmundson
Cc: davidedmundson, hpereiradacosta, #frameworks

D7326: [File KIO slave] Fix applying special file attributes

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:9ac7832b859b: [File KIO slave] Fix applying special file 
attributes (authored by broulik).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7326?vs=18184=18187

REVISION DETAIL
  https://phabricator.kde.org/D7326

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/file.cpp

To: broulik, kde-frameworks-devel, dfaure
Cc: #frameworks

D7326: [File KIO slave] Fix applying special file attributes

[David Faure]

dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7326

To: broulik, kde-frameworks-devel, dfaure
Cc: #frameworks

D7230: Save up a bunch of stat() calls on application start

[David Faure]

dfaure added a comment.


  A minor "breakage" though, just a menu item that brings a selection of 0 or 1 
language, i.e. showing the user that no translation is available. Arguably 
that's actually better user interface than a missing menu item (A: "click on 
that menu item!" -- B: "where? you're dreaming, there's no such menu item!")

REPOSITORY
  R263 KXmlGui

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7230

To: apol, #frameworks, dfaure, asturmlechner
Cc: ltoscano, aacid, dfaure, elvisangelaccio, broulik

D7230: Save up a bunch of stat() calls on application start

[Andreas Sturmlechner]

asturmlechner added a comment.


  Thanks for adding me, at least Gentoo indeed supports stripping translations 
even after they were moved into the packages - so that would break.

REPOSITORY
  R263 KXmlGui

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7230

To: apol, #frameworks, dfaure, asturmlechner
Cc: ltoscano, aacid, dfaure, elvisangelaccio, broulik

D7326: [File KIO slave] Fix applying special file attributes

[Kai Uwe Broulik]

broulik created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  QFile does not support special attributes like sticky. This would cause us to 
always discard them.
  
  BUG: 365795

TEST PLAN
  Comes with a unit test that passes only with this patch. Did not try windows 
compilation.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7326

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/ioslaves/file/file.cpp

To: broulik, kde-frameworks-devel, dfaure
Cc: #frameworks

D7318: KFileItemDelegate: Always reserve space for icons

[Fabian Vogt]

fvogt added a comment.


  The underlying issue is that KIconLoader's implementation of isNull (in 
virtual_hook) is not really meaningful.
  In cases where a name for a nonexistent icon is passed, pixmap will draw the 
"unknown" icon, but isNull still returns true.
  
  So the question is whether this is intentional or not. The API doesn't really 
make that clear.
  
  If isNull should return false iff pixmap returns something other than an 
empty pixmap, the implementation for KIconLoader
  would probably always return false...
  
  In any case, I still think this diff isn't wrong by itself as the drawing of 
the pixmap is still unconditional and so should be reflected
  in the text margin.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: cfeck, #frameworks

D7249: Return high-resolution line edit clear icon

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> kstyle.cpp:419
> +
> +QIcon icon = QIcon::fromTheme(directionalThemeName);
> +

just QIcon::fromTheme(directionalThemeName, "edit-clear")

REPOSITORY
  R252 Framework Integration

REVISION DETAIL
  https://phabricator.kde.org/D7249

To: broulik, kde-frameworks-devel, hpereiradacosta, davidedmundson
Cc: davidedmundson, hpereiradacosta, #frameworks

D7318: KFileItemDelegate: Always reserve space for icons

[Christoph Feck]

cfeck added a comment.


  BUG: 372207

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: cfeck, #frameworks

D7318: KFileItemDelegate: Always reserve space for icons

[Fabian Vogt]

fvogt added a comment.


  It would be possible to avoid drawing null QIcons as well, but that would 
result in broken alignment.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: #frameworks

D7318: KFileItemDelegate: Always reserve space for icons

[Fabian Vogt]

fvogt created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Even null icons are drawn so also reserve space for them to avoid overlapping.

TEST PLAN
  http://imgur.com/a/fG2T0

REPOSITORY
  R241 KIO

BRANCH
  iconfix-master

REVISION DETAIL
  https://phabricator.kde.org/D7318

AFFECTED FILES
  src/widgets/kfileitemdelegate.cpp

To: fvogt, #frameworks
Cc: #frameworks

D7249: Return high-resolution line edit clear icon

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  +1 from me. Sounds sensible.

REPOSITORY
  R252 Framework Integration

REVISION DETAIL
  https://phabricator.kde.org/D7249

To: broulik, kde-frameworks-devel, hpereiradacosta
Cc: hpereiradacosta, #frameworks

Re: Tests leaving behind large temporary files

[Ben Cooksley]

On Mon, Aug 14, 2017 at 11:40 PM, Bhushan Shah  wrote:
> Hello Ben,

Hi Bhushan,

>
> On Mon, Aug 14, 2017 at 10:59:49PM +1200, Ben Cooksley wrote:
>> Could someone please investigate, determine which test(s) are
>> responsible and fix those tests to cleanup after themselves?
>>
>
> I did some investigation on those left-out files, and it seems it's not
> the code of frameworks but code of CI itself leaving those files,
>
> I found a small file "/tmp/tmp8ubko4sx" on FreeBSD builder, looking at
> trings of the file,
>
> # strings /tmp/tmp8ubko4sx
> checksum: 39909cda53863cdc13d719a7b800f5abf6a2e73eb89270c26f3cf94ce4dbc570
> scmRevision: null
> timestamp: 1502708857.009341
>
> And then I looked at the ci builder code and turns out those parts come
> from the capture-workspace.py code of ci-tooling.
>
> We need to make that code failproof it seems instead of the frameworks
> code.

Oops. That would explain why it shows up on both platforms for sure...
Thanks for finding that.

I've now fixed that code to cleanup after itself.

>
> Thanks!

Cheers,
Ben

>
> --
> Bhushan Shah
> http://blog.bshah.in
> IRC Nick : bshah on Freenode
> GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D

D7281: KCoreAddons: Enforce hidden symbol visibility in nested private classes

[Ben Cooksley]

bcooksley added a comment.


  I'm not sure why that happened - it automatically associated correctly so it 
should have closed.
  
  It does appear that one of the reviews 
(https://phabricator.kde.org/tag/frameworks/) hasn't approved it yet so that 
may be the reason why.
  (Phabricator upstream are big on stuff not going in without proper review)

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D7281

To: mpyne, #frameworks, kossebau
Cc: bcooksley, kossebau