D14274: Modelines: add byte-order-mark & small fixes

[Dominik Haumann]

dhaumann added a comment.


  Sidenote: Looking into the code in KTextEditor, there is:
  
 } else if (var == QLatin1String("bom") || var == 
QLatin1String("byte-order-mark") || var == QLatin1String("byte-order-marker")) {
if (checkBoolValue(val, &state)) {
m_config->setBom(state);
}
}
  
  So byte-order-mark is also supported, the longer version byte-order-marker is 
just there for backwards compatibility.

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, turbov, dhaumann, cullmann, vkrause, #kate
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.
Restricted Application added a project: Kate.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Beside the short arc confusion, the current state can go in, or?

REPOSITORY
  R216 Syntax Highlighting

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

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

D14274: Modelines: add byte-order-mark & small fixes

[Christoph Cullmann]

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


   is only a performance optimization, as it just skips over 
whitespaces before any of the costly rules are tried to match. I think that 
should just be added again.
  Otherwise, I have no issues with this changes.
  What was exactly the problem with the unit test? Having one would be nice.

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, turbov, dhaumann, cullmann, vkrause, #kate
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D14526: AppArmor: update syntax and various improvements/fixes

[Nibaldo González]

nibags closed this revision.

REPOSITORY
  R216 Syntax Highlighting

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

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

D14526: AppArmor: update syntax and various improvements/fixes

[Nibaldo González]

nibags updated this revision to Diff 39550.
nibags added a comment.


  Fix conflicts in references.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14526?vs=39549&id=39550

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

AFFECTED FILES
  autotests/folding/usr.bin.apparmor-profile-test.fold
  autotests/html/usr.bin.apparmor-profile-test.html
  autotests/input/usr.bin.apparmor-profile-test
  autotests/reference/usr.bin.apparmor-profile-test.ref
  data/syntax/apparmor.xml

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

D13869: [solid] Notify when interface to mounted fs is lost

[Anthony Fieroni]

anthonyfieroni abandoned this revision.
anthonyfieroni added a comment.


  https://phabricator.kde.org/D14661

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D14661: Force reevaluation of Predicates if interfaces are removed

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ok, +1 then.

REPOSITORY
  R245 Solid

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

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

D14526: AppArmor: update syntax and various improvements/fixes

[Nibaldo González]

nibags updated this revision to Diff 39549.
nibags added a comment.


  - Update references.
  
  Regenerate file "autotest/reference/usr.bin.apparmor-profile-test.ref"

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14526?vs=38880&id=39549

BRANCH
  arcpatch-D14526

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

AFFECTED FILES
  autotests/folding/usr.bin.apparmor-profile-test.fold
  autotests/html/usr.bin.apparmor-profile-test.html
  autotests/input/usr.bin.apparmor-profile-test
  autotests/reference/usr.bin.apparmor-profile-test.ref
  data/syntax/apparmor.xml

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

D14702: Java: add binaries & hex-float, and support underscores in numbers

[Nibaldo González]

nibags closed this revision.

REPOSITORY
  R216 Syntax Highlighting

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

To: nibags, dhaumann, cullmann, vkrause, #kate
Cc: ngraham, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, bruns, 
demsking, cullmann, sars, dhaumann

KDE CI: Frameworks kcoreaddons kf5-qt5 SUSEQt5.9 - Build # 49 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20SUSEQt5.9/49/
 Project:
Frameworks kcoreaddons kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 12 Aug 2018 11:28:34 +
 Build duration:
3 min 13 sec and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 22 test(s), Skipped: 0 test(s), Total: 24 test(s)Failed: TestSuite.kdirwatch_qfswatch_unittestFailed: TestSuite.kdirwatch_stat_unittest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report91%
(10/11)85%
(69/81)85%
(69/81)75%
(6324/8422)43%
(10137/23413)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests93%
(26/28)93%
(26/28)97%
(2696/2772)49%
(5797/11895)src.desktoptojson100%
(2/2)100%
(2/2)78%
(88/113)38%
(122/322)src.lib50%
(1/2)50%
(1/2)62%
(352/572)26%
(243/924)src.lib.caching100%
(2/2)100%
(2/2)45%
(354/782)18%
(184/1038)src.lib.io82%
(9/11)82%
(9/11)68%
(854/1265)37%
(1049/2827)src.lib.jobs71%
(5/7)71%
(5/7)57%
(160/281)38%
(52/138)src.lib.plugin100%
(7/7)100%
(7/7)85%
(654/770)42%
(975/2315)src.lib.randomness100%
(2/2)100%
(2/2)70%
(67/96)58%
(44/76)src.lib.text63%
(5/8)63%
(5/8)47%
(368/778)44%
(845/1899)src.lib.util100%
(10/10)100%
(10/10)81%
(731/908)52%
(826/1581)tests0%
(0/2)0%
(0/2)0%
(0/85)0%
(0/398)

KDE CI: Frameworks kcoreaddons kf5-qt5 SUSEQt5.10 - Build # 104 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20SUSEQt5.10/104/
 Project:
Frameworks kcoreaddons kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 12 Aug 2018 11:28:34 +
 Build duration:
6 min 35 sec and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 22 test(s), Skipped: 0 test(s), Total: 24 test(s)Failed: TestSuite.kdirwatch_qfswatch_unittestFailed: TestSuite.kdirwatch_stat_unittest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report91%
(10/11)85%
(69/81)85%
(69/81)75%
(6323/8422)43%
(10137/23417)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests93%
(26/28)93%
(26/28)97%
(2696/2772)49%
(5796/11893)src.desktoptojson100%
(2/2)100%
(2/2)78%
(88/113)38%
(123/326)src.lib50%
(1/2)50%
(1/2)61%
(351/572)26%
(243/928)src.lib.caching100%
(2/2)100%
(2/2)45%
(354/782)18%
(184/1038)src.lib.io82%
(9/11)82%
(9/11)68%
(854/1265)37%
(1047/2821)src.lib.jobs71%
(5/7)71%
(5/7)57%
(160/281)38%
(52/138)src.lib.plugin100%
(7/7)100%
(7/7)85%
(654/770)42%
(975/2317)src.lib.randomness100%
(2/2)100%
(2/2)70%
(67/96)58%
(44/76)src.lib.text63%
(5/8)63%
(5/8)47%
(368/778)45%
(846/1899)src.lib.util100%
(10/10)100%
(10/10)81%
(731/908)52%
(827/1583)tests0%
(0/2)0%
(0/2)0%
(0/85)0%
(0/398)

D14593: Update Logtalk language syntax support

[Christoph Cullmann]

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


  Thanks for the contribution.
  I think the version=... should just be an integer, just increment it by one.
  Beside that, it would be very nice to have some test file in the autotests 
folder like for other languages for this with the new features you cover (at 
least some parts would already be nice).

REPOSITORY
  R216 Syntax Highlighting

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

To: pmoura, dhaumann, cullmann, vkrause, #framework_syntax_highlighting, #kate
Cc: kwrite-devel, kde-frameworks-devel, pmoura, michaelh, genethomas, 
kevinapavew, ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14757: Warn user before copy operation if available space is not enough

[David Faure]

dfaure added a comment.


  You can just emit the warning signal, from the job.

REPOSITORY
  R241 KIO

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

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

D14661: Force reevaluation of Predicates if interfaces are removed

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in udisksmanager.cpp:239
> Your  code seems to work, but i don't get it - why device is added again? How 
> predicate will work on removed device?

Adding it again allows the Predicate to determine if this device is still 
relevant.
If e.g. the Predicate is "give me all block devices, independent if it is 
formatted or not", then the device will be added, if the Predicate only allows 
formatted devices, the device will be filtered out.
The policy is left to the Predicate and not to the backend.

REPOSITORY
  R245 Solid

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

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

D4716: Add some more directives to MIPS assembler highlighting

[Dominik Haumann]

dhaumann added a comment.
Restricted Application added a project: Kate.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Could you provide a test snippet now? I will integrate this then.

REPOSITORY
  R216 Syntax Highlighting

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

To: arichardson, dhaumann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann, #frameworks

D14758: use view lines for wheel scrolling, not real lines

[Nathaniel Graham]

ngraham added a comment.


  Thanks for the patch, Sven!
  
  There are two issues with scrolling:
  
  1. Multi-line strings are inappropriately treated as a single line for the 
purposes of scrolling
  2. Lack of per-pixel scrolling that touchpad users expect.
  
  This patch fixes #1, but not #2. I've untangles the Bugzilla tickets such 
that https://bugs.kde.org/show_bug.cgi?id=256561 now only tracks #1, so I think 
you're free to add `BUG: 256561` to this patch. We will track pixel-by-pixel 
scrolling separately with https://bugs.kde.org/show_bug.cgi?id=378275.

REPOSITORY
  R39 KTextEditor

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

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

D7033: Port ki18n from QtScript to QtQml

[Allan Sandfeld Jensen]

carewolf added a comment.


  I solved the Ts.acall. Unfortunately I forgot how arc works, so it became a 
new entry https://phabricator.kde.org/D14769

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

To: carewolf, ilic
Cc: kde-frameworks-devel, huftis, ngraham, dhaumann, ltoscano, kfunk, michaelh, 
bruns

D14757: Warn user before copy operation if available space is not enough

[Shubham]

shubham edited reviewers, added: dfaure; removed: broulik, ngraham.

REPOSITORY
  R241 KIO

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

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

D14757: Warn user before copy operation if available space is not enough

[Pino Toscano]

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


  Hint: please make sure you build **and** test the next versions of your 
patches, otherwise it is just a waste of everybody's time.

INLINE COMMENTS

