D25025: decodeBCJ2: Fix assert with broken files

[Aleix Pol Gonzalez]

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

REPOSITORY
  R243 KArchive

BRANCH
  master

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

To: aacid, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25023: Make kconfig_compiler generate code with const

[Aleix Pol Gonzalez]

apol added a comment.


  I don't know if we can do that. There's been some changes we've pulled back 
assuming people could be installing + exporting classes generated from 
kconfig_compiler. :/

REPOSITORY
  R237 KConfig

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

To: aacid
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23384: [WIP] Adding support for mounting KIOFuse URLs for applications that don't use KIO

[Alexander Saoutkin]

feverfew updated this revision to Diff 68931.
feverfew added a comment.


  - Switch from KDED module to DBus service

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23384?vs=64440=68931

BRANCH
  KDEDModule (branched from master)

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/core/desktopexecparser.cpp
  src/core/kiofuseinterface.cpp
  src/core/kiofuseinterface.h
  src/widgets/krun.cpp

To: feverfew, fvogt, chinmoyr
Cc: davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, michaelh, 
bruns

D25025: decodeBCJ2: Fix assert with broken files

[Albert Astals Cid]

aacid created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
aacid requested review of this revision.

REVISION SUMMARY
  Make sure we're not reading past the bytearray data

REPOSITORY
  R243 KArchive

BRANCH
  master

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

AFFECTED FILES
  src/k7zip.cpp

To: aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24884: I18N_NOOP2 was deprecated but we can't replace by I18NC_NOOP as it expends it as 2 elements (context + text)

[Albert Astals Cid]

aacid added a comment.


  You don't want to accept there's a potential problem with typos and that 
either using KLocalizedString or forcing you to store the context is the 
solution?
  
  Fine then just undeprecate Noop2, but adding strip that does the same is 
stupid

REPOSITORY
  R249 KI18n

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

To: mlaurent, dfaure, ilic
Cc: kossebau, aacid, vkrause, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D25023: Make kconfig_compiler generate code with const

[Albert Astals Cid]

aacid created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
aacid requested review of this revision.

REVISION SUMMARY
  KSharedConfig::Ptr is not very expensive to copy but const & is cheaper

REPOSITORY
  R237 KConfig

BRANCH
  master

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

AFFECTED FILES
  autotests/kconfig_compiler/test8a.cpp.ref
  autotests/kconfig_compiler/test8a.h.ref
  autotests/kconfig_compiler/test8c.cpp.ref
  autotests/kconfig_compiler/test8c.h.ref
  src/kconfig_compiler/kconfig_compiler.cpp

To: aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25015: Update breeze theme shadows

[Nathaniel Graham]

ngraham added a comment.


  If anything I wonder if we should make them bigger too, to better match the 
default Breeze shadows.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24982: Small improvements in some XML files

[Dominik Haumann]

dhaumann added a comment.


  Hm right... too bad. I was hoping to find an automated way to detect this. 
Since relying on the user to optimize the RegExps will always be suboptimal. 
@cullmann Do you have any ideas?

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D24982: Small improvements in some XML files

[Nibaldo González]

nibags added a comment.


  No, the dynamic flag is used to insert the captures already stored in `%N` 
(the captures are stored in the RegExpr rules and then "inserted" in the rules 
with the dynamic flag)

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25015: Update breeze theme shadows

[Filip Fila]

filipf added a comment.


  Testing it. Usability wise the strength is perfectly fine; the shadows still 
do their job.
  
  Visually it's a definitive improvement for me.
  
  Only thing I'm not sure of is the weaker strength in the corners, but I'm 
going to keep testing to see if it's just a matter of getting used to it.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: filipf, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25014: I have received negative feedback (and I agree with it) on the current state of breeze shadows: they are quite dark, narrow, and feel unnatural. I tried to adress that by making shadows more s

[Niccolò Venerandi]

niccolove abandoned this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25015: Update breeze theme shadows

