D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Laurent Montel]

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

INLINE COMMENTS

> CMakeLists.txt:65
> +  rename/batchrenamedialog.cpp
> +rename/batchrenamedialogmodel_p.cpp
> +rename/batchrenametypes_p.cpp

fix indent

> batchrenamedialog.cpp:48
> +{
> +auto *mainLayout = new QVBoxLayout();
> +setWindowTitle(i18nc("@title:window", "Rename Items"));

new QVBoxLayout(this);
and remove setLayout(mainLayout);

> batchrenamedialog.cpp:60
> +
> +m_previewTable = new QTableView();
> +m_previewTable->setModel(model);

add parent ?

> batchrenamedialog.cpp:172
> +// Button Bar
> +auto *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | 
> QDialogButtonBox::Cancel | QDialogButtonBox::Help);
> +m_btnOk = buttonBox->button(QDialogButtonBox::Ok);

add parent here too

> batchrenamevar_p.h:25
> +
> +#include 
> +#include 

not necessary to add QtCore

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure, mlaurent
Cc: mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, 
michaelh, bruns

D16421: Improve emblem contrast, legibility and consistency

[Nathaniel Graham]

ngraham added a comment.


  In D16421#348381 , @ndavis wrote:
  
  > Here's how that looks: F6349192: Screenshot_20181024_234540.png 

  
  
  Looks good to me!
  
  In D16421#348361 , @ndavis wrote:
  
  > Here's another attempt:
  >  F6349199: Screenshot_20181024_235038.png 

  >
  > F6349201: Screenshot_20181024_235027.png 

  >
  > F6349203: Screenshot_20181024_235012.png 

  
  
  I see what you mean. I think it looks great at the two larger sizes bug agree 
that the smaller one isn't as good. However, it's still an improvement over the 
status quo: F6349218: Screenshot_20181024_220645.png 

  
  At that small a size, it's currently illegible. So this is at least no worse.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: ngraham, bcooksley, kde-frameworks-devel, #vdg, michaelh, bruns

D16421: Improve emblem contrast, legibility and consistency

[Noah Davis]

ndavis added a comment.


  In D16421#348363 , @ngraham wrote:
  
  > Yeah I understand. I'm not the hugest fan of the orange either, and now 
that I think about it, semantically it's not really accurate either since that 
color is for warning or unusual states. Maybe it should just be blue like 
`emblem-added` is?
  
  
  Here's how that looks: F6349192: Screenshot_20181024_234540.png 

  
  > 
  > 
  > In D16421#348361 , @ndavis wrote:
  > 
  >> > Another thing is the `emblem-symbolic-link` icon. It's the only 
common-ish one that doesn't follow the pattern of having a colored background 
with a border, which might muddy the design language you've chosen (which I 
love).
  >>
  >> You're right, but I also like how the chain link looks. How is this? 
F6349126: Screenshot_20181024_222138.png 
  > 
  > 
  > With that, it doesn't look so much like a chain link anymore because the 
two sides of it are so close together. Could we basically use the existing 
chain link shape but put it within a background? Maybe it doesn't even need to 
be square; it could be rectangular, or even pill-shaped.
  
  The problem with changing `emblem-symbolic-link` to have a background is that 
it doesn't shrink down very well. You have to distort it to make it fit at all 
and retain the same look. At 8px, it's simply impossible to make the middle 
section of the chain without needing to use up the entire horizontal width.
  Here's another attempt:
  F6349199: Screenshot_20181024_235038.png 

  
  F6349201: Screenshot_20181024_235027.png 

  
  F6349203: Screenshot_20181024_235012.png 


REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: ngraham, bcooksley, kde-frameworks-devel, #vdg, michaelh, bruns

D16421: Improve emblem contrast, legibility and consistency

[Noah Davis]

ndavis edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: ngraham, bcooksley, kde-frameworks-devel, #vdg, michaelh, bruns

D16421: Improve emblem contrast, legibility and consistency

[Nathaniel Graham]

ngraham added a comment.


  In D16421#348361 , @ndavis wrote:
  
  > I agree with you, but I also don't like the look of orange. It's just not a 
color that I would expect. I would be OK with making `emblem-added` and 
`emblem-remove` grey like the reporter (Tyson Tan) from bug#399968 asked for, 
but I don't know if that would work well for programs besides Dolphin.
  >  Here's what orange would look like:
  >  F6349094: Screenshot_20181024_215403.png 

  
  
  Yeah I understand. I'm not the hugest fan of the orange either, and now that 
I think about it, semantically it's not really accurate either since that color 
is for warning or unusual states. Maybe it should just be blue like 
`emblem-added` is?
  
  In D16421#348361 , @ndavis wrote:
  
  > > Another thing is the `emblem-symbolic-link` icon. It's the only 
common-ish one that doesn't follow the pattern of having a colored background 
with a border, which might muddy the design language you've chosen (which I 
love).
  >
  > You're right, but I also like how the chain link looks. How is this? 
F6349126: Screenshot_20181024_222138.png 
  
  
  With that, it doesn't look so much like a chain link anymore because the two 
sides of it are so close together. Could we basically use the existing chain 
link shape but put it within a background? Maybe it doesn't even need to be 
square; it could be rectangular, or even pill-shaped.
  
  >> Since this fixes all three bugs, you can replace
  >> 
  >>   https://bugs.kde.org/show_bug.cgi?id=399356
  >>   https://bugs.kde.org/show_bug.cgi?id=399357
  >>   https://bugs.kde.org/show_bug.cgi?id=399968
  >> 
  >> 
  >> with
  >> 
  >>   BUG: 399356
  >>   BUG: 399357
  >>   BUG: 399968
  >>   FIXED-IN: 5.52
  > 
  > You mean I should change the summary to say that?
  
  Exactly!

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: ngraham, bcooksley, kde-frameworks-devel, #vdg, michaelh, bruns

D16421: Improve emblem contrast, legibility and consistency

[Noah Davis]

ndavis added a comment.


  In D16421#348347 , @ngraham wrote:
  
  > Wow, these are truly excellent. I think you've done an amazing job!
  
  
  Thanks!
  
  > One thing I'd like to discuss is whether or not we want the `emblem-remove` 
icon to be red. This color is typically reserved for destructive actions and 
error conditions, and the emblem as far as I can tell is only used in 
Dolphin--where its usage denotes something that is neither destructive nor an 
error. Users might be worried that clicking on it will actually remove the 
item! I wonder if a the Icon Orange color might be more suitable. What do you 
think?
  
  I agree with you, but I also don't like the look of orange. It's just not a 
color that I would expect. I would be OK with making `emblem-added` and 
`emblem-remove` grey like the reporter (Tyson Tan) from bug#399968 asked for, 
but I don't know if that would work well for programs besides Dolphin.
  Here's what orange would look like:
  F6349094: Screenshot_20181024_215403.png 

  
  > Another thing is the `emblem-symbolic-link` icon. It's the only common-ish 
one that doesn't follow the pattern of having a colored background with a 
border, which might muddy the design language you've chosen (which I love).
  
  You're right, but I also like how the chain link looks. How is this? 
F6349126: Screenshot_20181024_222138.png 
  
  > Also, I don't think the filled-in background really works: F6349078: link 
icon.png 
  
  That's fair and after using it for a little while, it's actually pretty bad 
with the dark theme. I just saw D16307 , so 
it seems to be unnecessary anyway.
  
  > Since this fixes all three bugs, you can replace
  > 
  >   https://bugs.kde.org/show_bug.cgi?id=399356
  >   https://bugs.kde.org/show_bug.cgi?id=399357
  >   https://bugs.kde.org/show_bug.cgi?id=399968
  > 
  > 
  > with
  > 
  >   BUG: 399356
  >   BUG: 399357
  >   BUG: 399968
  >   FIXED-IN: 5.52
  
  You mean I should change the summary to say that?

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: ngraham, bcooksley, kde-frameworks-devel, #vdg, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Nathaniel Graham]

ngraham added a comment.


  It makes sense to me to have that in this same patch. Pretty awesome stuff 
here.

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure
Cc: asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D16421: Improve emblem contrast, legibility and consistency

[Nathaniel Graham]

ngraham added subscribers: bcooksley, ngraham.
ngraham added a comment.


  Wow, these are truly excellent. I think you've done an amazing job!
  
  One thing I'd like to discuss is whether or not we want the `emblem-remove` 
icon to be red. This color is typically reserved for destructive actions and 
error conditions, and the emblem as far as I can tell is only used in 
Dolphin--where its usage denotes something that is neither destructive nor an 
error. Users might be worried that clicking on it will actually remove the 
item! I wonder if a the Icon Orange color might be more suitable. What do you 
think?
  
  Another thing is the `emblem-symbolic-link` icon. It's the only common-ish 
one that doesn't follow the pattern of having a colored background with a 
border, which might muddy the design language you've chosen (which I love). 
Also, I don't think the filled-in background really works: F6349078: link 
icon.png 
  
  Just throwing out some discussion points, but those are pretty minor and 
overall this is already a big improvement IMHO.
  
  ---
  
  Since this fixes all three bugs, you can replace
  
https://bugs.kde.org/show_bug.cgi?id=399356
https://bugs.kde.org/show_bug.cgi?id=399357
https://bugs.kde.org/show_bug.cgi?id=399968
  
  with
  
BUG: 399356
BUG: 399357
BUG: 399968
FIXED-IN: 5.52
  
  ---
  
  Unfortunately, the patch does not apply cleanly, and I don't think it's your 
fault:
  
arc patch D16421

[...]

Checking patch icons/emblems/22/emblem-pause.svg...
Checking patch icons/emblems/22/emblem-mounted.svg...
Checking patch dev/null => icons/emblems/22/emblem-locked.svg...
error: dev/null: does not exist in index
Checking patch icons/emblems/22/emblem-information.svg...
Checking patch icons/emblems/22/emblem-important.svg...
Checking patch dev/null => icons/emblems/22/emblem-favorite.svg...
error: dev/null: does not exist in index
Checking patch icons/emblems/22/emblem-error.svg...
Checking patch icons/emblems/22/emblem-encrypted-unlocked.svg...
[...]
Checking patch icons-dark/emblems/22/emblem-mounted.svg...
Checking patch dev/null => icons-dark/emblems/22/emblem-locked.svg...
error: dev/null: does not exist in index
Checking patch icons-dark/emblems/22/emblem-information.svg...
Checking patch icons-dark/emblems/22/emblem-important.svg...
Checking patch dev/null => icons-dark/emblems/22/emblem-favorite.svg...
error: dev/null: does not exist in index
Checking patch icons-dark/emblems/22/emblem-error.svg...
Checking patch icons-dark/emblems/22/emblem-encrypted-unlocked.svg...
  
  What's going on here is that some symlinks are being replaced with new files, 
and other files are being replaced with symlinks. `arc` doesn't seem too happy 
about this. @bcooksley or anyone else from #sysadmin 
, any idea what to do here?

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: ngraham, bcooksley, kde-frameworks-devel, #vdg, michaelh, bruns

D16421: Improve emblem contrast, legibility and consistency

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: kde-frameworks-devel, #vdg, michaelh, ngraham, bruns

D16421: Improve emblem contrast, legibility and consistency

[Noah Davis]

ndavis edited the summary of this revision.
ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg
Cc: kde-frameworks-devel, #vdg, michaelh, ngraham, bruns

D16421: Improve emblem contrast, legibility and consistency

[Noah Davis]

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

REVISION SUMMARY
  Added outlines to 16 and 22 px icons
  Improved the legibility of 8px icons
  Added new 8, 16 and 22 px versions of existing emblems
  Improved the consistency of emblem icons
  
  Related bugs:
  https://bugs.kde.org/show_bug.cgi?id=399356
  https://bugs.kde.org/show_bug.cgi?id=399357
  https://bugs.kde.org/show_bug.cgi?id=399968

REPOSITORY
  R266 Breeze Icons

BRANCH
  emblem-outlines (branched from master)

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

AFFECTED FILES
  icons-dark/emblems/16/checkmark.svg
  icons-dark/emblems/16/emblem-added.svg
  icons-dark/emblems/16/emblem-checked.svg
  icons-dark/emblems/16/emblem-encrypted-unlocked.svg
  icons-dark/emblems/16/emblem-error.svg
  icons-dark/emblems/16/emblem-favorite.svg
  icons-dark/emblems/16/emblem-important.svg
  icons-dark/emblems/16/emblem-information.svg
  icons-dark/emblems/16/emblem-locked.svg
  icons-dark/emblems/16/emblem-mounted.svg
  icons-dark/emblems/16/emblem-pause.svg
  icons-dark/emblems/16/emblem-question.svg
  icons-dark/emblems/16/emblem-remove.svg
  icons-dark/emblems/16/emblem-shared.svg
  icons-dark/emblems/16/emblem-success.svg
  icons-dark/emblems/16/emblem-symbolic-link.svg
  icons-dark/emblems/16/emblem-unavailable.svg
  icons-dark/emblems/16/emblem-unlocked.svg
  icons-dark/emblems/16/emblem-unmounted.svg
  icons-dark/emblems/16/emblem-warning.svg
  icons-dark/emblems/16/rating-unrated.svg
  icons-dark/emblems/16/rating.svg
  icons-dark/emblems/16/vcs-added.svg
  icons-dark/emblems/16/vcs-conflicting.svg
  icons-dark/emblems/16/vcs-locally-modified-unstaged.svg
  icons-dark/emblems/16/vcs-locally-modified.svg
  icons-dark/emblems/16/vcs-normal.svg
  icons-dark/emblems/16/vcs-removed.svg
  icons-dark/emblems/16/vcs-update-required.svg
  icons-dark/emblems/22/checkmark.svg
  icons-dark/emblems/22/emblem-added.svg
  icons-dark/emblems/22/emblem-checked.svg
  icons-dark/emblems/22/emblem-encrypted-unlocked.svg
  icons-dark/emblems/22/emblem-error.svg
  icons-dark/emblems/22/emblem-favorite.svg
  icons-dark/emblems/22/emblem-important.svg
  icons-dark/emblems/22/emblem-information.svg
  icons-dark/emblems/22/emblem-locked.svg
  icons-dark/emblems/22/emblem-mounted.svg
  icons-dark/emblems/22/emblem-pause.svg
  icons-dark/emblems/22/emblem-question.svg
  icons-dark/emblems/22/emblem-remove.svg
  icons-dark/emblems/22/emblem-shared.svg
  icons-dark/emblems/22/emblem-success.svg
  icons-dark/emblems/22/emblem-symbolic-link.svg
  icons-dark/emblems/22/emblem-unavailable.svg
  icons-dark/emblems/22/emblem-unlocked.svg
  icons-dark/emblems/22/emblem-unmounted.svg
  icons-dark/emblems/22/emblem-warning.svg
  icons-dark/emblems/22/rating-unrated.svg
  icons-dark/emblems/22/rating.svg
  icons-dark/emblems/22/vcs-added.svg
  icons-dark/emblems/22/vcs-conflicting.svg
  icons-dark/emblems/22/vcs-locally-modified-unstaged.svg
  icons-dark/emblems/22/vcs-locally-modified.svg
  icons-dark/emblems/22/vcs-normal.svg
  icons-dark/emblems/22/vcs-removed.svg
  icons-dark/emblems/22/vcs-update-required.svg
  icons-dark/emblems/8/emblem-added.svg
  icons-dark/emblems/8/emblem-checked.svg
  icons-dark/emblems/8/emblem-encrypted-unlocked.svg
  icons-dark/emblems/8/emblem-error.svg
  icons-dark/emblems/8/emblem-favorite.svg
  icons-dark/emblems/8/emblem-important.svg
  icons-dark/emblems/8/emblem-information.svg
  icons-dark/emblems/8/emblem-locked.svg
  icons-dark/emblems/8/emblem-mounted.svg
  icons-dark/emblems/8/emblem-pause.svg
  icons-dark/emblems/8/emblem-question.svg
  icons-dark/emblems/8/emblem-remove.svg
  icons-dark/emblems/8/emblem-shared.svg
  icons-dark/emblems/8/emblem-symbolic-link.svg
  icons-dark/emblems/8/emblem-unavailable.svg
  icons-dark/emblems/8/emblem-unlocked.svg
  icons-dark/emblems/8/emblem-unmounted.svg
  icons-dark/emblems/8/emblem-warning.svg
  icons-dark/emblems/8/rating-unrated.svg
  icons-dark/emblems/8/rating.svg
  icons-dark/emblems/8/vcs-added.svg
  icons-dark/emblems/8/vcs-conflicting.svg
  icons-dark/emblems/8/vcs-locally-modified-unstaged.svg
  icons-dark/emblems/8/vcs-locally-modified.svg
  icons-dark/emblems/8/vcs-normal.svg
  icons-dark/emblems/8/vcs-removed.svg
  icons-dark/emblems/8/vcs-update-required.svg
  icons/emblems/16/checkmark.svg
  icons/emblems/16/emblem-added.svg
  icons/emblems/16/emblem-checked.svg
  icons/emblems/16/emblem-encrypted-unlocked.svg
  icons/emblems/16/emblem-error.svg
  icons/emblems/16/emblem-favorite.svg
  icons/emblems/16/emblem-important.svg
  icons/emblems/16/emblem-information.svg
  icons/emblems/16/emblem-locked.svg
  icons/emblems/16/emblem-mounted.svg
  icons/emblems/16/emblem-pause.svg
  icons/emblems/16/emblem-question.svg
  icons/emblems/16/emblem-remove.svg
  icons/emblems/16/emblem-shared.svg
  icons/emblems/16/emblem-success.svg
  

D16395: Update the "About KDE" text

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R263 KXmlGui

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

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications, #kde_promo
Cc: xyquadrat, rizzitello, ltoscano, aspotashev, abetts, kde-frameworks-devel, 
michaelh, ngraham, bruns

D16395: Update the "About KDE" text

[Nathaniel Graham]

ngraham updated this revision to Diff 44192.
ngraham added a comment.


  Tweak the wording, incorporating @xyquadrat's suggestions

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16395?vs=44135=44192

BRANCH
  update-about-kde-text (branched from master)

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

AFFECTED FILES
  src/kaboutkdedialog_p.cpp

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications, #kde_promo
Cc: xyquadrat, rizzitello, ltoscano, aspotashev, abetts, kde-frameworks-devel, 
michaelh, ngraham, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Emirald Mateli]

emateli updated this revision to Diff 44189.
emateli added a comment.


  - use _p notation. Add BatchRenameDialog to installable headers
  
  ---
  
  Besides any current review, the last step for this patch would be to actually 
use the BatchRenameJob already in KIO. Regarding that: Should I put it into a 
separate patch and make this depend on it or continue working here?

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14631?vs=42324=44189

BRANCH
  batchrename2

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/batchrenametypestest.cpp
  autotests/batchrenametypestest.h
  src/widgets/CMakeLists.txt
  src/widgets/rename/batchrenamedialog.cpp
  src/widgets/rename/batchrenamedialog.h
  src/widgets/rename/batchrenamedialogmodel_p.cpp
  src/widgets/rename/batchrenamedialogmodel_p.h
  src/widgets/rename/batchrenametypes_p.cpp
  src/widgets/rename/batchrenametypes_p.h
  src/widgets/rename/batchrenamevar_p.cpp
  src/widgets/rename/batchrenamevar_p.h
  src/widgets/rename/filenameutils_p.cpp
  src/widgets/rename/filenameutils_p.h
  tests/CMakeLists.txt
  tests/batchrenamedialogtest_gui.cpp

To: emateli, #frameworks, dfaure
Cc: asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, michaelh, bruns

D16403: Add services and appstream runner debug categories to categories file

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:17a0d34d0e83: Add services and appstream runner debug 
categories to categories file (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16403?vs=44160=44185

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

AFFECTED FILES
  plasma-workspace.categories

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16403: Add services and appstream runner debug categories to categories file

[David Edmundson]

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

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16412: Add the possibility to have a keyboard shortcut to create a file

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> knewfilemenu.cpp:755
>  } else {
> +if (m_firstFileEntry == nullptr) {
> +m_firstFileEntry = 

`!m_firstFileEntry`

> knewfilemenu.cpp:1263-1299
> +QString text = d->m_firstFileEntry->text;
> +text.remove(QStringLiteral("...")); // the ... is fine for the menu item 
> but not for the default filename
> +text = text.trimmed(); // In some languages, there is a space in front 
> of "...", see bug 268895
> +d->m_copyData = KNewFileMenuCopyData();
> +d->m_copyData.m_src = d->m_firstFileEntry->templatePath;
> +
> +const QUrl defaultFile = QUrl::fromLocalFile(directory.toLocalFile() + 
> '/' + KIO::encodeFileName(text));

Can we try to not duplicate this code? We should create this new `createFile()` 
function such that we can also call it from `executeRealFileOrDir()`

> knewfilemenu.h:162
> + */
> +void createFile();
> +

Missing @since

REPOSITORY
  R241 KIO

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

To: thsurrel, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D16416: z/OS CLIST file syntax highlighting

[Phil Young]

phily created this revision.
phily added a reviewer: Framework: Syntax Highlighting.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
phily requested review of this revision.

REVISION SUMMARY
  There was no support for CLIST syntax highlighting so I decided to create 
this file to add support.

REPOSITORY
  R216 Syntax Highlighting

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

AFFECTED FILES
  data/syntax/clist.xml

To: phily, #framework_syntax_highlighting
Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, 
cullmann, sars, dhaumann

D16415: Creating new syntax highlighting file for Job Control Language (JCL)

[Phil Young]

phily created this revision.
phily added a reviewer: Framework: Syntax Highlighting.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
phily requested review of this revision.

REVISION SUMMARY
  There is currently no support for JCL (Job Control Language) syntax 
highlighting. This XML file adds that support.

REPOSITORY
  R216 Syntax Highlighting

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

AFFECTED FILES
  data/syntax/jcl.xml

To: phily, #framework_syntax_highlighting
Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, 
cullmann, sars, dhaumann

D16414: Changes to REXX syntax highlighting

[Phil Young]

phily created this revision.
phily added a reviewer: Framework: Syntax Highlighting.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
phily requested review of this revision.

REVISION SUMMARY
  The rexx syntax highlighting was incomplete. It was missing keywords, sub 
keywords and control flow keywords.

REPOSITORY
  R216 Syntax Highlighting

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

AFFECTED FILES
  data/syntax/rexx.xml

To: phily, #framework_syntax_highlighting
Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, 
cullmann, sars, dhaumann

D16412: Add the possibility to have a keyboard shortcut to create a file

[Thomas Surrel]

thsurrel added a dependent revision: D16413: Add a keyboard shortcut to create 
file.

REPOSITORY
  R241 KIO

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

To: thsurrel, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16412: Add the possibility to have a keyboard shortcut to create a file

[Thomas Surrel]

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

REVISION SUMMARY
  It is already possible to create a directory with a keyboard shortcut (F10 in
  Dolphin). This patch adds the same possibility for creating a file.
  The file template used will be the first one encountered in the list, so it 
should
  be a text file by default, or a user defined one if any.

REPOSITORY
  R241 KIO

BRANCH
  newfileshortcut (branched from master)

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp
  src/filewidgets/knewfilemenu.h

To: thsurrel, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16395: Update the "About KDE" text

[Julian Schraner]

xyquadrat added a comment.


  > KDE is a cooperative enterprise in which no single entity controls the 
efforts or efforts or products of KDE to the exclusion of others.
  
  I think this sentence is too complex. I'd suggest writing something like "KDE 
is a cooperative enterprise: nobody controls all efforts or products. Instead, 
we work together to achieve our common goals."

REPOSITORY
  R263 KXmlGui

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

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications, #kde_promo
Cc: xyquadrat, rizzitello, ltoscano, aspotashev, abetts, kde-frameworks-devel, 
michaelh, ngraham, bruns

D15842: Remove useless "No X-KDE-DBus-ServiceName found" message

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R297:22c5fd9f5a16: Remove useless No 
X-KDE-DBus-ServiceName found message (authored by bruns).

REPOSITORY
  R297 KDED

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15842?vs=44172=44173

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

AFFECTED FILES
  src/kded.cpp

To: bruns, #frameworks, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15842: Remove useless "No X-KDE-DBus-ServiceName found" message

[Stefan Brüns]

bruns updated this revision to Diff 44172.
bruns retitled this revision from "Lower log level for X-KDE-DBus-ServiceName 
missing message" to "Remove useless "No X-KDE-DBus-ServiceName found" message".
bruns added a comment.


  Completely remove message

REPOSITORY
  R297 KDED

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15842?vs=42570=44172

BRANCH
  master

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

AFFECTED FILES
  src/kded.cpp

To: bruns, #frameworks, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16395: Update the "About KDE" text

[Nathaniel Graham]

ngraham added a reviewer:  KDE Promo.
ngraham added a comment.


  Agreed, we can wait until after 5.52 is tagged before landing this (assume we 
even get consensus before then. :) )
  
  In D16395#347814 , @abetts wrote:
  
  > > KDE has created the friendly and powerful Plasma desktop environment
  >
  > KDE has created the most friendly and powerful desktop environment, Plasma.
  
  
  That seems like boasting to me, and invites comparisons to other environments 
that we could lose in some people's eyes. I'd prefer not to make this change,
  
  In D16395#347814 , @abetts wrote:
  
  > > KDE is a cooperative enterprise in which no single entity
  >
  > KDE is a disaggregated development community not governed by...
  
  
  Hmm, I like the existing one better. In particular, "disaggregated" is a very 
complex and uncommon word that could be avoided in the interests of clear 
communication.

REPOSITORY
  R263 KXmlGui

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

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications, #kde_promo
Cc: rizzitello, ltoscano, aspotashev, abetts, kde-frameworks-devel, michaelh, 
ngraham, bruns

D15842: Lower log level for X-KDE-DBus-ServiceName missing message

[David Edmundson]

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


  If it's not a requirement for it to be set (and it isn't) I agree it's not a 
useful warning. But it's also not remotely useful debug.
  
  It's easy to see when something is registered, and we still have a warning if 
registration fails.
  
  Just delete it.

REPOSITORY
  R297 KDED

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16392: [Config] Remove KDE4 config support, stop writing arbitrary config files

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:f7d13812e2c5: [Config] Remove KDE4 config support, stop 
writing arbitrary config files (authored by bruns).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16392?vs=44130=44171

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

AFFECTED FILES
  src/lib/indexerconfig.cpp

To: bruns, #baloo, #frameworks, poboiko, ngraham
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D16392: [Config] Remove KDE4 config support, stop writing arbitrary config files

[Nathaniel Graham]

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


  Fair enough, that's true. Go for it!

REPOSITORY
  R293 Baloo

BRANCH
  excludes

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

To: bruns, #baloo, #frameworks, poboiko, ngraham
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D15842: Lower log level for X-KDE-DBus-ServiceName missing message

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R297 KDED

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

To: bruns, #frameworks, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

bruns added a dependent revision: D16410: [Bookmarks Runner] Avoid leaking 
FetchSqlite instances.

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

bruns added a dependency: D16405: Add debug category for bookmarks runner.

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[Stefan Brüns]

bruns added a dependent revision: D16409: [Bookmarks Runner] Open database 
connection in the query thread.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16410: [Bookmarks Runner] Avoid leaking FetchSqlite instances

[Stefan Brüns]

bruns added a dependency: D16409: [Bookmarks Runner] Open database connection 
in the query thread.

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16410: [Bookmarks Runner] Avoid leaking FetchSqlite instances

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  The favicon and places instances are created in the prepare() call,
  leaking the instance from the last invocation. Delete the old
  instance in teardown().
  
  See also T9626 .

TEST PLAN
  run several queries in krunner
  -> instances are no longer leaked

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  QSqlDatabase connection instances are global reference counted objects which
  can be crated using the QSqlDatabase::addDatabase(...) method and later
  retrieved with QSqlDatabase::database(connectionname).
  
  Connections should only be used from a single thread, and should be cleaned
  up with QSD::removeDatabase(name), which implicitly closes the connection
  when the last reference is removed.
  
  Currently, the same connection (name) is reused over all threads, and
  the connection is only removed implicitly by replacing it via addDatabase().
  This leads to warnings, i.e.: "QSqlDatabasePrivate::addDatabase: duplicate
  connection name '...', old connection removed."
  
  As one reference is held by the m_db member, the implicit removal triggers
  another warning: "QSqlDatabasePrivate::removeDatabase: connection '..."
  is still in use, all queries will cease to work."
  
  According to the documentation, "It is highly recommended that you do not
  keep a copy of the QSqlDatabase around as a member of a class, as this
  will prevent the instance from being correctly cleaned up on shutdown."
  There is no need to keep a reference using a member variable, as retrieving
  an open connection via QSqlDatabase::database(...) is cheap.
  
  Create a per-thread DB connection on first use, and remove the connections
  when the FetchSqlite instance is torn down.
  
  Delaying the open is beneficial for the favicon instance, which may be
  unused when the icon is already cached on disk.
  
  See also T9626 .

TEST PLAN
  execute a query in krunner
  
  1. Results are as expected
  2. The 'QSqlDatabasePrivate::removeDatabase: connection ... is still in use, 
all queries will cease to work.' warning no longer appears
  3. The 'QSqlDatabasePrivate::addDatabase: duplicate connection name ..., old 
connection removed' warning no longer appears

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  runners/bookmarks/fetchsqlite.cpp
  runners/bookmarks/fetchsqlite.h

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[Stefan Brüns]

bruns added a comment.


  Only applies cleanly after D16403 .

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[Stefan Brüns]

bruns added a dependency: D16403: Add services and appstream runner debug 
categories to categories file.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16403: Add services and appstream runner debug categories to categories file

[Stefan Brüns]

bruns added a dependent revision: D16405: Add debug category for bookmarks 
runner.

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[Stefan Brüns]

bruns added a comment.


  Depends on the cleanup in D16404 , as the 
autogenerated `bookmarks_debug.cpp` is not linked to the 
tests/testChromeBookmarks binary otherwise.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16404: [Bookmarks Runner] Cleanup tests CMakeList

[Stefan Brüns]

bruns added a dependent revision: D16405: Add debug category for bookmarks 
runner.

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[Stefan Brüns]

bruns added a dependency: D16404: [Bookmarks Runner] Cleanup tests CMakeList.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[David Edmundson]

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

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Dependency for followup patches, no functional change

TEST PLAN
  make

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  plasma-workspace.categories
  runners/bookmarks/CMakeLists.txt

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16402: Cleanup - remove unnecessary QIcon include

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:9945aecaf2eb: Cleanup - remove unnecessary QIcon include 
(authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16402?vs=44159=44163

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

AFFECTED FILES
  runners/bookmarks/bookmarkmatch.h

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16404: [Bookmarks Runner] Cleanup tests CMakeList

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Build the test dependencies as static library. Remove duplicate and
  commented out lines. Use ecm_add_test instead of setting up everything
  manually.

TEST PLAN
  make && ctest

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  runners/bookmarks/CMakeLists.txt
  runners/bookmarks/tests/CMakeLists.txt

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16402: Cleanup - remove unnecessary QIcon include

[David Edmundson]

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

REPOSITORY
  R120 Plasma Workspace

BRANCH
  db_fix_2

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

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16403: Add services and appstream runner debug categories to categories file

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Make the debug categories visible in kdebugsettings

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  plasma-workspace.categories

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16402: Cleanup - remove unnecessary QIcon include

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  QIcon is only used in the implementation, not in the interface.

TEST PLAN
  compile tested, no functional changes

REPOSITORY
  R120 Plasma Workspace

BRANCH
  db_fix_2

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

AFFECTED FILES
  runners/bookmarks/bookmarkmatch.h

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16387: [sftp] put request chunk debugging into own category

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R320:bf4563e8d291: [sftp] put request chunk debugging into own 
category (authored by sitter).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16387?vs=44112=44153#toc

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16387?vs=44112=44153

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

AFFECTED FILES
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: sitter, asn, broulik, ngraham
Cc: kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

D15070: Bindings: Support using sys paths for python install directory

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> FindPythonModuleGeneration.cmake:455
>  install(DIRECTORY 
> ${CMAKE_BINARY_DIR}/py${pyversion}/${GPB_PYTHONNAMESPACE}
> -DESTINATION 
> lib/python${pyversion${pyversion}_maj_min}/site-packages)
>  install(FILES ${sip_files} 
> "${CMAKE_CURRENT_BINARY_DIR}/sip/${GPB_PYTHONNAMESPACE}/${GPB_MODULENAME}/${GPB_MODULENAME}mod.sip"

Hardcoded install directory - keeping this for backwards compatibility!

REPOSITORY
  R240 Extra CMake Modules

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

To: bruns, #frameworks
Cc: cgiboudeaux, bcooksley, kde-frameworks-devel, kde-buildsystem, michaelh, 
ngraham, bruns

D15070: Bindings: Support using sys paths for python install directory

[Stefan Brüns]

bruns added a comment.


  In D15070#347945 , @cgiboudeaux 
wrote:
  
  > In D15070#347944 , @bruns wrote:
  >
  > > So, after another week, no reason has been given not to accept this.
  > >
  > > 1. It fixes broken behavior on several platforms
  > > 2. It does not break current setups
  > > 3. It is consistent with other config variables
  >
  >
  > That's not true, you're refusing to fix the issues. Why should we invest 
time reviewing your changes, exactly?
  
  
  I answered this inline
  
  > - The wrong hardcoded lib/ destination wasn't fixed
  
  If I change this, it breaks the backwards compatibility. It currently is 
broken, and you have to opt-in in the current fixed behaviour.
  
  > - The empty 'if' is still there
  
  And I answered why it is there. Your proposals how to "fix" this leads to 
inconsistent behavior. Changing this in a way which keeps consistent behaviour 
makes the code less readable (either more nesting or longer conditions in the 
if statement). Policy is not there to be followed blindly.

REPOSITORY
  R240 Extra CMake Modules

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

To: bruns, #frameworks
Cc: cgiboudeaux, bcooksley, kde-frameworks-devel, kde-buildsystem, michaelh, 
ngraham, bruns

D15070: Bindings: Support using sys paths for python install directory

[Christophe Giboudeaux]

cgiboudeaux added a comment.


  In D15070#347944 , @bruns wrote:
  
  > So, after another week, no reason has been given not to accept this.
  >
  > 1. It fixes broken behavior on several platforms
  > 2. It does not break current setups
  > 3. It is consistent with other config variables
  
  
  That's not true, you're refusing to fix the issues. Why should we invest time 
reviewing your changes, exactly?
  
  - The wrong hardcoded lib/ destination wasn't fixed
  - The empty 'if' is still there

REPOSITORY
  R240 Extra CMake Modules

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

To: bruns, #frameworks
Cc: cgiboudeaux, bcooksley, kde-frameworks-devel, kde-buildsystem, michaelh, 
ngraham, bruns

D15070: Bindings: Support using sys paths for python install directory

[Stefan Brüns]

bruns added a comment.


  So, after another week, no reason has been given not to accept this.
  
  1. It fixes broken behavior on several platforms
  2. It does not break current setups
  3. It is consistent with other config variables

REPOSITORY
  R240 Extra CMake Modules

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

To: bruns, #frameworks
Cc: cgiboudeaux, bcooksley, kde-frameworks-devel, kde-buildsystem, michaelh, 
ngraham, bruns

D16395: Update the "About KDE" text

[Luigi Toscano]

ltoscano added a comment.


  
  
  In D16395#347938 , @rizzitello 
wrote:
  
  > Why does this dialog not say anything about the version of KDE.
  
  
  There is *no* version of KDE. KDE is only the community. It's clearly stated 
in that message.
  
  > I think here the KF5 version and maybe the Qt Version should be shown here. 
I always found it kinda odd that the "About Application" dialog contains this 
and not the About KDE dialog.
  
  That's the right place where they belong: information about the application 
and its dependencies.

REPOSITORY
  R263 KXmlGui

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

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications
Cc: rizzitello, ltoscano, aspotashev, abetts, kde-frameworks-devel, michaelh, 
ngraham, bruns

D16395: Update the "About KDE" text

[Chris Rizzitello]

rizzitello added a comment.


  Why does this dialog not say anything about the version of KDE. I think here 
the KF5 version and maybe the Qt Version should be shown here. I always found 
it kinda odd that the "About Application" dialog contains this and not the 
About KDE dialog.

REPOSITORY
  R263 KXmlGui

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

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications
Cc: rizzitello, ltoscano, aspotashev, abetts, kde-frameworks-devel, michaelh, 
ngraham, bruns

D7038: [server] Respect input region of sub-surfaces on pointer surface focus

[Roman Gilg]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R127:24bdc06747ed: [server] Respect input region of 
sub-surfaces on pointer surface focus (authored by romangg).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7038?vs=44149=44151

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

AFFECTED FILES
  src/server/pointer_interface.cpp
  src/server/surface_interface.cpp
  src/server/surface_interface.h

To: romangg, #frameworks, graesslin, davidedmundson
Cc: davidedmundson, zzag, kde-frameworks-devel, graesslin, plasma-devel, 
ragreen, Pitel, schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D16266: [Extractor] Make extractor crash resilient

[Stefan Brüns]

bruns marked an inline comment as done.
bruns added inline comments.

INLINE COMMENTS

> poboiko wrote in filecontentindexer.cpp:91
> OK, we can simply count how many times we got `finishedIndexingFile`, and 
> just go to the corresponding position in the batch.
> It's just the binary search here does look a bit unnecessary to me...

Counting would give a good hint which one failed, but still no guarantee. Files 
may be added or removed during the indexer run, so the position is only 
approximate. The indexer may also have sent a "finished" message, and crash 
afterwards in a destructor call.

Doing a binary search is straight forward and avoids any dependencies or 
assumptions about other parts of the code.

Also, this code should be only temporary anyway - if the extractor is run in a 
separate process which only receives the file using a readonly file descriptor 
(for sandboxing) and passes back the result, the problematic documents id is 
known by the parent process.

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, #frameworks, poboiko, ngraham
Cc: broulik, apol, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

KDE CI: Frameworks » kwayland » kf5-qt5 SUSEQt5.10 - Build # 93 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kwayland/job/kf5-qt5%20SUSEQt5.10/93/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Wed, 24 Oct 2018 11:59:04 +
 Build duration:
5 min 27 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 44 test(s), Skipped: 0 test(s), Total: 45 test(s)Failed: TestSuite.kwayland-testWaylandSeat
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report63%
(5/8)92%
(234/254)92%
(234/254)87%
(24630/28453)53%
(9715/18369)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client100%
(41/41)100%
(41/41)98%
(11551/11758)50%
(6107/12323)autotests.server100%
(5/5)100%
(5/5)99%
(353/356)49%
(169/344)src.client99%
(71/72)99%
(71/72)85%
(5724/6770)65%
(1503/2322)src.compat100%
(2/2)100%
(2/2)100%
(81/81)100%
(0/0)src.server100%
(115/115)100%
(115/115)87%
(6921/7979)65%
(1936/2965)src.tools0%
(0/2)0%
(0/2)0%
(0/693)0%
(0/272)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/69)0%
(0/10)tests0%
(0/14)0%
(0/14)0%
(0/747)0%
(0/133)

D7038: [server] Respect input region of sub-surfaces on pointer surface focus

[Roman Gilg]

romangg updated this revision to Diff 44149.
romangg added a comment.


  Rebase on master.

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7038?vs=42733=44149

BRANCH
  inputRegionSubSurfaces

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

AFFECTED FILES
  src/server/pointer_interface.cpp
  src/server/surface_interface.cpp
  src/server/surface_interface.h

To: romangg, #frameworks, graesslin, davidedmundson
Cc: davidedmundson, zzag, kde-frameworks-devel, graesslin, plasma-devel, 
ragreen, Pitel, schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D7038: [server] Respect input region of sub-surfaces on pointer surface focus

[Roman Gilg]

romangg edited the summary of this revision.
romangg edited the test plan for this revision.

REPOSITORY
  R127 KWayland

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

To: romangg, #frameworks, graesslin, davidedmundson
Cc: davidedmundson, zzag, kde-frameworks-devel, graesslin, plasma-devel, 
ragreen, Pitel, schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D16392: [Config] Remove KDE4 config support, stop writing arbitrary config files

[Stefan Brüns]

bruns added a comment.


  In D16392#347781 , @ngraham wrote:
  
  > Looks good, but what's our transition plan? Does this need any special 
handling for people who already have old config files?
  
  
  Currently, there is some garbage left in a few config files (potentially any 
application which **writes** some baloo config keys), on my system:
  
  balooctl: ~/.config/baloorc
  systemsettings/search: ~/.config/kcmshell5rc, ~/.config/systemsettingsrc
  
  I am not aware of any bad side effects having these garbage entries. baloo 
itself (or any of its library functions) newer read these keys from
  the mentioned files, only from ~/.config/baloofilerc.
  
  Adding cleanup code is of course possible, but this code:
  
  - has to be run at an appropriate time (each config read? write? session 
start?)
  - has to parse the config (at least systemsettingsrc is quite large)
  - is extra code which has to be maintained
  
  As this change is an improvement even without the cleanup, I would postpone 
the cleanup.

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, #frameworks, poboiko, ngraham
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D16395: Update the "About KDE" text

[Luigi Toscano]

ltoscano added a comment.


  I'd say that the respecting the string freeze this time makes sense, as we 
need a bit more time to discuss this. Maybe the promo team can help too, as 
this is an important message.

REPOSITORY
  R263 KXmlGui

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

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications
Cc: ltoscano, aspotashev, abetts, kde-frameworks-devel, michaelh, ngraham, bruns

D16395: Update the "About KDE" text

[Alexander Potashev]

aspotashev added a comment.


  FYI, according to an old custom mentioned at 
https://community.kde.org/Schedules/Frameworks , KF5.52 is already in string 
freeze, thus this change may only be pushed after 5.52 is released. However you 
can ask for an exception in the kde-i18n-doc mailing list unless it gets too 
close to KF5.52 tagging day.

REPOSITORY
  R263 KXmlGui

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

To: ngraham, #vdg, #plasma, #frameworks, #kde_applications
Cc: aspotashev, abetts, kde-frameworks-devel, michaelh, ngraham, bruns

D16350: Allow KHelpCenter to open the right pages of KDE help when KHelpClient is invoked with an anchor

[Yuri Chornoivan]

This revision was automatically updated to reflect the committed changes.
Closed by commit R265:8a9cec559211: Allow KHelpCenter to open the right pages 
of KDE help when KHelpClient is… (authored by yurchor).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16350?vs=44126=44145#toc

REPOSITORY
  R265 KConfigWidgets

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16350?vs=44126=44145

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

AFFECTED FILES
  src/khelpclient.cpp

To: yurchor, #documentation, #frameworks, mlaurent
Cc: mlaurent, kde-frameworks-devel, michaelh, ngraham, bruns

D16385: [sftp] reformat to 4 space indentation

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R320:972b5f5ef074: [sftp] reformat to 4 space indentation 
(authored by sitter).

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16385?vs=44109=44144

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

AFFECTED FILES
  sftp/kio_sftp.cpp

To: sitter, asn, ngraham, apol
Cc: asn, kde-frameworks-devel, kfm-devel, broulik, sourabhboss, feverfew, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

KDE CI: Frameworks » plasma-framework » kf5-qt5 SUSEQt5.9 - Build # 174 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/plasma-framework/job/kf5-qt5%20SUSEQt5.9/174/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Wed, 24 Oct 2018 08:14:10 +
 Build duration:
3 min 57 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 14 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.plasma-iconitemtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(9/18)45%
(57/126)45%
(57/126)39%
(5190/13182)29%
(2738/9444)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests93%
(13/14)93%
(13/14)96%
(1068/1117)51%
(552/1084)src.declarativeimports.calendar0%
(0/6)0%
(0/6)0%
(0/463)0%
(0/231)src.declarativeimports.core44%
(7/16)44%
(7/16)33%
(749/2240)27%
(386/1442)src.declarativeimports.plasmacomponents0%
(0/6)0%
(0/6)0%
(0/497)0%
(0/187)src.declarativeimports.plasmaextracomponents0%
(0/3)0%
(0/3)0%
(0/42)0%
(0/22)src.declarativeimports.platformcomponents0%
(0/3)0%
(0/3)0%
(0/58)0%
(0/14)src.declarativeimports.platformcomponents.utils0%
(0/2)0%
(0/2)0%
(0/14)0%
(0/2)src.plasma64%
(14/22)64%
(14/22)48%
(1700/3506)39%
(1030/2633)src.plasma.packagestructure57%
(4/7)57%
(4/7)37%
(51/138)42%
(5/12)src.plasma.private63%
(12/19)63%
(12/19)61%
(955/1559)43%
(428/1003)src.plasma.scripting67%
(2/3)67%
(2/3)20%
(34/166)10%
(13/128)src.plasmapkg0%
(0/1)0%
(0/1)0%
(0/45)0%
(0/40)src.plasmaquick27%
(3/11)27%
(3/11)30%
(589/1977)19%
(319/1702)src.plasmaquick.private50%
(1/2)50%
(1/2)29%
(31/107)36%
(5/14)src.scriptengines.qml.plasmoid17%
(1/6)17%
(1/6)1%
(13/1098)0%
(0/906)tests.dpi0%
(0/2)0%
(0/2)0%
(0/21)0%
(0/2)tests.kplugins0%
(0/2)0%
(0/2)0%
(0/61)0%
(0/16)tests.testengine0%

D16311: RFC: [KFilePlacesView] Use asynchronous KIO::FileSystemFreeSpaceJob

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kfileplacesview.cpp:257
> You can make lambda mutable in there you can modify m_freeSpaceInfo without 
> declare it mutable

No ignore it, it does not work.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, dfaure, lbeltrame
Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D16311: RFC: [KFilePlacesView] Use asynchronous KIO::FileSystemFreeSpaceJob

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> kfileplacesview.cpp:256
> +(!info.lastUpdated.isValid() || 
> info.lastUpdated.secsTo(QDateTime::currentDateTimeUtc()) > 60)) {
> +info.job = KIO::fileSystemFreeSpace(url);
> +connect(info.job, ::FileSystemFreeSpaceJob::result, 
> this, [this, persistentIndex](KIO::Job *job, KIO::filesize_t size, 
> KIO::filesize_t available) {

You will get the job as lambda parameter.

> kfileplacesview.cpp:257
> +info.job = KIO::fileSystemFreeSpace(url);
> +connect(info.job, ::FileSystemFreeSpaceJob::result, 
> this, [this, persistentIndex](KIO::Job *job, KIO::filesize_t size, 
> KIO::filesize_t available) {
> +PlaceFreeSpaceInfo  = 
> m_freeSpaceInfo[persistentIndex];

You can make lambda mutable in there you can modify m_freeSpaceInfo without 
declare it mutable

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, dfaure, lbeltrame
Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D16265: [Scheduler] Use flag to track when a runner is going idle

[Luca Beltrame]

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


  The changes look sane to me. Perhaps wait a couple more days until any other 
objection is raised, then if not, commit away.

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, #frameworks, poboiko, ngraham, lbeltrame
Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D16311: RFC: [KFilePlacesView] Use asynchronous KIO::FileSystemFreeSpaceJob

[Luca Beltrame]

lbeltrame added a comment.


  +1, but I'd like to hear people more experienced than me.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, dfaure, lbeltrame
Cc: kde-frameworks-devel, michaelh, ngraham, bruns