> copyjob.cpp:891
> +if (m_totalSize > m_freeSpace) {
> +SimpleJobPrivate *sjp;
> +int msgRes;

Uninitialized pointer, this will crash two lines later...
Also, this is the base class of the private class used for this job, and this 
function is part of that class already; so why aren't you just invoking it?

> copyjob.cpp:893
> +int msgRes;
> +msgRes = 
> sjp->requestMessageBox(JobUiDelegateExtension::WarningYesNo,
> +QString("Warning!"),

The order of the parameters does not match at all the actual parameters of the 
function.
Also, all the text/caption strings **must** be translated.

> copyjob.cpp:899
> + "Do you still want to 
> continue?",
> +  m_dest.path(),
> +  
> KIO::convertSize(m_totalSize),

Assuming `m_dest` is a local file, then `toLocalFile()` is the right function 
to call (`path()` will give a different result on Windows).

> copyjob.cpp:909
> +
> +if (msgRes == KMessageBox::Yes) {
> +goto yes;

The return value is `JobUiDelegateExtension::MessageBoxType`, not 
`KMessageBox::ButtonCode`.

REPOSITORY
  R241 KIO

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

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

D14762: Proposal: Add "None" highlighting to repository per default

[Christoph Cullmann]

cullmann updated this revision to Diff 39515.
cullmann added a comment.


  Fixes translation + code editor example.
  No special handling needed there (+ fix for section without name)

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14762?vs=39509&id=39515

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

AFFECTED FILES
  examples/codeeditor.cpp
  src/lib/definition_p.h
  src/lib/repository.cpp

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

D7033: Port ki18n from QtScript to QtQml

[Allan Sandfeld Jensen]

carewolf updated this revision to Diff 39526.
carewolf added a comment.


  Fixed acall using a pure javascript bridge

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7033?vs=39460&id=39526

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  src/CMakeLists.txt
  src/ktranscript.cpp

To: carewolf, ilic
Cc: kde-frameworks-devel, huftis, ngraham, dhaumann, ltoscano, kfunk, michaelh, 
bruns

D14763: Definition: Expose command markers

[Dominik Haumann]

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

REVISION SUMMARY
  Added functios include:
  
  - QString singleLineCommentMarker() const
  - CommentPosition singleLineCommentPosition() const
  - QPair multiLineCommentMarker() const
  
  Note: Some highlighting xml files define multiple singleLine and multiLine
  comments. Currently, the last one wins...

TEST PLAN
  make && make test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  expose-comment-markers (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h

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

D14639: Add syntax-highlighting for RDoc (R documentation)

[Christoph Cullmann]

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


  Looks ok, thanks for the contribution.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  rdoc

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

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

D14632: keyword rule: Spport for keywords inclusion from another language/file

[Christoph Cullmann]

cullmann added a comment.


  I would like a
  
   name##language  

  
  approach more, as then one could even nicely extend the included lists and as 
one is forced to have some list around it,
  they will show up in the kewordLists automatically.
  
  We make nice progress with integrating the syntax framework into ktexteditor.
  I think after akademy we can merge this in a finalzed form ;=)

REPOSITORY
  R216 Syntax Highlighting

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

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

D14758: use view lines for wheel scrolling, not real lines

[Dominik Haumann]

dhaumann added a subscriber: ngraham.
dhaumann added a comment.


  Not yet thoroughly reviewed, but some thoughts:
  
  - There is a bug report for this: https://bugs.kde.org/show_bug.cgi?id=256561 
Could you verify that the issues in the bug report are addressed by this patch? 
See also the comments by @ngraham in this bug report.
  - could you add: BUG: 256561
  - Since the commit logs now always appear in the KF5 changelog, maybe the 
subject could read: Fix: Scroll view lines instead of not real lines for wheel 
and touchpad scrolling

REPOSITORY
  R39 KTextEditor

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

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

D14743: Copy PlasmaCore binary qmldir file structure into bin for unit testsThis way tests pass before installation.

[David Edmundson]

davidedmundson marked 2 inline comments as done.

REPOSITORY
  R242 Plasma Framework (Library)

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

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

D14236: Add some improvements to kate-syntax-highlighter for use in scripting

[Dominik Haumann]

dhaumann added a reviewer: vkrause.

REPOSITORY
  R216 Syntax Highlighting

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

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

D14762: Proposal: Add "None" highlighting to repository per default

[Christoph Cullmann]

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

REVISION SUMMARY
  Rational: e.g. KTextEditor uses the repository to manage all syntax 
definitions.
  But to allow the user to use "None" as highlighting, we special case that 
everywhere and use there Definition().
  I think it would be much cleaner if the "None" definition is just part of the 
repository.
  An other approach would be to be able to add it later ourself.

TEST PLAN
  With this patch applied, I can just use definitions() everywhere in 
KTextEditor KateSyntaxManager without any special casing.

REPOSITORY
  R216 Syntax Highlighting

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

AFFECTED FILES
  src/lib/definition_p.h
  src/lib/repository.cpp

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

D14760: YAML: fix keys, add numbers and other improvements

[Christoph Cullmann]

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


  The screenshots look convincing!

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-yaml

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

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

D13940: Add syntax highlighting support for Stan

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.
Restricted Application added a project: Kate.
Restricted Application added a subscriber: kwrite-devel.


  Looks ok, should I push that for you as "Jeffrey Arnold 
"?

REPOSITORY
  R216 Syntax Highlighting

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

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

D14763: Definition: Expose command markers

[Dominik Haumann]

dhaumann added a comment.


  @vkrause We simply decided to commit this now. But a review from your side is 
still appreciated ;)

REPOSITORY
  R216 Syntax Highlighting

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

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

D14769: Port ki18n from QtScript to QtQml

[Allan Sandfeld Jensen]

carewolf created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
carewolf requested review of this revision.

REVISION SUMMARY
  Moves away from the deprecated QtScript module
  
  Change-Id: I02c6aa24bc74e64618dc046efb9bb5ee37740d5f

REPOSITORY
  R249 KI18n

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  src/CMakeLists.txt
  src/ktranscript.cpp

To: carewolf
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14702: Java: add binaries & hex-float, and support underscores in numbers

[Christoph Cullmann]

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


  Looks reasonable.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  java-improve-numbers

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

To: nibags, dhaumann, cullmann, vkrause, #kate
Cc: ngraham, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, bruns, 
demsking, cullmann, sars, dhaumann

D14760: YAML: fix keys, add numbers and other improvements

[Nibaldo González]

nibags closed this revision.

REPOSITORY
  R216 Syntax Highlighting

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

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

D14603: SCSS: update syntax. CSS: fix Operator and Selector Tag highlighting

[Christoph Cullmann]

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


  I would be for merging this and later replacing with the include list stuff.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  scss

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

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

D14757: Warn user before copy operation if available space is not enough

[Shubham]

shubham updated this revision to Diff 39531.
shubham added a comment.


  use simpleJobPrivate

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14757?vs=39492&id=39531

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/core/copyjob.cpp

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

D14757: Warn user before copy operation if available space is not enough

[Shubham]

shubham added a comment.


  @dfaure is it that message boxes cant be used in non gui library as suggested 
by pino. Is there any harm in using kmessagebox instead?

REPOSITORY
  R241 KIO

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

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

D14308: [Exe Thumbnailer] Improve icon selection algorithm

[Kai Uwe Broulik]

broulik updated this revision to Diff 39519.
broulik edited the summary of this revision.
broulik added a comment.


  I found a way to get the actual depth of the icon extracted
  
  - Prefer higher DPI as well

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14308?vs=39462&id=39519

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

AFFECTED FILES
  thumbnail/icoutils_common.cpp

To: broulik, #frameworks, dfaure, ngraham, pali, vonreth, antlarr
Cc: anthonyfieroni, bruns

D14773: completion widget: fix minimum header section size

[Dominik Haumann]

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

REPOSITORY
  R39 KTextEditor

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

To: brauch, #kate, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D14769: Port ki18n from QtScript to QtQml

[Allan Sandfeld Jensen]

carewolf added a reviewer: ilic.

REPOSITORY
  R249 KI18n

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

To: carewolf, ilic
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14526: AppArmor: update syntax and various improvements/fixes

[Christoph Cullmann]

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


  Has tests, looks ok. Thanks!

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  update-apparmor

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

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

D14763: Definition: Expose command markers

[Dominik Haumann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:7a8843f171e8: Definition: Expose command markers 
(authored by dhaumann).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14763?vs=39512&id=39545

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h

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

D14763: Definition: Expose command markers

[Christoph Cullmann]

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


  > I think that is ok
  

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  expose-comment-markers (branched from master)

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

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

D14762: Proposal: Add "None" highlighting to repository per default

[Christoph Cullmann]

cullmann updated this revision to Diff 39522.
cullmann added a comment.


  make test shall work again ;=)

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14762?vs=39515&id=39522

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  autotests/testhighlighter.cpp
  examples/codeeditor.cpp
  src/lib/definition_p.h
  src/lib/repository.cpp

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

D14762: Proposal: Add "None" highlighting to repository per default

[Christoph Cullmann]

cullmann added a comment.


  :=) Perhaps I should fix the unit tests, too.

REPOSITORY
  R216 Syntax Highlighting

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

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

D14772: Deprecate KFilterProxySearchLine

[Sune Vuorela]

svuorela created this revision.
svuorela added reviewers: kossebau, dfaure.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
svuorela requested review of this revision.

REVISION SUMMARY
  It is too primitive and simple for anything but the absolutely most basic 
things
  
  It is fixed to FixedStrings, search in all columns and caseinsensitive. And 
it seems to
  not really be fixeable. So just  mark it as deprecated and remove it at a 
later point.

TEST PLAN
  It has approximately 6 users across the kde codebase, let's fix them one by 
one.

REPOSITORY
  R276 KItemViews

BRANCH
  master

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

AFFECTED FILES
  src/kfilterproxysearchline.h

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

D14674: handle non-ASCII encodings of file names in tar archives

[Rinat Ibragimov]

ibragimovrinat marked 2 inline comments as done.

REPOSITORY
  R243 KArchive

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

To: ibragimovrinat, dfaure, kossebau
Cc: xyquadrat, broulik, cfeck, ibragimovrinat, kde-frameworks-devel, michaelh, 
ngraham, bruns

D7033: Port ki18n from QtScript to QtQml

[Allan Sandfeld Jensen]

carewolf added a comment.


  Note the latest patch still has a maximum number of arguments for acall. I 
checked all the usages, and the only place that uses acall with a variable set 
of arguments is the sr (Serbian) translation. Since supporting a variable 
number of arguments would require changes to QtDeclarative and thus mean we 
would need to support both models until KDE depends on Qt 5.13, perhaps it 
would be easier to just rewrite the serbian translations apply_to_word function?

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

To: carewolf, ilic
Cc: kde-frameworks-devel, huftis, ngraham, dhaumann, ltoscano, kfunk, michaelh, 
bruns

D14692: Use custom QScrollArea with size hint not limited by font size

[Valeriy Malov]

valeriymalov added a comment.


  I don't know, it's frameworks after all but I guess I can land it today since 
it's a popular bug. Should I just land it on master since there seem to be no 
other branches?

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

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

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Hey there, what's happening to this? I think this is a really nice feature 
and it would be very sad if it would bitrot :(
  Anything anyone can assist with? We are currently at Akademy, KDE's annual 
conference, so we would have time to discuss any issues right now.

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, dhaumann, cullmann, ngraham, brauch, 
michaelh, kevinapavew, bruns, demsking, sars, #frameworks

D14769: Port ki18n from QtScript to QtQml

[Allan Sandfeld Jensen]

carewolf abandoned this revision.

REPOSITORY
  R249 KI18n

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

To: carewolf, ilic
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14757: Warn user before copy operation if available space is not enough

[Shubham]

shubham added a comment.


  In D14757#306938 , @pino wrote:
  
  > Also, considering this is in a job in `KIOCore` (i.e. non-gui library), I 
suspect that using a message box directly is the wrong way to do it. @dfaure 
what is your opinion on that?

REPOSITORY
  R241 KIO

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

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

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

brauch added a comment.


  The "overly sensitive touchpad" issue seems to be missing accumulation of 
scroll events, so this patch to my understanding should not have it.

REPOSITORY
  R39 KTextEditor

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

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

D14707: autotests: skip '/' fstab check with zfs

[David Faure]

dfaure added a comment.


  The check is trying to verify that possibleMountPoints() works, by picking a 
mount point that we know has to exist, the root mount point. Anything else is 
hard to rely on, in a unittest ;)
  At least that was the logic in the old days, before all this magic that 
circumvents having entries in fstab...
  
  I don't think Live USB sticks are an issue (nobody is going to run unittests 
on them), but clearly containers are a valid use case which shouldn't break 
unittests.
  
  I'm not sure how to fix this without just removing any automated verification 