[Niccolò Venerandi]

niccolove retitled this revision from "I have received negative feedback (and I 
agree with it) on the current state of breeze shadows: they are quite dark, 
narrow, and feel unnatural. I tried to adress that by making shadows more 
sparse and a bit lighter especially on the angles while..." to "Update breeze 
theme shadows".
niccolove edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24593: Modified breeze shadows

[Niccolò Venerandi]

niccolove abandoned this revision.
niccolove added a comment.


  Let's try again https://phabricator.kde.org/D25015

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: Luwx, filipf, ngraham, davidedmundson, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, bruns

D25015: I have received negative feedback (and I agree with it) on the current state of breeze shadows: they are quite dark, narrow, and feel unnatural. I tried to adress that by making shadows more s

[Niccolò Venerandi]

niccolove added a reviewer: VDG.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, #vdg
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25015: I have received negative feedback (and I agree with it) on the current state of breeze shadows: they are quite dark, narrow, and feel unnatural. I tried to adress that by making shadows more s

[Niccolò Venerandi]

niccolove created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
niccolove requested review of this revision.

REVISION SUMMARY
  ...trying to keep it distinguishable when on white background. I received 
some positive feedback on these shadows from T10891 
 and the VDG channel. More specifically, I 
changed dialogs/background.svg to have a) longer linear gradients and b) radial 
gradients instead of linear on the four sides to make the center darker than 
the angles.

TEST PLAN
  F7629404: Screenshot_20191019_164936.png 

  F7629444: Screenshot_20191019_165736.png 

  F7629446: Screenshot_20191019_165804.png 


REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  breeze-shadows (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze/dialogs/background.svg

To: niccolove
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25014: I have received negative feedback (and I agree with it) on the current state of breeze shadows: they are quite dark, narrow, and feel unnatural. I tried to adress that by making shadows more s

[Niccolò Venerandi]

niccolove created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
niccolove requested review of this revision.

REVISION SUMMARY
  ...trying to keep it distinguishable when on white background. I received 
some positive feedback on these shadows from T10891 
 and the VDG channel. More specifically, I 
changed dialogs/background.svg to have a) longer linear gradients and b) radial 
gradients instead of linear on the four sides to make the center darker than 
the angles.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  breeze-shadows (branched from master)

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

AFFECTED FILES
  src/desktoptheme/breeze/dialogs/background.svg

To: niccolove
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Can't wait to get clang-format fully automated into the review workflow

INLINE COMMENTS

