D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

[Christoph Cullmann]

cullmann added a comment.


  :=) You are right, without that, it might be even faster as we only do it 
once.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, vkrause, cullmann, kossebau
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

[Dominik Haumann]

dhaumann added a comment.


  @cullmann I don't see how we can optimize unneed `hasXXX` calls away: 
`Format::isDefaultTextStyle(const Theme &theme)` internally uses the same 
'hasXXX' calls, i.e. if we use both, we do it twice. Right now I don't see a 
trivial way to improve the patch.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, vkrause, cullmann, kossebau
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21298: Add example app for printing highlighted text to pdf

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> main.cpp:36
> +parser.addPositionalArgument(QStringLiteral("source"), 
> QStringLiteral("The source file to print."));
> +parser.addPositionalArgument(QStringLiteral("pdf"), QStringLiteral("The 
> PDF file to prin to."));
> +parser.process(app);

typo, "prin"

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  addpdfprintingexample

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

To: kossebau, vkrause, dhaumann
Cc: pino, dhaumann, kwrite-devel, kde-frameworks-devel, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D21298: Add example app for printing highlighted text to pdf

[Dominik Haumann]

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


  I ngeneral looks good to me. Maybe use ecm_optional_add_subdirectory - this 
also allows 3rd party users to omit the code and it still works.

INLINE COMMENTS

> CMakeLists.txt:6
> +
> +add_subdirectory(codepdfprinter)

Maybe we use the optional variant like:

  ecm_optional_add_subdirectory (codepdfprinter)

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  addpdfprintingexample

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

To: kossebau, vkrause, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, 
bruns, demsking, cullmann, sars

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.12 - Build # 108 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.12/108/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Mon, 20 May 2019 03:31:02 +
 Build duration:
1 hr 3 min and counting
   JUnit Tests
  Name: projectroot Failed: 4 test(s), Passed: 48 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_dropjobtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.src.ioslaves.trash.tests.testtrashName: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

[Friedrich W. H. Kossebau]

kossebau added a comment.


  I can confirm that this fixes the thumbnail bug for me: when I change the 
color theme, get rid of current thumbnail process (`killall thumbnail.so`) and 
trigger a reload of text thumbnails in Dolphin, I now always get consistently 
same colors with enough contrast used.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, vkrause, cullmann, kossebau
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21298: Add example app for printing highlighted text to pdf

[Friedrich W. H. Kossebau]

kossebau created this revision.
kossebau added a reviewer: vkrause.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Gives an example how to use KSyntaxHighlighting when simply wanting to process
  a QTextDocument into rich text and output that.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  addpdfprintingexample

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

AFFECTED FILES
  CMakeLists.txt
  examples/CMakeLists.txt
  examples/codepdfprinter/CMakeLists.txt
  examples/codepdfprinter/codepdfprinter.cpp
  examples/codepdfprinter/codepdfprinter.h
  examples/codepdfprinter/main.cpp

To: kossebau, vkrause
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

[Dominik Haumann]

dhaumann added a comment.


  Well, this change only affects the SyntaxHighlighter, which KTextEditor does 
not use ;)
  
  For completeness: 
https://code.woboq.org/qt5/qtbase/src/gui/text/qtextdocument.cpp.html#673
  Here, we can see that QTextDocument::drawContents() internally uses a 
QAbstractTextDocumentLayout::PaintContext ctx, which itself provides a 
default-constructed QPalette. A defaut-constructed QPalette equals the 
application palette. So the only other way for the text thumbnailer is to 
temporarily set the QApplication palette.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, vkrause, cullmann, kossebau
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21249: Test current filter before setting a new one

[Jan-Marek Glogowski]

jglogowski added a comment.


  In D21249#467037 , @dfaure wrote:
  
  > A unittest addition would be good, too.
  
  
  Done
  
  In D21249#467073 , @dfaure wrote:
  
  > Yes, naming is hard because the method is dual-purposed ;-)
  >
  > If the method only tried to match (but not to set), then the naming would 
be much simpler.
  
  
  …
  Done
  
  > 
  > 
  >> It "felt" strange, but I don't care much.
  > 
  > I do care, because others will try to understand and possibly modify this 
code later, so it should not "feel strange".
  
  I should have written. "Fine with me.". No offense intended.

INLINE COMMENTS

> dfaure wrote in kfilewidget.cpp:2495
> You wrote "done", but it's still there.

Should have been - will do.

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21249: Test current filter before setting a new one

[Jan-Marek Glogowski]

jglogowski updated this revision to Diff 58317.
jglogowski added a comment.


  Readd dropped lines from tests/CMakeLists.txt

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21249?vs=58316&id=58317

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  tests/CMakeLists.txt
  tests/kfilewidgettest_filter.cpp

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21249: Test current filter before setting a new one

[Jan-Marek Glogowski]

jglogowski updated this revision to Diff 58316.
jglogowski added a comment.


  - Return a QString from findMatchingFilter and handle setCurrentFilter in 
main function
  - Add filter unit test
  
  For a manual test I added a "Raw (*)" filter to the bug test program.
  After playing with it, I decided to not test the currentFilter() against 
QLatin1String("*"), so if a user selects such an entry, it'll disable 
auto-filter and -extension selection.
  Since we never auto-select the "*" filter, this honors the users filter 
selection.
  
  Took me a while to write the unit test without QDialog. I was a bit tricked 
by Q_ASSERT used in other tests ;-)

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21249?vs=58291&id=58316

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  tests/CMakeLists.txt
  tests/kfilewidgettest_filter.cpp

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

[Christoph Cullmann]

cullmann added a comment.


  I think one reason for the early out was that actually even the "isXXX" 
checks are very expensive, at least if I remember correctly they did show up a 
lot in my profiling in the past.
  Perhaps one should profile this once more (and if still visible) at least 
skip all the isXXX checks for the isDefault... case.
  Otherwise I have no issues with this.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, vkrause, cullmann, kossebau
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

[Dominik Haumann]

dhaumann added a comment.


  The text thumbnailer was added in: D19432 
.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, vkrause, cullmann, kossebau
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21293: Markdown: Use color with higher contrast for lists

[gregormi]

gregormi added a comment.


  In D21293#467150 , @dhaumann wrote:
  
  > New color is red: F6835074: Screenshot_20190519_175848.png 

  >
  > @gregormi If you want other colors, please propose them yourself :-)
  
  
  Thanks. Let's see how it works out.
  
  Regarding the screenshot, I see that "code 1, code2" is also the same orange 
as list items previously were. I think it would be better to change that as 
well, e.g. to blue as in "block quote" but a bit darker.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, gregormi
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21295: SyntaxHighlighter: Fix foreground color for text without special highlighting

[Dominik Haumann]

dhaumann created this revision.
dhaumann added reviewers: vkrause, cullmann, kossebau.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
dhaumann requested review of this revision.

REVISION SUMMARY
  QTextDocument seems to use the widget palette (or application
  palette) when drawing text. That is, by default the palette's
  foreground color is used when no special QTextCharFormat is
  specified.
  
  This patch changes applyFormat() such that the foreground color
  is always set to avoid the fallback to the QPalette's foreground
  color.
  
  BUG: 406821
  BUG: 406816
  FIXED-IN: 5.59

TEST PLAN
  make && make test + manual testing with the codeeditor example

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-syntax-highlighter-default-color

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

AFFECTED FILES
  examples/codeeditor.cpp
  src/lib/syntaxhighlighter.cpp

To: dhaumann, vkrause, cullmann, kossebau
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D20626: Refactor and cleanup

[Shubham]

shubham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

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

KDE CI: Frameworks » kwidgetsaddons » kf5-qt5 SUSEQt5.12 - Build # 32 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kwidgetsaddons/job/kf5-qt5%20SUSEQt5.12/32/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Sun, 19 May 2019 16:13:32 +
 Build duration:
9 min 8 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5WidgetsAddons-5.59.0.xmlcompat_reports/KF5WidgetsAddons_compat_report.htmllogs/KF5WidgetsAddons/5.59.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: 0 test(s), Passed: 19 test(s), Skipped: 0 test(s), Total: 19 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(2/4)45%
(75/168)45%
(75/168)35%
(5230/15090)26%
(2015/7825)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(20/20)100%
(20/20)99%
(1672/1681)50%
(801/1610)examples.kmessagebox0%
(0/1)0%
(0/1)0%
(0/17)0%
(0/2)src48%
(55/114)48%
(55/114)29%
(3558/12286)20%
(1214/6058)tests0%
(0/33)0%
(0/33)0%
(0/1106)0%
(0/155)

D21293: Markdown: Use color with higher contrast for lists

[Dominik Haumann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:7be1380c52a8: Markdown: Use color with higher contrast 
for lists (authored by dhaumann).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21293?vs=58308&id=58309

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

AFFECTED FILES
  autotests/html/basic.markdown.html
  autotests/html/test.markdown.html
  data/syntax/markdown.xml

To: dhaumann, cullmann, gregormi
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21293: Markdown: Use color with higher contrast for lists

[Christoph Cullmann]

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


  Looks ok for me.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  improve-markdown-colors

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

To: dhaumann, cullmann, gregormi
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21293: Markdown: Use color with higher contrast for lists

[Dominik Haumann]

dhaumann added a comment.


  New color is red: F6835074: Screenshot_20190519_175848.png 

  
  @gregormi If you want other colors, please propose them yourself :-)

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, gregormi
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21293: Markdown: Use color with higher contrast for lists

[Dominik Haumann]

dhaumann created this revision.
dhaumann added reviewers: cullmann, gregormi.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
dhaumann requested review of this revision.

REVISION SUMMARY
  BUG: 405824
  FIXED-IN: 5.59

TEST PLAN
  make && make test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  improve-markdown-colors

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

AFFECTED FILES
  autotests/html/basic.markdown.html
  autotests/html/test.markdown.html
  data/syntax/markdown.xml

To: dhaumann, cullmann, gregormi
Cc: kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D21287: Try to find always an unique visible document name without a number suffix

[loh tar]

loh.tar edited the test plan for this revision.

REPOSITORY
  R39 KTextEditor

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

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

D21287: Try to find always an unique visible document name without a number suffix

[loh tar]

loh.tar added a comment.


  
  
  > PS: The tab switcher also has something similar to make a distiction 
between files with the same name D16054 
  
  I noticed this patch. It's more a workaround for the core issue.
  
  > besides that the quick open already shows the path to distinguish the files
  
  Like D16054 , but without harm. Not so 
bad to see the full path there
  
  > Looking at the screenshots, I find it rather intransparent which document 
gets what name,
  
  Yes it is. Better solutions are welcome.
  My examples, or better test files, are of course the worst thinkable case and 
not well suited to demonstrate the (hoped) benefit of this patch.
  
  > is this really an improvement?
  
  It's the right place to fix the issue. You have then every where the unique 
names without extra quirks. Consider e.g. confirmation dialogues.
  
  > And: I have the feeling this patch should pay attention to performance. 
Sometimes, users have hundreds of files open, and this should not slow down any 
open operation.
  
  Well, speedups are always nice. On session restore is it really often called 
and with my test setup the names often renamed. But should that really cause a 
noticable slowdown? I have doubts.
  
  Perhaps has session restore potential for improvements, and the whole naming 
process too. The function is called for each file surprising often just to set 
the first true name. "Emtpty/NoName"->"Untitled"->"FileName"

REPOSITORY
  R39 KTextEditor

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

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

D14464: Cancel privilege operation for read-only target with the current user as owner

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> ngraham wrote in global.h:248
> 5.59 now

no, not that one, it was already there ;)
The line was only modified to add a comma.

> ngraham wrote in global.h:250
> 5.59 now

and a trailing comma (as allowed by C++11) so that it doesn't need to be 
modified next time :-)

REPOSITORY
  R241 KIO

BRANCH
  master

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

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

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.12 - Build # 107 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.12/107/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Sun, 19 May 2019 15:02:16 +
 Build duration:
9 min 3 sec and counting
   JUnit Tests
  Name: projectroot Failed: 5 test(s), Passed: 47 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_jobtestFailed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_dropjobtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.src.ioslaves.trash.tests.testtrashName: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

KDE CI: Frameworks » kwidgetsaddons » kf5-qt5 SUSEQt5.12 - Build # 31 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kwidgetsaddons/job/kf5-qt5%20SUSEQt5.12/31/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Sun, 19 May 2019 14:33:01 +
 Build duration:
4 min 44 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5WidgetsAddons-5.59.0.xmlcompat_reports/KF5WidgetsAddons_compat_report.htmllogs/KF5WidgetsAddons/5.59.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: 18 test(s), Skipped: 0 test(s), Total: 19 test(s)Failed: projectroot.autotests.ksqueezedtextlabelautotest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(2/4)43%
(73/168)43%
(73/168)33%
(5009/15076)25%
(1930/7825)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests95%
(19/20)95%
(19/20)92%
(1538/1677)46%
(739/1610)examples.kmessagebox0%
(0/1)0%
(0/1)0%
(0/17)0%
(0/2)src47%
(54/114)47%
(54/114)28%
(3471/12276)20%
(1191/6058)tests0%
(0/33)0%
(0/33)0%
(0/1106)0%
(0/155)

KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.10 - Build # 117 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5.10/117/
 Project:
kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 19 May 2019 13:41:41 +
 Build duration:
18 min and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5TextEditor-5.59.0.xmlcompat_reports/KF5TextEditor_compat_report.htmllogs/KF5TextEditor/5.59.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: 61 test(s), Skipped: 0 test(s), Total: 62 test(s)Failed: projectroot.autotests.inlinenote_testName: projectroot.autotests.src Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report79%
(22/28)88%
(260/295)88%
(260/295)68%
(32998/48712)50%
(15611/31019)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.src100%
(38/38)100%
(38/38)93%
(4660/5000)49%
(1753/3606)autotests.src.vimode100%
(9/9)100%
(9/9)99%
(5472/5513)58%
(993/1710)src.buffer88%
(15/17)88%
(15/17)90%
(1708/1900)76%
(1214/1602)src.completion100%
(11/11)100%
(11/11)57%
(1781/3125)43%
(1164/2735)src.completion.expandingtree100%
(3/3)100%
(3/3)40%
(184/459)22%
(76/348)src.dialogs0%
(0/4)0%
(0/4)0%
(0/815)0%
(0/192)src.document100%
(4/4)100%
(4/4)60%
(1925/3196)49%
(1583/3243)src.export0%
(0/4)0%
(0/4)0%
(0/119)0%
(0/162)src.include.ktexteditor93%
(14/15)93%
(14/15)91%
(215/237)65%
(145/224)src.inputmode100%
(8/8)100%
(8/8)59%
(178/301)44%
(34/77)src.mode88%
(7/8)88%
(7/8)35%
(285/806)19%
(129/695)src.part0%
(0/1)0%
(0/1)0%
(0/7)100%
(0/0)src.printing0%
(0/4)0%
(0/4)0%
(0/834)0%
(0/294)src.render100%
(7/7)100%
(7/7)77%
(958/1246)67%
(631/944)src.schema29%
(2/7)29%
(2/7)1%
(20/1492)1%
(6/673)src.script100%
(16/16)100%
(16/16)68%
(731/1069)58%
(268/465)src.search100%
   

D21283: Replace Simple View and Tree view by an icon view and compact view

[Méven Car]

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


  Clean up reference to action decoration menu

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21283?vs=58280&id=58303

BRANCH
  icon-compact-views

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/filewidgets/kdiroperatordetailview.cpp
  src/filewidgets/kfilewidget.cpp
  src/widgets/kfile.cpp
  src/widgets/kfile.h

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

D21287: Try to find always an unique visible document name without a number suffix

[Dominik Haumann]

dhaumann added a comment.


  Looking at the screenshots, I find it rather intransparent which document 
gets what name, besides that the quick open already shows the path to 
distinguish the files -> is this really an improvement?
  
  And: I have the feeling this patch should pay attention to performance. 
Sometimes, users have hundreds of files open, and this should not slow down any 
open operation.
  
  Since I am not yet convinced by this path: -1 right now...
  
  Any comments? :-)
  
  PS: The tab switcher also has something similar to make a distiction between 
files with the same name D16054 

INLINE COMMENTS

> katedocument.cpp:4229
> +
> +QString fullUrl = removeNewLines(url.url());
> +

missing const.

> katedocument.cpp:4235
> +
> +QStringList splittedUrl = fullUrl.split(QLatin1Char('/'), 
> QString::SkipEmptyParts);
> +

Could you use QString::splitRef() to avoid not needed QString allocations? You 
will get a QVector instead of a QStringList.

> katedocument.cpp:4240
> +// ...to avoid odd naming in rare cases we force numbering fallback
> +if (splittedUrl.at(0) == QLatin1String("file:")) {
> +splittedUrl.removeAt(0);

I wonder how this behaves on Windows :-)

> katedocument.cpp:4242
> +splittedUrl.removeAt(0);
> +if (splittedUrl.at(0) == QLatin1String("home")) {
> +splittedUrl.removeAt(0);

Same here: Windows?

> katedocument.cpp:4273
> +// Collect all files with same name...
> +typedef QPair FooPair;
> +QList fooList;

FooPair? ...

REPOSITORY
  R39 KTextEditor

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

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

D21289: Fix: Use default indentation mode for new files

[Dominik Haumann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:11fb4c4d5e10: Fix: Use default indentation mode for new 
files (authored by dhaumann).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21289?vs=58296&id=58300

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

AFFECTED FILES
  src/document/katedocument.cpp

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

D21287: Try to find always an unique visible document name without a number suffix

[Christoph Cullmann]

cullmann added a comment.


  I like the idea of having better short document names without numbers.
  The algo looks a bit like what the document list tries to do with the 
shortest paths differentiating two files.
  I think the documents stuff ignores the changed doc names as it looks for 
file name changes, but that might be wrong.
  
  For the code: I would prefer that one uses e.g. just a call to an algorithm 
for reversing: https://en.cppreference.com/w/cpp/algorithm/reverse
  For the data structures, please use some vector for the pairs, the QList will 
heap allocate all stuff larger than a pointer.

REPOSITORY
  R39 KTextEditor

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

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

D19608: DocumentPrivate: Make bracket handling smart

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:df555c9b2e57: honor auto-bracket setting for balance check 
(authored by cullmann).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D19608?vs=56105&id=58298#toc

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19608?vs=56105&id=58298

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

AFFECTED FILES
  src/document/katedocument.cpp

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

D21289: Fix: Use default indentation mode for new files

[Christoph Cullmann]

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


  > ok
  

REPOSITORY
  R39 KTextEditor

BRANCH
  fix-default-indent-mode

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

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

D21249: Test current filter before setting a new one

[David Faure]

dfaure added a comment.


  Yes, naming is hard because the method is dual-purposed ;-)
  
  If the method only tried to match (but not to set), then the naming would be 
much simpler.
  
 QString findMatchingFilter() const;

if (!findMatchingFilter(...).isEmpty())
   return;

foreach() {
const QString filter = findMatchingFilter();
if (!filter.isEmpty()) {
 setCurrentFilter(filter);
return;
}
}
  
  > It "felt" strange, but I don't care much.
  
  I do care, because others will try to understand and possibly modify this 
code later, so it should not "feel strange".

INLINE COMMENTS

> dfaure wrote in kfilewidget.cpp:2495
> The added '|' isn't needed, is it?
> 
> str.left(-1) returns the whole string.

You wrote "done", but it's still there.

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21249: Test current filter before setting a new one

[Jan-Marek Glogowski]

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

INLINE COMMENTS

> elvisangelaccio wrote in kfilewidget.cpp:132
> Nitpick: I'd make `filter` the first parameter. And we usually don't use 
> `const boll` in signatures, but just `bool`.
> 
> `bUpdate` doesn't tell the reader what this variable is used for. How about 
> calling it `updateCurrentFilter` or similar?

Kind of Done.

> dfaure wrote in kfilewidget.cpp:2463
> Is the bUpdate bool necessary?
> 
> Without it, we'd call setCurrentFilter(currentFilter()) which should be a 
> no-op, right?
> 
> Alternatively, the method could return a QString, and the (second) caller 
> could call setCurrentFilter.
> 
> I just don't like a method that's sometimes a getter and sometimes a setter 
> (basically).

> Is the bUpdate bool necessary?



  void KFileFilterCombo::setCurrentFilter(const QString &filter)
  {
  setCurrentIndex(d->m_filters.indexOf(filter));
  emit filterChanged();
  }

filterChanged will unconditionally start the "cycle", which will set the wrong 
filter.
Wanted to keep my changes more local.

> Alternatively, the method could return a QString, and the (second) caller 
> could call setCurrentFilter.
>  I just don't like a method that's sometimes a getter and sometimes a setter 
> (basically).

Also had that. It "felt" strange, but I don't care much.

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21289: Fix: Use default indentation mode for new files

[Dominik Haumann]

dhaumann updated this revision to Diff 58296.
dhaumann added a comment.


  - Use updateConfig() to set indentation mode

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21289?vs=58294&id=58296

BRANCH
  fix-default-indent-mode

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

AFFECTED FILES
  src/document/katedocument.cpp

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

D21289: Fix: Use default indentation mode for new files

[Christoph Cullmann]

cullmann added a comment.


  Hmm, perhaps updateConfig would be better to not miss other stuff.

REPOSITORY
  R39 KTextEditor

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

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

D21288: Remove .conf extension from "INI Files" hl, to determine the highlighter using MIME type

[Nibaldo González]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:71446d3e9e5e: Remove .conf extension from "INI 
Files" hl, to determine the highlighter using… (authored by nibags).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21288?vs=58292&id=58295

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

AFFECTED FILES
  data/syntax/ini.xml

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

D21289: Fix: Use default indentation mode for new files

[Dominik Haumann]

dhaumann added a comment.


  Alternatively, we could call updateConfig() at the end of the KateDocument 
constructor which does - among other things - the same.

REPOSITORY
  R39 KTextEditor

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

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

D21289: Fix: Use default indentation mode for new files

[Dominik Haumann]

dhaumann created this revision.
dhaumann added a reviewer: cullmann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
dhaumann requested review of this revision.

REVISION SUMMARY
  BUG: 375502

TEST PLAN
  make && make test

REPOSITORY
  R39 KTextEditor

BRANCH
  fix-default-indent-mode

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

AFFECTED FILES
  src/document/katedocument.cpp

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

D21286: Perl: fix the // operator

[Nibaldo González]

nibags closed this revision.

REPOSITORY
  R216 Syntax Highlighting

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

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

D21288: Remove .conf extension from "INI Files" hl, to determine the highlighter using MIME type

[Christoph Cullmann]

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


  OK with that.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  remove-ext-conf-ini

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

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

D19608: DocumentPrivate: Make bracket handling smart

[Christoph Cullmann]

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


  I think moving the config check some lines up will do the trick.

REPOSITORY
  R39 KTextEditor

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

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

D21249: Test current filter before setting a new one

[Jan-Marek Glogowski]

jglogowski updated this revision to Diff 58291.
jglogowski added a comment.


  Changes:
  
  - Dropped the duplicate comment in matchFilter (not sure if it makes sense at 
all)
  - Replace bool param with enum class to improve readability - should have 
done this from the start
  - Drop const from enum as requested; I like useing const wherever possible 
setting preconditions…
  
  Remarks:
  
  - Wondering why there is still this foreach... maybe was just missed
  - Naming is hard and I couldn't come up with something I really liked 
(MatchPoliy vs MatchAction etc.)

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21249?vs=58215&id=58291

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21286: Perl: fix the // operator

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.


  Yes, please push, thanks.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-perl

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

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

D21249: Test current filter before setting a new one

[David Faure]

dfaure added a comment.


  A unittest addition would be good, too.

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21288: Remove .conf extension from "INI Files" hl, to determine the highlighter using MIME type

[Nibaldo González]

nibags created this revision.
nibags added reviewers: Framework: Syntax Highlighting, dhaumann, cullmann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.

REVISION SUMMARY
  BUG: 400290
  
  The extension `.conf` is very generic, since depending on the context, it can 
be used with a different language, and assigning it to the "INI Files" 
highlighter isn't very convenient.
  For example, in "Fontconfig" the XML files have a `.conf` extension.
  
  Therefore, I propose that the `.conf` extension isn't assigned to any syntax 
highlighter, so use the MIME type to determine the highlighting mode in those 
files.
  
  For example, when applying this patch, the Fontconfig files (located in 
`/etc/fonts/`) are highlighted as XML by default.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  remove-ext-conf-ini

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

AFFECTED FILES
  data/syntax/ini.xml

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

D21249: Test current filter before setting a new one

[David Faure]

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

INLINE COMMENTS

> kfilewidget.cpp:2463
> +if (rx.exactMatch(filename)) {
> +if (bUpdate && p != QLatin1String("*")) {   // never match the 
> catch-all filter
> +filterWidget->setCurrentFilter(filter);

Is the bUpdate bool necessary?

Without it, we'd call setCurrentFilter(currentFilter()) which should be a 
no-op, right?

Alternatively, the method could return a QString, and the (second) caller could 
call setCurrentFilter.

I just don't like a method that's sometimes a getter and sometimes a setter 
(basically).

> kfilewidget.cpp:2495
>  QString filename = 
> urlStr.mid(urlStr.lastIndexOf(QLatin1Char('/')) + 1); // only filename
> +if (matchFilter(filename, filterWidget->currentFilter() + 
> QLatin1Char('|'), false))
> +return;

The added '|' isn't needed, is it?

str.left(-1) returns the whole string.

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21280: kioslave: preserve argv[0], to fix applicationDirPath() on non-Linux

[David Faure]

dfaure updated this revision to Diff 58289.
dfaure added a comment.


  Use QVarLengthArray to fix the memory leak

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21280?vs=58268&id=58289

BRANCH
  2019_freebsd_fixed

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

AFFECTED FILES
  src/kioslave/kioslave.cpp

To: dfaure, sitter, davidedmundson, apol, adridg
Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D21287: Try to find always an unique visible document name without a number suffix

[loh tar]

loh.tar created this revision.
loh.tar added a reviewer: KTextEditor.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
loh.tar requested review of this revision.

REVISION SUMMARY
  When you e.g. work on multiple projects at the same time you have often the 
case that you have documents with a number suffix on tabs or other places like 
side bars. These suffix give no hint which file it really is.
  This patch add some folder name to the document to make the name unique, 
which should be in any case more helpful than the number suffix.
  
  Drawbacks
  
  - The unique names can change when you open a file
  - The numbering is kept as fallback for rare cases. When it is in use are 
these numbers also not "static" like in case of "Untitled" documents

TEST PLAN
  No Patch
  F6834935: no-patch.png 
  Patch
  F6834936: patch.png 
  
  Hint: The shown "Documents" plugin did not notice name changes, keep this in 
mind when you play around

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/document/katedocument.cpp
  src/document/katedocument.h

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

D21286: Perl: fix the // operator

[Dominik Haumann]

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


  Thanks for the fix, please commit.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-perl

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

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

D21286: Perl: fix the // operator

[Nibaldo González]

nibags created this revision.
nibags added reviewers: Framework: Syntax Highlighting, dhaumann, cullmann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.

REVISION SUMMARY
  BUG: 407327
  
  The "/" character is used for patterns and this causes the "//" operator not 
to be highlighted correctly in some situations, such as after identifiers.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-perl

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

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

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

D21283: Replace Simple View and Tree view by an icon view and compact view

[Méven Car]

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


  Rename IconView kind Simple to Icon

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21283?vs=58279&id=58280

BRANCH
  icon-compact-views

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/filewidgets/kdiroperatordetailview.cpp
  src/widgets/kfile.cpp
  src/widgets/kfile.h

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

D21249: Test current filter before setting a new one

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kfilewidget.cpp:132
>  void updateLocationEditExtension(const QString &);
> +bool matchFilter(const QString &filename, const QString &filter, const 
> bool bUpdate);
>  void updateFilter();

Nitpick: I'd make `filter` the first parameter. And we usually don't use `const 
boll` in signatures, but just `bool`.

`bUpdate` doesn't tell the reader what this variable is used for. How about 
calling it `updateCurrentFilter` or similar?

REPOSITORY
  R241 KIO

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

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

D21249: Test current filter before setting a new one

[Elvis Angelaccio]

elvisangelaccio added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, 
bruns

D21283: Replace Simple View and Tree view by an icon view and compact view

[Méven Car]

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


  Capitalization

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21283?vs=58277&id=58279

BRANCH
  icon-compact-views

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/filewidgets/kdiroperatordetailview.cpp
  src/widgets/kfile.cpp
  src/widgets/kfile.h

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

D21283: Replace Simple View and Tree view by an icon view and compact view

[Méven Car]

meven edited the summary of this revision.
meven edited the test plan for this revision.
meven added a reviewer: ngraham.

REPOSITORY
  R241 KIO

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

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

D21283: Replace Simple View and Tree view by an icon view and compact view

[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
  Remove the simple view with the decoration position option, splitting the 
feature between an IconView and a Compact View (replacing the Tree view)
  
  A step towards "Make the view modes match the ones in Dolphin (e.g. Icons, 
Compact, and Details, with an option for "Allow expansion" in the settings 
menu" of
  T8552 

REPOSITORY
  R241 KIO

BRANCH
  icon-compact-views

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/filewidgets/kdiroperatordetailview.cpp
  src/widgets/kfile.cpp
  src/widgets/kfile.h

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