from this test method... :(

REPOSITORY
  R241 KIO

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

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

D14762: Proposal: Add "None" highlighting to repository per default

[Christoph Cullmann]

cullmann added a comment.


  Problem: How to get the "None" into the translation catalog?

REPOSITORY
  R216 Syntax Highlighting

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

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

D14760: YAML: fix keys, add numbers and other improvements

[Nibaldo González]

nibags edited the summary of this revision.
nibags added reviewers: dhaumann, cullmann, Framework: Syntax Highlighting.

REPOSITORY
  R216 Syntax Highlighting

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

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

D14674: handle non-ASCII encodings of file names in tar archives

[Rinat Ibragimov]

ibragimovrinat updated this revision to Diff 39537.
ibragimovrinat added a comment.


  Updated diff to use QString everywhere instead of QStringLiteral and 
QLatin1String. As those are in tests, performance penalty shouldn't matter. 
Also using QCOMPARE for comparisons.

REPOSITORY
  R243 KArchive

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14674?vs=39480&id=39537

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

AFFECTED FILES
  autotests/karchivetest.cpp
  autotests/karchivetest.h
  autotests/tar_non_ascii_file_name.tar.gz
  src/ktar.cpp

To: ibragimovrinat, dfaure, kossebau
Cc: xyquadrat, broulik, cfeck, ibragimovrinat, kde-frameworks-devel, michaelh, 
ngraham, bruns

D14236: Add some improvements to kate-syntax-highlighter for use in scripting

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> htmlhighlighter.h:42-43
>  void highlightFile(const QString &fileName);
> +void highlightFile(const QString &fileName, const QString &title);
> +void highlightFile(QFile &file, const QString &title);
>  

Instead of both the variants, what about a single one with QIODevice, e.g.:

  void highlightData(QIODevice *dev, const QString &title = QString());

this way you can highlight also an in-memory buffer, for example.

(For the same reason, a `setOutputDevice(QIODevice *dev)` would be nice too, 
but it's out of scope for this patch.)

REPOSITORY
  R216 Syntax Highlighting

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

To: xciml, #framework_syntax_highlighting, vkrause
Cc: pino, kde-frameworks-devel, kwrite-devel, #framework_syntax_highlighting, 
michaelh, genethomas, kevinapavew, ngraham, bruns, demsking, cullmann, vkrause, 
sars, dhaumann

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

brauch added a comment.


  I will modify the commit message as suggested.
  
  I read through the bug report, and I think this patch does fix the main issue 
discussed there. The patch suggested by one user there is even quite similar 
(if a bit less complete) than this one. The original complaint (touchpad vs. 
mouse wheel) to me personally sounds a bit strange and I'm not sure if that is 
actually the point. I'm not even sure if that is what the original reporter was 
in fact annoyed by (in contrast to "what he wrote he was annoyed by").

REPOSITORY
  R39 KTextEditor

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

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

D14763: Definition: Expose command markers

[Christoph Cullmann]

cullmann added a comment.


  Ok with that Volker?

REPOSITORY
  R216 Syntax Highlighting

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

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

D14773: completion widget: fix minimum header section size

[Sven Brauch]

brauch created this revision.
brauch added a reviewer: Kate.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
brauch requested review of this revision.

REVISION SUMMARY
  The header is hidden, but if the minimum size is -1 (the default), Qt
  chooses something non-zero as the minimum size. The completion widget
  then sets it to something, assumes it has that width, and messes up
  its layout when it actually ends up being larger than that.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/completion/expandingtree/expandingtree.cpp

To: brauch, #kate
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D14593: Update Logtalk language syntax support

[Nibaldo González]

nibags added reviewers: dhaumann, cullmann, vkrause, Framework: Syntax 
Highlighting, Kate.

REPOSITORY
  R216 Syntax Highlighting

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

To: pmoura, dhaumann, cullmann, vkrause, #framework_syntax_highlighting, #kate
Cc: kwrite-devel, kde-frameworks-devel, pmoura, michaelh, genethomas, 
kevinapavew, ngraham, bruns, demsking, cullmann, vkrause, sars, dhaumann

D14760: YAML: fix keys, add numbers and other improvements

[Nibaldo González]

nibags created this revision.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.

REVISION SUMMARY
  Fix some reported bugs and update the syntax for YAML 1.2:
  
  - Fixes the end of a Key:
  
  BUG: 366586
  
  Any line of text before `:` is highlighted as Key, which is very general and 
gives rise to errors. As mentioned in the bug, in a line like the following, 
everything is considered as a Key, until the last `:`
  {image1}
  
  To fix this, it's necessary to add a space/end-line to the end of a Key 
`KEY:(?=\s|$)`. 
  The YAML documentation mentions: *Mappings use a colon and space (“: ”) to 
mark each key: value pair* [1].
  
  - Fix delimiter characters in a Key:
  
  BUG: 389636
  
  On the next line, it is highlighted as a Key: `{b:`
  
  Within hashes and lists, the characters `{`, `[` and `,` should not be 
highlighted as Keys, since these are delimiting characters that mark the 
start/end of an element.
  
  - Allow inline attributes in lists: `[key1: value, key2: value, ...]`
  
  Previously, when writing a key, the entire line was highlighted as a value, 
now the value ends with "," or "]" (in the example, `3, 2, !!ddfgdf` is Key and 
`sdsd 1, takeoff]}` is value).
  
  For this, the context "attribute-pre-inline" is separated in the contexts 
"list-attribute-pre-inline" and "hash-attribute-pre-inline"; and 
"attribute-inline" is separated in "list-attribute-inline" and 
"hash-attribute-inline". The only difference is that one ends with "}" and 
another with "]".
  
  - Highlight values in Key with multiple lines. That is, when ":" is the first 
character of a line.
  
  - Highlight references, aliases, strings, etc. in hashes. Before, the hashes 
only support content of the type `key: value`. This improves highlighting in 
keys with many lines.
  
  - Add mapping-key operator: `?`. In the current highlight, references, 
aliases, sequences and dash after "?" are not highlighted, that is corrected. 
This also applies to lists and hashes.
  
  In Keys, the operator "?" now it is highlighted with the "Operator" attribute.
  
  - Add escapes [2]. Single quote strings admit only one type of escape: `''`. 
Also, allow escapes in keys quoted. Ex: `"key\"a": value` or 'key''a': value`.
  
  - Add Integers [4] & Floats [5] [3] (the documentation shows the exact 
regular expressions).
  
  - Add booleans [7] & improves the detection of null (add missing null 
keywords) [6]. These are highlighted with "dsChar".
  
  NOTE: Booleans, null, integers & floats require specific delimiters to avoid 
incorrect highlighting (for example, not highlight numbers present in sentences 
or dates). For that reason I use several RegExpr instead of keywords or other 
rules.
  
  **References:**
  
  - [1] YAML Specs, Collections: http://yaml.org/spec/1.2/spec.html#id2759963
  - [2] YAML Specs, Escaped Characters: 
http://yaml.org/spec/1.2/spec.html#id2776092
  - [3] YAML Specs, Integer & Float: 
http://yaml.org/spec/1.2/spec.html#id2803828
  - [4] Integer Language-Independent Type for YAML™ Version 1.1: 
http://yaml.org/type/int.html
  - [5] Floating-Point Language-Independent Type for YAML™ Version 1.1: 
http://yaml.org/type/float.html
  - [6] Null Language-Independent Type for YAML™ Version 1.1: 
http://yaml.org/type/null.html
  - [7] Boolean Language-Independent Type for YAML™ Version 1.1: 
http://yaml.org/type/bool.html

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-yaml

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

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

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

D14763: Definition: Expose command markers

[Christoph Cullmann]

cullmann added a comment.


  I think later we should check for not more than one marker per type in the 
files, but otherwise that is ok for me now.

REPOSITORY
  R216 Syntax Highlighting

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

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

D14757: Warn user before copy operation if available space is not enough

[David Faure]

dfaure added a comment.


  Yes, Pino is right. Trust Pino ;)

REPOSITORY
  R241 KIO

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

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

D14757: Warn user before copy operation if available space is not enough

[David Faure]

dfaure added a comment.


  Ah, hmm, my suggestion would allow to display a warning messagebox, but not 
to offer "continue/cancel" type of interaction.
  
  You can use SimpleJobPrivate::requestMessageBox instead, with the type 
argument set to JobUiDelegateExtension::WarningContinueCancel, instead.

REPOSITORY
  R241 KIO

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

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

D14743: Copy PlasmaCore binary qmldir file structure into bin for unit testsThis way tests pass before installation.

[David Faure]

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

INLINE COMMENTS

> CMakeLists.txt:80
> +COMMAND ${CMAKE_COMMAND} -E
> +copy 
> ${CMAKE_BINARY_DIR}/bin/libcorebindingsplugin.so 
> ${CMAKE_BINARY_DIR}/bin/org/kde/plasma/core/libcorebindingsplugin.so)
> +add_custom_command(TARGET copy POST_BUILD

The filename won't work on Windows. Here's more portable syntax:

  add_custom_command(TARGET copy POST_BUILD
  COMMAND ${CMAKE_COMMAND} -E
  copy $ 
${CMAKE_BINARY_DIR}/bin/org/kde/plasma/core/)

REPOSITORY
  R242 Plasma Framework (Library)

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

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

Re: purpose unittests on Windows

[David Faure]

On samedi 11 août 2018 07:02:18 CEST Ben Cooksley wrote:
> On Fri, Aug 10, 2018 at 7:28 PM David Faure  wrote:
> > Any idea why purpose can't find the KIO http and file protocols -- on
> > Windows?
> > 
> > https://build.kde.org/view/Frameworks/job/Frameworks%20purpose%20kf5-qt5%2
> > 0WindowsMSVCQt5.10/76/testReport/junit/(root)/TestSuite/alternativesmodelt
> > est/
> > 
> > That's very odd, because the dependency from purpose on kio is there in
> > kde-build-metadata (it wouldn't build otherwise), and using kio_file from
> > other frameworks surely works (e.g. in KParts).
> > 
> > The lookup for protocols looks for "kf5/kio" under all Qt plugin
> > directories, i.e. $QT_PLUGIN_PATH.
> > 
> > The CI log says
> > QT_PLUGIN_PATH= 'C:\CI\workspace\Frameworks purpose kf5-qt5
> > WindowsMSVCQt5.10\install-prefix\lib\plugins;C:\Craft\CI\windows-msvc2017
> > _64-cl-debug\lib\qca-qt5'
> > 
> > => is there a kf5\kio subdir in C:\CI\workspace\Frameworks purpose kf5-qt5
> > WindowsMSVCQt5.10\install-prefix\lib\plugins ?
> I've checked and the following seem to be present in that directory,
> which looks correct to me:
> 
> ModeLastWriteTime Length Name
> - -- 
> -a8/10/2018   8:39 PM 274944 file.dll
> -a8/10/2018   8:40 PM 352256 ftp.dll
> -a8/10/2018   8:40 PM 185856 ghelp.dll
> -a8/10/2018   8:39 PM 185856 help.dll
> -a8/10/2018   8:45 PM 946688 http.dll
> -a8/10/2018   8:40 PM 189440 remote.dll
> -a8/10/2018   8:40 PM 122880 trash.dll
> 
> Is there any debug output we can enable to see how it is searching for
> the plugins?

I have now added debug output to kcoreaddons and kio, and the result is:

Checking for plugins in ("C:/CI/workspace/Frameworks purpose kf5-qt5 
WindowsMSVCQt5.10/build/bin/kf5/kio", 
"C:/Craft/CI/windows-msvc2017_64-cl-debug/plugins/kf5/kio")

This list doesn't include the above directory, C:\CI\workspace\Frameworks 
purpose kf5-qt5 WindowsMSVCQt5.10\install-prefix\lib\plugins.

Yet, ECMAddTests.cmake adds to QT_PLUGIN_PATH, it doesn't overwrite it...
I wonder if set(PATHSEP "\\;") is correct though (why double backslash?)
QCoreApplication::libraryPaths reads that env var and splits it using 
QDir::listSeparator() which is ';' on Windows.
Kevin, any reason for the backslash?
Do you see any other reason for this to fail, otherwise?

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

KDE CI: Frameworks kcoreaddons kf5-qt5 SUSEQt5.9 - Build # 48 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20SUSEQt5.9/48/
 Project:
Frameworks kcoreaddons kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 12 Aug 2018 09:26:53 +
 Build duration:
3 min 23 sec and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 22 test(s), Skipped: 0 test(s), Total: 24 test(s)Failed: TestSuite.kdirwatch_qfswatch_unittestFailed: TestSuite.kdirwatch_stat_unittest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report91%
(10/11)85%
(69/81)85%
(69/81)75%
(6324/8422)43%
(10138/23415)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests93%
(26/28)93%
(26/28)97%
(2696/2772)49%
(5797/11895)src.desktoptojson100%
(2/2)100%
(2/2)78%
(88/113)38%
(122/322)src.lib50%
(1/2)50%
(1/2)62%
(352/572)26%
(243/924)src.lib.caching100%
(2/2)100%
(2/2)45%
(354/782)18%
(184/1038)src.lib.io82%
(9/11)82%
(9/11)68%
(854/1265)37%
(1049/2827)src.lib.jobs71%
(5/7)71%
(5/7)57%
(160/281)38%
(52/138)src.lib.plugin100%
(7/7)100%
(7/7)85%
(654/770)42%
(976/2317)src.lib.randomness100%
(2/2)100%
(2/2)70%
(67/96)58%
(44/76)src.lib.text63%
(5/8)63%
(5/8)47%
(368/778)44%
(845/1899)src.lib.util100%
(10/10)100%
(10/10)81%
(731/908)52%
(826/1581)tests0%
(0/2)0%
(0/2)0%
(0/85)0%
(0/398)

KDE CI: Frameworks kcoreaddons kf5-qt5 SUSEQt5.10 - Build # 103 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcoreaddons%20kf5-qt5%20SUSEQt5.10/103/
 Project:
Frameworks kcoreaddons kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 12 Aug 2018 09:26:53 +
 Build duration:
3 min 14 sec and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 22 test(s), Skipped: 0 test(s), Total: 24 test(s)Failed: TestSuite.kdirwatch_qfswatch_unittestFailed: TestSuite.kdirwatch_stat_unittest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report91%
(10/11)85%
(69/81)85%
(69/81)75%
(6325/8422)43%
(10148/23419)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests93%
(26/28)93%
(26/28)97%
(2698/2772)49%
(5806/11893)src.desktoptojson100%
(2/2)100%
(2/2)78%
(88/113)38%
(123/326)src.lib50%
(1/2)50%
(1/2)61%
(351/572)26%
(243/928)src.lib.caching100%
(2/2)100%
(2/2)45%
(354/782)18%
(184/1038)src.lib.io82%
(9/11)82%
(9/11)68%
(854/1265)37%
(1047/2821)src.lib.jobs71%
(5/7)71%
(5/7)57%
(160/281)38%
(52/138)src.lib.plugin100%
(7/7)100%
(7/7)85%
(654/770)42%
(976/2319)src.lib.randomness100%
(2/2)100%
(2/2)70%
(67/96)58%
(44/76)src.lib.text63%
(5/8)63%
(5/8)47%
(368/778)45%
(846/1899)src.lib.util100%
(10/10)100%
(10/10)81%
(731/908)52%
(827/1583)tests0%
(0/2)0%
(0/2)0%
(0/85)0%
(0/398)

D14674: handle non-ASCII encodings of file names in tar archives

[David Faure]

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


  Great job, thanks for the unittest.
  
  Just a few minor adjustments and this is good to go in.

INLINE COMMENTS

> karchivetest.cpp:776
> +const QString longName =
> +QStringLiteral("раз-два-три-четыре-пять-вышел-зайчик-погулять-вдруг-"
> +   "охотник-выбегает-прямо-в-зайчика.txt");

This should be QString::fromUtf8(), because QStringLiteral doesn't support 
UTF-8 on Windows. I *think* it also doesn't support multiline literals, so make 
it a single line.

Alternatively, just use QString("...") like you do further below, that supports 
utf8 and multiline literals.

> karchivetest.cpp:798
> +
> +QVERIFY(listing.count() == 1);
> +

QCOMPARE(listing.count(), 1);

REPOSITORY
  R243 KArchive

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

To: ibragimovrinat, dfaure, kossebau
Cc: xyquadrat, broulik, cfeck, ibragimovrinat, kde-frameworks-devel, michaelh, 
ngraham, bruns

D14757: Warn user before copy operation if available space is not enough

[Shubham]

shubham added a comment.


  In D14757#306938 , @pino wrote:
  
  > Also, considering this is in a job in `KIOCore` (i.e. non-gui library), I 
suspect that using a message box directly is the wrong way to do it. Most 
probably you need to use the UI delegate of the job for this.
  
  
  you mean to use KIO/JobUiDelegate ?

REPOSITORY
  R241 KIO

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

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

D14759: Add bool Definition::isWordWrapDelimiter(QChar)

[Dominik Haumann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:b422c7e3732a: Add bool 
Definition::isWordWrapDelimiter(QChar) (authored by dhaumann).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14759?vs=39501&id=39502

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h

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

D14759: Add bool Definition::isWordWrapDelimiter(QChar)

[Christoph Cullmann]

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

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  add-word-wrap-delimiter (branched from master)

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

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

D14759: Add bool Definition::isWordWrapDelimiter(QChar)

[Dominik Haumann]

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


  - Compile ;)

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14759?vs=39500&id=39501

BRANCH
  add-word-wrap-delimiter (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h

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

D14759: Add bool Definition::isWordWrapDelimiter(QChar)

[Dominik Haumann]

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

REVISION SUMMARY
  Checking word wrap delimiters is used by some langauge to
  make sure static word wrap does not create invalid syntax,
  for instance in LaTeX.

TEST PLAN
  make && make test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  add-word-wrap-delimiter (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/definition_p.h

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

D14757: Warn user before copy operation if available space is not enough

[Pino Toscano]

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


  Also, considering this is in a job in `KIOCore` (i.e. non-gui library), I 
suspect that using a message box directly is the wrong way to do it. Most 
probably you need to use the UI delegate of the job for this.

INLINE COMMENTS

> CMakeLists.txt:146
>   KF5::Service
> + KF5::WidgetsAddons
>   Qt5::Network

This is a private dependency, so it must go to the `PRIVATE` section of 
`target_link_libraries`.

> copyjob.cpp:66
>  
> -
>  #include 

Unneeded change.

> copyjob.cpp:890
> +if (m_totalSize > m_freeSpace) {
> +KMessageBox::warningYesNo(nullptr, i18n("You do not have 
> sufficient space available. Do you still want to continue?"));
> +}

The return value of `KMessageBox::warningYesNo` is ignored, so this will 
continue regardless of the user choice.
Also, the message itself is not informative enough: what are the space needed, 
and the space available? And which directory for?

REPOSITORY
  R241 KIO

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

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

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

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

REVISION SUMMARY
  I don't think there is an ideal solution for this problem, this one has the 
disadvantage that the code handling wheel scrolling is not related to the 
scroll bar any more. It still seems like the simplest one. What do you think?

REPOSITORY
  R39 KTextEditor

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

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

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

D14757: Warn user before copy operation if available space is not enough

[Shubham]

shubham created this revision.
shubham added reviewers: broulik, ngraham.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
shubham requested review of this revision.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/core/copyjob.cpp

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