> deletejob.cpp:76
> + */
> +void rmfile(const QUrl& url, bool isLink){
> +emit rmfileResult(QFile::remove(url.toLocalFile()), url, isLink);

missing space before '{', sorry I missed it until now.

> deletejob.cpp:100
>  , m_reportTimer(nullptr)
> +, m_ioworker(nullptr)
>  {

funny that you use the ctor init list for this one, and not a default value at 
declaration time, like you did for m_thread :-)

> deletejob.cpp:197
> +
> +if (m_thread == nullptr) {
> +m_thread = new QThread();

nitpick: it would be more expected to do `if (!m_ioworker) {` here, since 
m_ioworker is what we're creating on demand. m_thread is just an implementation 
detail.

> deletejob.cpp:404
> +// If local file, try do it directly
> +if (m_currentURL.isLocalFile()){
> +// separate thread will do the work

space before `{` here as well

> deletejob.cpp:432
> +
> +void DeleteJobPrivate::deleteDirUsingJob (const QUrl )
> +{

I still see a space between method name and `(` here :-)

REPOSITORY
  R241 KIO

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

To: meven, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24884: I18N_NOOP2 was deprecated but we can't replace by I18NC_NOOP as it expends it as 2 elements (context + text)

[David Faure]

dfaure added a comment.


  I agree with Friederich. In kio/filewidgets/kfileplacesmodel.cpp (for which 
you accepted the change in D24970 ) I'm 
passing the context to the method because I18NC_NOOP forces me to, but then I'm 
*discarding* this context because:
  
  1. I have nowhere to store it
  2. I don't need to store it, it's the same for every item --- just like in 
Friederich's example.
  
  Why store duplicated and redundant data?
  
  So if I had a STRIP version that would all be much simpler.
  
  One has to be careful anyway when using I18N_NOOP, to actually call i18n on 
those strings, and to never call i18n on a variable whose contents wasn't 
marked with I18N_NOOP. The API can't be foolproof against that. So I don't see 
why it's trying to be foolproof against the context being provided externally, 
at the expense of more complicated code for no purpose.

REPOSITORY
  R249 KI18n

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

To: mlaurent, dfaure, ilic
Cc: kossebau, aacid, vkrause, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D24884: I18N_NOOP2 was deprecated but we can't replace by I18NC_NOOP as it expends it as 2 elements (context + text)

[Friedrich W. H. Kossebau]

kossebau added a comment.


  As said, I do not think this would be "better" code. And it's violating a bit 
the goal of zero cost abstractions, given this adds runtime need which could be 
avoided compared to the old code.
  When using the I18N_NOOP, one has to know what you do and when you later on 
pass data pointers instead of literals to i18n calls. Thinking people who do 
that are in general challenged to ensure the proper context call is still used 
might be challenged in other places before IMHO.
  
  And no, I do not want to do hacks in my own code, that proposal will be 
ignored. Let's have KI18n provide a nice API for developers, and not get in 
their way in some halfhearted way to be foolproof.

REPOSITORY
  R249 KI18n

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

To: mlaurent, dfaure, ilic
Cc: kossebau, aacid, vkrause, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D24884: I18N_NOOP2 was deprecated but we can't replace by I18NC_NOOP as it expends it as 2 elements (context + text)

[Albert Astals Cid]

aacid added a comment.


  That's the problem with deprecation warnings, they force you to write better 
code.
  
  You don't like the fact that you have to do it, but at least please 
acknowledge that having "data type" in one place and then "data type" again in 
a totally different place just works "by change"/"by magic".
  
  Also if you don't like how I18NC_NOOP(context, text) behaves you can always 
define your own I18NC_NOOP(context, text) before this include and make it strip 
the context. it's bad, but ...

REPOSITORY
  R249 KI18n

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

To: mlaurent, dfaure, ilic
Cc: kossebau, aacid, vkrause, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D24892: Fix usage of the new deprecation macros for assignIconsToContextMenu

[Volker Krause]

This revision was automatically updated to reflect the committed changes.
Closed by commit R302:3ff850df6b48: Fix usage of the new deprecation macros for 
assignIconsToContextMenu (authored by vkrause).

REPOSITORY
  R302 KIconThemes

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24892?vs=68617=68897

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

AFFECTED FILES
  src/kicontheme.cpp
  src/kicontheme.h

To: vkrause, kossebau
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24989: support multiple rcc files with icons themes

[Christoph Cullmann]

cullmann added a reviewer: dfaure.
cullmann added a comment.


  Will alter the initializer before pushing, if it is clarified if the approach 
is ok.
  Given the lack of mmap for resource files on Windows, I actually would prefer 
to hide the resources in dummy plugins, but I guess it would make sense to do 
this in a second step.

REPOSITORY
  R302 KIconThemes

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

To: cullmann, #frameworks, vonreth, dfaure
Cc: dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25003: [KDesktopFile] Add a property and propertyKde functions to access custom properties

[Aleix Pol Gonzalez]

apol added a comment.


  Hi Meven, I guess I didn't make myself clear, sorry about that. This is not 
what I meant.
  
  What I meant was here D24956  you can 
query the desktop entry directly, we don't need to expose access to it.

REPOSITORY
  R237 KConfig

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

To: meven, #frameworks, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24736: Grab the keyboard when KeySequenceItem is recording

[Aleix Pol Gonzalez]

apol accepted this revision.

REPOSITORY
  R296 KDeclarative

BRANCH
  grabkeyboard (branched from master)

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

To: davidre, #frameworks, davidedmundson, apol
Cc: apol, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D24736: Grab the keyboard when KeySequenceItem is recording

[Aleix Pol Gonzalez]

apol added a comment.


  +1 Makes sense to me.

REPOSITORY
  R296 KDeclarative

BRANCH
  grabkeyboard (branched from master)

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

To: davidre, #frameworks, davidedmundson
Cc: apol, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D24736: Grab the keyboard when KeySequenceItem is recording

[David Edmundson]

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

REPOSITORY
  R296 KDeclarative

BRANCH
  grabkeyboard (branched from master)

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

To: davidre, #frameworks, davidedmundson
Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24993: Add integrated inline spelling menu to KTextEdit

[Tommy Lincoln]

pajamapants3000 added inline comments.

INLINE COMMENTS

> spellingmenu.h:44
> + * are handled internally, without any extra effort required from the parent.
> + * Slots are available to facilitate these actions on-demand if needed.
> + *

Reviewing, I noticed that these slots are actually protected. I will need to 
either update the documentation or the visibility of these slots. My 
inclination is toward the latter, but I will wait for any feedback before 
making changes.

REPOSITORY
  R310 KTextWidgets

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

To: pajamapants3000, #vdg, #frameworks, cullmann, cfeck
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24983: KateModeMenuList: improve word wrap

[Dominik Haumann]

dhaumann added a comment.


  Good, fine with me.
  
  Related: Maybe the translation should also be shorter, but the general 
possible problem persists. Please go ahead.

REPOSITORY
  R39 KTextEditor

BRANCH
  improve-word-wrap

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

To: nibags, #ktexteditor, cullmann, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D24982: Small improvements in some XML files

[Dominik Haumann]

dhaumann added a comment.


  > [...]
  >  One option would be to add a **capture** or **dontCapture** attribute to 
enable or disable captures for RegExpr rules. Also, captures could be enabled 
or disabled in all RegExpr rules using the  group, adding an 
element for that.
  
  But don't we have this already with the `dynamic` flag? Or am I mixing things?

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25010: [StatJob] Use A QFlag to specify the details returned by statJob

[Méven Car]

meven updated this revision to Diff 68893.
meven added a comment.


  Fix wrong comparison

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25010?vs=68892=68893

BRANCH
  stat-qflags

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

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

To: meven, #frameworks, dfaure, kossebau
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25010: [StatJob] Use A QFlag to specify the details returned by statJob

[Méven Car]

meven retitled this revision from "Use A QFlag to specify the details returned 
by statJob" to "[StatJob] Use A QFlag to specify the details returned by 
statJob".
meven added reviewers: Frameworks, dfaure, kossebau.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure, kossebau
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25010: [StatJob] Use A QFlag to specify the details returned by statJob

[Méven Car]

meven updated this revision to Diff 68892.
meven added a comment.


  Amend commit message

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25010?vs=68890=68892

BRANCH
  stat-qflags

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

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

To: meven, #frameworks, dfaure, kossebau
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25010: Use A QFlag to specify the details returned by statJob

[Méven Car]

meven created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

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

To: meven
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24884: I18N_NOOP2 was deprecated but we can't replace by I18NC_NOOP as it expends it as 2 elements (context + text)

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Looking at deprecated API usage in Okteta, I came over the use of some use of 
I18N_NOOP2 as well. The use-case there though seems pretty fine to me, porting 
to I18NC_NOOP will complicate the logic without further need, as the context is 
the same for all strings in the given array, and always would be:
  
static constexpr const char* 
longTypeNames[static_cast(PrimitiveDataType::END) + 1] = {
I18N_NOOP2("data type", "bool (1 byte)"),
I18N_NOOP2("data type", "signed byte"),
I18N_NOOP2("data type", "unsigned byte"),
I18N_NOOP2("data type", "char"),
I18N_NOOP2("data type", "bool (2 bytes)"),
I18N_NOOP2("data type", "signed short"),
I18N_NOOP2("data type", "unsigned short"),
I18N_NOOP2("data type", "bool (4 bytes)"),
I18N_NOOP2("data type", "signed int"),
I18N_NOOP2("data type", "unsigned int"),
I18N_NOOP2("data type", "bool (8 bytes)"),
I18N_NOOP2("data type", "signed long"),
I18N_NOOP2("data type", "unsigned long"),
I18N_NOOP2("data type", "float"),
I18N_NOOP2("data type", "double"),
I18N_NOOP2("data type", "bitfield"),
};
  
  used by some
  
return i18nc("data type", longTypeNames[static_cast(type)]);
  
  Porting to I18NC_NOOP would mean having to make the table an array of struct 
{context, text}, and extracting both properties from the table. More runtime 
cost and complicated logic to read. Which I find cumbersome. I would prefer to 
have a I18NC_NOOP_STRIP available.
  Let's have a warning in the API dox, to tell people they need to use this 
carefully instead. But making them having to write more complicated logic like 
here it not developer friendly.

REPOSITORY
  R249 KI18n

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

To: mlaurent, dfaure, ilic
Cc: kossebau, aacid, vkrause, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D24736: Grab the keyboard when KeySequenceItem is recording

[David Redondo]

davidre added a comment.


  Ping

REPOSITORY
  R296 KDeclarative

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

To: davidre, #frameworks
Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24999: [KIO::stat] Add a KF6 TODO to make details a Bitmask

[Friedrich W. H. Kossebau]

kossebau added a comment.


  It's actually an approach to be preferred in such cases, as it moves some of 
the work to be done at creating KF6 and porting to its new API for users of KF 
to now and the time span until KF6, keeping the unavoidable hurdle at the 
actual KF5->KF6 change as small as possible.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D24999: [KIO::stat] Add a KF6 TODO to make details a Bitmask

[Méven Car]

meven added a comment.


  In D24999#555191 , @kossebau wrote:
  
  > Instead of the TODO one could already add an overload of the method now, 
and deprecate the current one. So that in KF6 times the deprecated could be 
dumped, and there would just be the preferred variant.
  
  
  Nice idea

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D25003: [KDesktopFile] Add a property and propertyKde functions to access custom properties

[Méven Car]

meven created this revision.
meven added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  Needed to be able to use `propertyKde("NoGlobalShortcut", false)` instead of 
`noDisplay()` as in D24956 
  Similar to `KService::property` but uses a default value.

TEST PLAN
  ctest

REPOSITORY
  R237 KConfig

BRANCH
  master

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

AFFECTED FILES
  autotests/kdesktopfiletest.cpp
  autotests/kdesktopfiletest.h
  src/core/kdesktopfile.cpp
  src/core/kdesktopfile.h

To: meven, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25003: [KDesktopFile] Add a property and propertyKde functions to access custom properties

[Méven Car]

meven added a reviewer: apol.

REPOSITORY
  R237 KConfig

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

To: meven, #frameworks, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24999: [KIO::stat] Add a KF6 TODO to make details a Bitmask

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Instead of the TODO one could already add an overload of the method now, and 
deprecate the current one. So that in KF6 times the deprecated could be dumped, 
and there would just be the preferred variant.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure
Cc: kossebau, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

Re: [sysadmin/release-tools/frameworks/5.0] /: KF5: auto-increase QT_DISABLE_DEPRECATED_BEFORE when upgrading the min Qt version

[Friedrich W. H. Kossebau]

Am Sonntag, 27. Oktober 2019, 11:26:51 CET schrieb David Faure:
> On dimanche 27 octobre 2019 03:07:01 CET Friedrich W. H. Kossebau wrote:
> > Am Freitag, 25. Oktober 2019, 00:35:46 CET schrieb David Faure:
> > > On jeudi 24 octobre 2019 22:24:55 CEST Friedrich W. H. Kossebau wrote:
> > > > BTW, we want to also set
> > > > 
> > > > -DQT_DEPRECATED_WARNINGS_SINCE=0x06
> > > > 
> > > > otherwise the deprecations done with QT_DEPRECATED_VERSION in Qt's API
> > > > will
> > > > not emit warnings, because QT_DEPRECATED_WARNINGS_SINCE defaults to
> > > > QT_DISABLE_DEPRECATED_BEFORE if set.
> > > 
> > > Oh, good to know. Done now for all of KF5.
> > 
> > And the same would need to be done also for the KF variant, i.e. setting
> > 
> > -DKF_DEPRECATED_WARNINGS_SINCE=0x06
> > 
> > otherwise the use of KF_DISABLE_DEPRECATED_BEFORE_AND_AT will also result
> > in warnings for API deprecated only in newer versions.
> 
> Hmm, true.
> 
> > Quite some explicit setup. But I made ECMGenerateExportHeader follow what
> > Qt does, as consistency rules here IMHO.
> > 
> > Both QT_DEPRECATED_WARNINGS_SINCE & DKF_DEPRECATED_WARNINGS_SINCE could be
> > though set via KDEFrameworkCompilerSettings perhaps, that would match what
> > is done for -Wzero-as-null-pointer-constant already.
> 
> That would make sense, good idea. I didn't want the DISABLE stuff to come
> from ECM, so we keep some control over it (and it's a good thing I did, see
> commit dd98b2b7175b in kimageformats)
> 
> But for warnings, not much harm can happen, so that's a good idea.

Created a patch to do so: D24990

Though leaving applying this patch to you, as you seem to have the tools 
prepared for mass-handling of KF repos, to remove the
-DQT_DEPRECATED_WARNINGS_SINCE=0x06
line again from all those now that it would be duplicated :)

Cheers
Friedrich

D24999: [KIO::stat] Add a KF6 TODO to make details a Bitmask

[Aleix Pol Gonzalez]

apol added a comment.


  you may want to explain a bit more what you mean there.
  it could make sense to keep the TODO in the .cpp file, no need to get it on 
every user

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24978: Add KService::noGlobalShortcut corresponding to X-KDE-NoGlobalShortcut

[Méven Car]

meven abandoned this revision.
meven added a comment.


  In D24978#555165 , @apol wrote:
  
  > This doesn't look like some API we'd be using very often. Would it make 
sense to just have kglobalsettings access it through KService::property?
  
  
  I agree, I didn't find property when I looked at KService in the first place, 
thanks.
  
  Abandonning

REPOSITORY
  R309 KService

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

To: meven, davidre, fvogt, apol, #plasma
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24990: KDEFrameworkCompilerSettings: enable all Qt % KF deprecation warnings

[Aleix Pol Gonzalez]

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

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  enableallqtkfdeprecationwarningsforframeworks

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

To: kossebau, #frameworks, #build_system, apol
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, 
michaelh, ngraham, bruns

D24978: Add KService::noGlobalShortcut corresponding to X-KDE-NoGlobalShortcut

[Aleix Pol Gonzalez]

apol added a comment.


  This doesn't look like some API we'd be using very often. Would it make sense 
to just have kglobalsettings access it through KService::property?

INLINE COMMENTS

> kservice.cpp:739
> +{
> +if 
> (qvariant_cast(property(QStringLiteral("X-KDE-NoGlobalShortcut"), 
> QVariant::Bool))) {
> +return true;

`return qvariant_cast(property(QStringLiteral("X-KDE-NoGlobalShortcut"), 
QVariant::Bool));`

REPOSITORY
  R309 KService

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

To: meven, davidre, fvogt, apol, #plasma
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu

[Konrad Materka]

kmaterka added a comment.


  This is a proper solution to BUG 365105. Can someone review it? Is everything 
OK with this?

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: kmaterka, apol, davidedmundson, #plasma, #frameworks, broulik, nicolasfella
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24999: [KIO::stat] Add a KF6 TODO to make details a Bitmask

[Méven Car]

meven created this revision.
meven added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/statjob.h

To: meven, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24999: [KIO::stat] Add a KF6 TODO to make details a Bitmask

[Méven Car]

meven added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: meven, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[Méven Car]

meven added a comment.


  In D24962#554961 , @dfaure wrote:
  
  > Any reason why you didn't implement my suggestion of
  >
  >DeleteJobIOWorker *ioworker() {
  >if (!m_ioworker) {
  >  ...
  >}
  >return m_ioworker;
  >}
  >   [...] 
  >   QMetaObject::invokeMethod(ioworker(), "rmfile", [...]);
  >   
  >
  > ?
  >  A call to an initSomething() method can easily be forgotten, while an 
on-demand getter ensure that the worker is created when it's needed (for the 
first time).
  >  Sorry for the nitpicking :-)
  
  
  Since m_ioworker is accessible where worker() would be needed, nothing keeps 
the user to use m_ioworker instead of worker() which is in the end is 
equivalent to forget to call initSomething.
  It was my own habit to use a init or ensureInit function in such cases, and 
is the main reason I was using one.
  But it is more explicit to have an accessor and for code coherence I have 
changed the code to have a *worker() function.

INLINE COMMENTS

> dfaure wrote in deletejob.cpp:412
> marked as done but I still see removeFirst, I'm confused.

I missed this line, I did it line 430.

REPOSITORY
  R241 KIO

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

To: meven, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24962: [DeleteJob] Use a separate worker thread to run actual IO operation

[Méven Car]

meven updated this revision to Diff 68872.
meven added a comment.


  Refactor initWorkerThread() to *worker() accessor

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24962?vs=68854=68872

BRANCH
  arcpatch-D24962

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

AFFECTED FILES
  src/core/deletejob.cpp

To: meven, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24981: Modelines: fix end of comment

[Nibaldo González]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:73489a79f9fd: Modelines: fix end of comment (authored by 
nibags).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24981?vs=68820=68870

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

AFFECTED FILES
  autotests/folding/test.js.fold
  autotests/html/test.js.html
  autotests/input/test.js
  autotests/reference/test.js.ref
  data/syntax/modelines.xml

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D24983: KateModeMenuList: improve word wrap

[Nibaldo González]

nibags added a comment.


  `QFontMetrics::boundingRect(text).width()` doesn't deliver an exact value of 
the text width, since I detected that in some translations and some desktop 
themes the text goes below the scroll bar. This problem doesn't exist with 
`QFontMetrics::horizontalAdvance(text)`.
  
  For example, in the "Fusion" application style, with Portuguese language, it 
may be incorrectly seen:
  Before / After
  F7676460: Sin nombre.png 

REPOSITORY
  R39 KTextEditor

BRANCH
  improve-word-wrap

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

To: nibags, #ktexteditor, cullmann, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D24982: Small improvements in some XML files

[Nibaldo González]

nibags added a comment.


  I had also thought about using `QRegularExpression::DontCaptureOption`, which 
is equivalent to using `(?:...)`, but I wasn't sure how much the real 
improvement in performance is. However, disabling captures avoids allocating 
unnecessary QString for each capture.
  
  One option would be to add a **capture** or **dontCapture** attribute to 
enable or disable captures for RegExpr rules. Also, captures could be enabled 
or disabled in all RegExpr rules using the  group, adding an 
element for that.

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D24884: I18N_NOOP2 was deprecated but we can't replace by I18NC_NOOP as it expends it as 2 elements (context + text)

[Laurent Montel]

mlaurent abandoned this revision.

REPOSITORY
  R249 KI18n

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

To: mlaurent, dfaure, ilic
Cc: aacid, vkrause, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns