D18509: Fixed Bug Which Caused Infinite Loop While Highlighting Kconfig Files

[Dominik Haumann]

dhaumann added a comment.


  Related bug report: https://bugs.kde.org/show_bug.cgi?id=403215

REPOSITORY
  R216 Syntax Highlighting

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

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

D18688: Check Exiv2::ValueType::typeId before converting it to rational

[Igor Poboiko]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:6e449d44bb5d: Check Exiv2::ValueType::typeId before 
converting it to rational (authored by poboiko).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18688?vs=50751&id=50760

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

AFFECTED FILES
  src/extractors/exiv2extractor.cpp

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

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.10 - Build # 11 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.10/11/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 03 Feb 2019 11:19:34 +
 Build duration:
15 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlcompat_reports/KF5FileMetaData_compat_report.htmllogs/KF5FileMetaData/5.55.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 15 test(s), Skipped: 0 test(s), Total: 16 test(s)Failed: projectroot.autotests.taglibextractortest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report80%
(4/5)87%
(48/55)87%
(48/55)82%
(3146/3848)48%
(4648/9737)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(16/16)100%
(16/16)98%
(852/871)49%
(1855/3769)src75%
(18/24)75%
(18/24)80%
(1061/1329)45%
(478/1060)src.extractors100%
(13/13)100%
(13/13)75%
(1175/1563)47%
(2244/4747)src.writers100%
(1/1)100%
(1/1)98%
(58/59)48%
(71/149)tests0%
(0/1)0%
(0/1)0%
(0/26)0%
(0/12)

KDE CI: Frameworks » kfilemetadata » kf5-qt5 SUSEQt5.12 - Build # 6 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kfilemetadata/job/kf5-qt5%20SUSEQt5.12/6/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Sun, 03 Feb 2019 11:19:34 +
 Build duration:
15 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlcompat_reports/KF5FileMetaData_compat_report.htmllogs/KF5FileMetaData/5.55.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 15 test(s), Skipped: 0 test(s), Total: 16 test(s)Failed: projectroot.autotests.taglibextractortest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report80%
(4/5)87%
(48/55)87%
(48/55)82%
(3146/3848)48%
(4648/9737)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(16/16)100%
(16/16)98%
(852/871)49%
(1855/3769)src75%
(18/24)75%
(18/24)80%
(1061/1329)45%
(478/1060)src.extractors100%
(13/13)100%
(13/13)75%
(1175/1563)47%
(2244/4747)src.writers100%
(1/1)100%
(1/1)98%
(58/59)48%
(71/149)tests0%
(0/1)0%
(0/1)0%
(0/26)0%
(0/12)

D18125: KateStatusBar: Add dictionary button

[loh tar]

loh.tar updated this revision to Diff 50763.
loh.tar set the repository for this revision to R39 KTextEditor.
loh.tar added a comment.


  - Apply nullptr check
  - Add non preferred dict to menu chose by user
  - Reorder actions
  - Remove gone "Spellcheck Selection" action

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18125?vs=49099&id=50763

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

AFFECTED FILES
  src/view/katestatusbar.cpp
  src/view/katestatusbar.h

To: loh.tar, #ktexteditor, cullmann
Cc: cullmann, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, hase, michaelh, bruns, demsking, sars

D18694: Add edit-none icon

[Björn Feber]

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

REVISION SUMMARY
  Adds an `edit-none` icon, needed by D18675 
.
  F6584574: edit-none.png 

TEST PLAN
  Open Cuttlefish and search for the icon.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-edit-none-icon (branched from master)

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

AFFECTED FILES
  icons-dark/actions/16/edit-none.svg
  icons/actions/16/edit-none.svg

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

D17302: Add test for adding properties to result

[Alexander Stippich]

astippich added a comment.


  Since nobody seems to care and this is just testing the status quo, I'll 
merge it next week unless somebody objects

REPOSITORY
  R293 Baloo

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

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

D18694: Add edit-none icon

[Nathaniel Graham]

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

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-edit-none-icon (branched from master)

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

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

D18694: Add edit-none icon

[Björn Feber]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:f3d16cb641ea: Add edit-none icon (authored by GB_2).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18694?vs=50767&id=50772

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

AFFECTED FILES
  icons-dark/actions/16/edit-none.svg
  icons/actions/16/edit-none.svg

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

D18675: Use different "None" item icon in grid view KCMs

[Björn Feber]

GB_2 updated this revision to Diff 50774.
GB_2 added a comment.


  Use new `edit-none` icon

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18675?vs=50724&id=50774

BRANCH
  use-different-icon-for-none-item (branched from master)

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/qml/GridDelegate.qml

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

D18675: Use different "None" item icon in grid view KCMs

[Björn Feber]

GB_2 edited the summary of this revision.

REPOSITORY
  R296 KDeclarative

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

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

D18675: Use different "None" item icon in grid view KCMs

[Nathaniel Graham]

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


  Master only please.

REPOSITORY
  R296 KDeclarative

BRANCH
  use-different-icon-for-none-item (branched from master)

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

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

D18675: Use different "None" item icon in grid view KCMs

[Björn Feber]

This revision was automatically updated to reflect the committed changes.
Closed by commit R296:e22529eeed0c: Use different "None" item icon in 
grid view KCMs (authored by GB_2).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18675?vs=50774&id=50775

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/qml/GridDelegate.qml

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

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

[Igor Poboiko]

poboiko created this revision.
poboiko added reviewers: Baloo, Frameworks.
Herald added projects: Frameworks, Baloo.
poboiko requested review of this revision.

REVISION SUMMARY
  If a folder was moved from an unwatched place, `KInotify` will receive an 
`EventMoveTo` event,
  which doesn't have an `EventMoveFrom` counterpart, and thus it will emit only 
`created` signal
  for the moved directory, but not its contents.
  Also, it won't install watches for the directory (as it does in 
`EventCreate`).
  
  Instead use FilteredDirIterator to emit created() signal for all the contents 
as well, and add inotify watch
  
  Few side notes:
  
  1. It's not really realted to symbolic links, we just need to move a folder 
from excluded place to included.
  2. If we move folder from another device, `Inotify` doesn't create `Move` 
signal, but `Create` instead, which seems to be handled properly
  
  (I'm not really sure why, because it smells like a race condition here. 
  If i.e. we got `Create` signal -> then `file1` gets moved -> then we add 
watches -> then `file2` gets moved, information about file1 should get lost.
  Am I missing something here?)
  
  BUG: 342224

TEST PLAN
  Added a test case for `KInotifyTest`, similar to one described in Bug 342224. 
  It passes now.

REPOSITORY
  R293 Baloo

BRANCH
  add-watch-moved (branched from master)

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

AFFECTED FILES
  autotests/unit/file/kinotifytest.cpp
  src/file/kinotify.cpp

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

D18699: [KIO/RenameDialog] Add new apply behaviour

[Chinmoy Ranjan Pradhan]

chinmoyr created this revision.
chinmoyr added reviewers: dfaure, ngraham.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
chinmoyr requested review of this revision.

REVISION SUMMARY
  This patch removes "Apply to All" checkbox and adds a combobox with following 
4 apply options:
  
  1. Apply to Current : Same as "Apply to All" unchecked.
  2. Apply to All
  3. Apply to Newer Dest : Applies changes (currently only overwrite) to 
conflicts with a newer version of dest file/dir. And skip the rest.
  4. Apply to Older Dest : Same as above but with older dest.
  
  F6584726: apply options 
  
  In this version of patch when selecting the last two options only 
"Overwrite"/"Write Into" button is enabled.
  
  F6584732: enabled buttons 
  
  As a result this patch also modifies CopyJob to implement the new overwrite 
behaviour.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/core/jobuidelegateextension.h
  src/widgets/renamedialog.cpp
  src/widgets/renamedialog.h

To: chinmoyr, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18699: [KIO/RenameDialog] Add new apply behaviour

[Chinmoy Ranjan Pradhan]

chinmoyr updated this revision to Diff 50778.
chinmoyr edited the summary of this revision.
chinmoyr added a comment.


  Add bug id.
  Remove old code.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18699?vs=50777&id=50778

BRANCH
  master

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/core/jobuidelegateextension.h
  src/widgets/renamedialog.cpp
  src/widgets/renamedialog.h

To: chinmoyr, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18699: [KIO/RenameDialog] Add new apply behaviour

[Kai Uwe Broulik]

broulik added a comment.


  I find the wording and usability a bit confusing, "Older Dest", "Newer Dest", 
etc. I think that dialog needs to be overhauled entirely for this approach to 
work, cf. to how windows uses command links for that (they're also quite hard 
to grasp at a glance imho), not sure, perhaps usability knows a good solution 
for this problem.

REPOSITORY
  R241 KIO

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

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

D17816: Support for xattrs on kio copy/move

[Cochise César]

cochise added a comment.


  Can we ship it, or using a subjob on `KIO::copy` is mandatory here?

REPOSITORY
  R241 KIO

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

To: cochise, dfaure
Cc: cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, 
ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, michaelh

D18699: [KIO/RenameDialog] Add new apply behaviour

[Chinmoy Ranjan Pradhan]

chinmoyr added a reviewer: VDG.

REPOSITORY
  R241 KIO

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

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

D18699: [KIO/RenameDialog] Add new apply behaviour

[Christoph Feck]

cfeck added a comment.


  The "Apply to All" button is used for all choices, including "Overwrite", 
"Skip", and "Rename".

REPOSITORY
  R241 KIO

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

To: chinmoyr, dfaure, ngraham, #vdg
Cc: cfeck, broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D18702: Remove monochrome font preferences icon and improve font selection icon

[Björn Feber]

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

REVISION SUMMARY
  Removes the monochrome font preferences icon to prevent showing it instead of 
the colorful icon in the system settings "Fonts" category tooltip. This also 
improves the font selection icons by making the three dots black/white instead 
of orange.
  F6584832: Fixed System Settings Fonts Category Tooltip Icon.png 

  F6584835: Improved Font Selection Icon.png 


TEST PLAN
  Open the system settings and hover over the "Fonts" category. Also search 
"font" in Cuttlefish, it should show the improved font selection icons.

REPOSITORY
  R266 Breeze Icons

BRANCH
  remove-monochrome-font-preferences-icon-and-improve-font-selection-icon 
(branched from master)

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

AFFECTED FILES
  icons-dark/actions/16/gtk-select-font.svg
  icons-dark/actions/22/gtk-select-font.svg
  icons-dark/actions/24/gtk-select-font.svg
  icons-dark/apps/22/preferences-desktop-font.svg
  icons/actions/16/gtk-select-font.svg
  icons/actions/22/gtk-select-font.svg
  icons/actions/24/gtk-select-font.svg
  icons/apps/22/preferences-desktop-font.svg

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

D18680: Fix ENABLE_CLAZY option with clazy >= 1.5

[Elvis Angelaccio]

elvisangelaccio updated this revision to Diff 50790.
elvisangelaccio marked an inline comment as done.
elvisangelaccio added a comment.


  Addressed comments

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18680?vs=50735&id=50790

BRANCH
  master

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

AFFECTED FILES
  kde-modules/KDECMakeSettings.cmake

To: elvisangelaccio, apol, smartins
Cc: arojas, anthonyfieroni, kde-frameworks-devel, kde-buildsystem, michaelh, 
ngraham, bruns

D18680: Fix ENABLE_CLAZY option with clazy >= 1.5

[Elvis Angelaccio]

elvisangelaccio edited the summary of this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: elvisangelaccio, apol, smartins
Cc: arojas, anthonyfieroni, kde-frameworks-devel, kde-buildsystem, michaelh, 
ngraham, bruns

Re: [sysadmin/ci-tooling] build-specs/Plasma: Disable execution of tests for plasma-integration.

[David Faure]

On vendredi 1 février 2019 10:32:19 CET Ben Cooksley wrote:
> On Fri, Feb 1, 2019 at 9:15 PM David Faure  wrote:
> > I'll try to get around to debugging this, but I'm surprised by one thing:
> > doesn't the same issue happen in KIO's own tests? It also has several
> > tests
> > that make use of KIO slaves directly, skipping KLauncher.
> 
> Indeed, I found it surprising as well.
> 
> Only thing I can think of is that the tests in KIO tell the slaves to
> exit before it finishes itself and exits - while the tests in
> plasma-integration don't do this.

There's no API in KIO for telling slaves to exit; they are supposed to just do 
that by themselves, after being idle for a while.

> > Ironically, the reason we skip klauncher is exactly to avoid this problem
> > in the first place (kdeinit and klauncher would keep running after the
> > test ended).
> 
> The CI tooling should eliminate the need to do this for the most part
> as long as you don't try to launch them yourself, as it will launch
> kdeinit as part of the environment setup process :)
> https://invent.kde.org/sysadmin/ci-tooling/blob/master/helpers/run-tests.py#

Well, let's try without KDE_FORK_SLAVES then?

I just pushed that in 
https://commits.kde.org/plasma-integration/a94bbdaadff282a26dece12172234b8bec1a29c9
also merged to master.

Can you re-enable CI and let me know if it works better - or worse? ;-)

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

D18702: Remove monochrome font preferences icon and improve font selection icon

[Noah Davis]

ndavis added a comment.


  Is this icon used by any other KDE software or 3rd party applications?

REPOSITORY
  R266 Breeze Icons

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

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

D18714: Add some Maxima constans to MaximaVariable list

[Nikita Sirgienko]

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

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

AFFECTED FILES
  data/syntax/maxima.xml

To: sirgienko, #framework_syntax_highlighting
Cc: kwrite-devel, kde-frameworks-devel, bmortimer, hase, michaelh, genethomas, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D18714: Add some Maxima constans to MaximaVariable list

[Nikita Sirgienko]

sirgienko updated this revision to Diff 50807.
sirgienko added a comment.


  Add Maxima function

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18714?vs=50805&id=50807

BRANCH
  master

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

AFFECTED FILES
  data/syntax/maxima.xml

To: sirgienko, #framework_syntax_highlighting
Cc: kwrite-devel, kde-frameworks-devel, bmortimer, hase, michaelh, genethomas, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D18714: Add some Maxima constans and function

[Nikita Sirgienko]

sirgienko retitled this revision from "Add some Maxima constans to 
MaximaVariable list" to "Add some Maxima constans and function".

REPOSITORY
  R216 Syntax Highlighting

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

To: sirgienko, #framework_syntax_highlighting
Cc: kwrite-devel, kde-frameworks-devel, bmortimer, hase, michaelh, genethomas, 
ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D18714: Add some Maxima constans and function

[Dominik Haumann]

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


  Looks good to me, the only missing part is to increase thw "version" number 
each time a highlighting file is changed.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

To: sirgienko, #framework_syntax_highlighting, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, bmortimer, hase, michaelh, 
genethomas, ngraham, bruns, demsking, cullmann, vkrause, sars

D18714: Add some Maxima constans and function

[Nikita Sirgienko]

sirgienko updated this revision to Diff 50809.
sirgienko added a comment.


  Increase version

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18714?vs=50807&id=50809

BRANCH
  master

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

AFFECTED FILES
  data/syntax/maxima.xml

To: sirgienko, #framework_syntax_highlighting, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, bmortimer, hase, michaelh, 
genethomas, ngraham, bruns, demsking, cullmann, vkrause, sars

D18714: Add some Maxima constans and function

[Nikita Sirgienko]

sirgienko added a comment.


  Should I land it via arc? Or mannualy commit it (for short commit message)?

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  master

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

To: sirgienko, #framework_syntax_highlighting, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, bmortimer, hase, michaelh, 
genethomas, ngraham, bruns, demsking, cullmann, vkrause, sars

D18664: Baloo engine: treat every non-success code as a failure

[Valeriy Malov]

valeriymalov planned changes to this revision.
valeriymalov added a comment.


  In D18664#404173 , @poboiko wrote:
  
  > 1. Are we actually sure this is gonna fix all those crashes? Otherwise I 
would suggest using CCBUG instead of BUG inside the commit message.
  
  
  I've chosen those bugs based on backtraces I could get myself but without 
core dumps I can't vouch for that, even with core dumps it'd be hard to tell. 
Alternatively I can CCBUG and after this gets into a release, ask in those bugs 
people if they still experience the crash and close them if they won't get back 
in a month or so.
  
  > 1. For now, logging is the only way to know if there's something bad going 
on. In that case I would suggest to at least increase severity of those 
messages - it would increase chances users will notice it. For example, use 
`qCCritical` (but this would also require additional check for 
okayish/non-critical return codes, such as `MDB_NOTFOUND`)
  
  These errors have good potential to flood users log files. I get around 30 
megabytes of those on each boot. I'd rather see them disabled by default.
  It might make sense to add a catch-all error message somewhere on higher 
level, that would get printed out once, but I haven't looked yet where such 
message could be placed.
  
  > 1. Looks like this patch is composed of two parts - introducing new logging 
category and and improving error handling. It would be also nice to split those 
into two separate patches.
  > 2. There are a lot of redundant `Q_ASSERT_X` left, which could be removed. 
I suggest just `grep`'ing over the code to catch'em all. I've started marking 
them here, but then I gave up - too many of them.
  > 3. There are also several unchecked return codes as well, such as inside 
`*DB::size()` calls. Those can also be found by `grep`'ing over `Q_ASSERT_X`.
  
  I'll look into these this week.

REPOSITORY
  R293 Baloo

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

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

D17816: Support for xattrs on kio copy/move

[David Faure]

dfaure added a comment.


  Using exec() from a job implementation is a big no no, it creates an 
unexpected re-entrancy, source of all sorts of problems. This cannot be shipped 
as is.
  
  See KCompositeJob's documentation for how to add a subjob - and/or see 
DirectorySizeJob for a simple example.

REPOSITORY
  R241 KIO

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

To: cochise, dfaure
Cc: cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, 
ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, michaelh

D16894: [ECM] use a macro to add compiler flags conditionally

[René J . V . Bertin]

rjvbb set the repository for this revision to R240 Extra CMake Modules.

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system, kfunk
Cc: cgiboudeaux, dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, 
#build_system, michaelh, ngraham, bruns

D16894: [ECM] use a macro to add compiler flags conditionally

[René J . V . Bertin]

rjvbb updated this revision to Diff 50812.
rjvbb added a comment.


  Now tested more exhaustively and with unittest.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16894?vs=50589&id=50812

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

AFFECTED FILES
  docs/module/ECMAddCompilerFlag.rst
  kde-modules/KDECompilerSettings.cmake
  kde-modules/KDEFrameworkCompilerSettings.cmake
  modules/ECMAddCompilerFlag.cmake
  tests/CMakeLists.txt
  tests/ECMAddCompilerFlag/CMakeLists.txt
  tests/ECMAddCompilerFlag/main.c

To: rjvbb, #build_system, kfunk
Cc: cgiboudeaux, dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, 
#build_system, michaelh, ngraham, bruns

D18702: Remove monochrome font preferences icon and improve font selection icon

[Nathaniel Graham]

ngraham added a comment.


  In D18702#404521 , @ndavis wrote:
  
  > Is this icon used by any other KDE software or 3rd party applications?
  
  
  Yes, quite a few: 
https://lxr.kde.org/search?_filestring=&_string=preferences-desktop-font
  
  Those apps won't experience any breakage, but they will suddenly display the 
scaled-down color version.

REPOSITORY
  R266 Breeze Icons

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

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

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

[Nathaniel Graham]

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


  This makes sense to me. Thanks for the test, too.

REPOSITORY
  R293 Baloo

BRANCH
  add-watch-moved (branched from master)

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

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

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

[Stefan Brüns]

bruns requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R293 Baloo

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

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

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

[Stefan Brüns]

bruns added a comment.


  To which "file1"/"file2" are you referring in the summary? The commit log 
should be self contained.

INLINE COMMENTS

> kinotify.cpp:431
> +}
> +addWatch(QFile::decodeName(path), d->mode, d->flags);
> +} else {

`QFile:decodeName(path)` called twice for the same path

> kinotify.cpp:431
> +}
> +addWatch(QFile::decodeName(path), d->mode, d->flags);
> +} else {

This also traverses the directory tree twice, once with all items (here), and 
once for  directories (in KInotify::addWatch(...)).
Calling `if (it.fileInfo().isDir()) { d->addWatch(it.filePath()) }` in the loop 
should have the same effect.

REPOSITORY
  R293 Baloo

BRANCH
  add-watch-moved (branched from master)

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

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

D18664: Baloo engine: treat every non-success code as a failure

[Stefan Brüns]

bruns added a comment.


  In general, these changes seem worthwhile.
  But as already said, splitting these into two parts - QDebug etc vs. Error 
handling - is a must.

REPOSITORY
  R293 Baloo

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

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

D18664: Baloo engine: treat every non-success code as a failure

[Christoph Cullmann]

cullmann added a comment.


  I think this is a step into the right direction.
  Thought I am still not sure if we not should go away from a own storage db 
for all this and port that over to e.g. use the tracker stuff.
  Given that fixing the issues will be more or less a complete rewrite, too.
  (I have some partly working port in my aged 
https://cgit.kde.org/clones/baloo/cullmann/tbaloo.git/)

REPOSITORY
  R293 Baloo

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

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

D18702: Remove monochrome font preferences icon and improve font selection icon

[Björn Feber]

GB_2 added a comment.


  In D18702#404695 , @ngraham wrote:
  
  > In D18702#404521 , @ndavis wrote:
  >
  > > Is this icon used by any other KDE software or 3rd party applications?
  >
  >
  > Yes, quite a few: 
https://lxr.kde.org/search?_filestring=&_string=preferences-desktop-font
  >
  > Those apps won't experience any breakage, but they will suddenly display 
the scaled-down color version.
  
  
  In most places it is used correctly as a SySe category icon and I think it 
would be fine if it would turn into a colorful one.
  It needs to be changes here though (the "All Fonts" list item):
  F6585852: IMG_20190204_073023_364.jpg 
  We could use `gnumeric-font` as an alternative.

REPOSITORY
  R266 Breeze Icons

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

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