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

[Christoph Cullmann]

cullmann closed this revision.
cullmann added a comment.


  Git commit e1c3d16a35ddae90de6e457a31f54d899abb84c8 by Christoph Cullmann, on 
behalf of Nibaldo González.
  Committed on 15/08/2018 at 18:22.
  Pushed by cullmann into branch 'master'.
  
  Modelines: add byte-order-mark & small fixes
  
  Summary:
  Changes:
  
  - Add missing keyword: `byte-order-mark` (there is only "byte-order-marker" 
and "bom"). Obtained from: 
https://docs.kde.org/stable5/en/applications/katepart/config-variables.html
  - Remove keywords delimiters "*" and "+", because in the 
"RemoveSpacesOptions" list, the keywords "*" and "+" are not highlighted.
  - Fix ";" in the "RemoveSpaces" context ("remove-trailing-spaces" variable). 
The character ";" it is not highlighted as in the rest of the variables, since 
when writing a value, the context ends.
  
  F6128802: modelines-spaces.png 
  
  - For a strange reason, the use of ``, 
in others files, causes the line continuation character ("\") to be highlighted 
as `dsNormal`. This is solved by removing the LineContinue rule in the "Normal" 
context in Modelines. I consider the LineContinue & DetectSpaces rules to be 
unnecessary in the "Normal" context, so I've eliminated them.
  
  F6128805: modelines-linecont.png 
  
  I was going to add a test file, but there are errors when generating the html 
and ref files, even with the unmodified modelines.xml file :S
  
  Reviewers: turbov, dhaumann, cullmann, vkrause, #kate 

  
  Reviewed By: cullmann, #kate 
  
  Subscribers: kwrite-devel, kde-frameworks-devel
  
  Tags: #kate , #frameworks 

  
  Differential Revision: https://phabricator.kde.org/D14274
  
  A  +10   -0autotests/folding/modelines.py.fold
  A  +17   -0autotests/html/modelines.py.html
  A  +10   -0autotests/input/modelines.py
  A  +10   -0autotests/reference/modelines.py.ref
  M  +7-4data/syntax/modelines.xml
  
  
https://commits.kde.org/syntax-highlighting/e1c3d16a35ddae90de6e457a31f54d899abb84c8

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

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

[Christoph Cullmann]

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


  Works, will integrate, thanks for the patch!

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  arcpatch-D14274

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

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

[Christoph Cullmann]

cullmann added a comment.


  Will try that out again.

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

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

[Nibaldo González]

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


  - Fix test (use Modelines in Python instead of Bash)

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14274?vs=39717=39789

BRANCH
  arcpatch-D14274

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

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

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

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

[Christoph Cullmann]

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

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

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

[Christoph Cullmann]

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


  I needed to revert this again, as we get tests failures:
  
  
https://build.kde.org/job/Frameworks%20syntax-highlighting%20kf5-qt5%20SUSEQt5.9/97/testReport/junit/(root)/TestSuite/testhighlighter_test/
  
  -# kate: 
syntax Bash 
; 
scheme 
Normal;
  -# kate: 
font-size 
14; 
background-color #FAFAFA 
;
  +# kate: syntax Bash ; scheme Normal;
  +# kate: font-size 14; background-color #FAFAFA ;

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

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

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:bffb66cb3ed8: Modelines: add byte-order-mark  small 
fixes (authored by nibags, committed by cullmann).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14274?vs=39552=39717

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

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

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

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

[Christoph Cullmann]

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


  OK, thanks.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  arcpatch-D14274

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

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

[Nibaldo González]

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


  - Corrects references erroneously included in previous update

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14274?vs=39551=39552

BRANCH
  arcpatch-D14274

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

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

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

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

[Nibaldo González]

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


  - Add test & add DetectSpace.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14274?vs=38477=39551

BRANCH
  fix-modelines

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

AFFECTED FILES
  autotests/folding/modelines.sh.fold
  autotests/html/modelines.sh.html
  autotests/input/modelines.sh
  autotests/reference/highlight.css.ref
  autotests/reference/modelines.sh.ref
  autotests/reference/review128925-1.css.ref
  autotests/reference/review128925-2.css.ref
  data/syntax/modelines.xml

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

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

[Nibaldo González]

nibags added a comment.


  I will update the diff including the tests.

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

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

[Nibaldo González]

nibags added a comment.


  So I keep the DetectSpaces rule and only delete the LineContinue.
  
  By having a file with Modelines as a test, randomly, different .html and .ref 
files are generated each time I run "testhighlighter_test" and 
"htmlhighlighter_test". That is, sometimes the .html and .ref files show the 
Modelines highlighted and sometimes not. Also, the word `kate:` should be 
highlighted with "dsAnnotation" (which is pink in the Normal scheme). I attach 
a screenshot of two generated .html files.
  
  F6190099: Screenshot_20180813_033824.png 

  
  The test file:
  F6190103: modelines.sh 

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

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, )) {
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

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

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

[Nibaldo González]

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


  - Delete update references

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14274?vs=38201=38477

BRANCH
  fix-modelines

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

AFFECTED FILES
  data/syntax/modelines.xml

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

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

[Nibaldo González]

nibags edited the summary of this revision.
nibags added reviewers: turbov, dhaumann, cullmann, vkrause, Kate.

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

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

[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
  Changes:
  
  - Add missing keyword: `byte-order-mark` (there is only "byte-order-marker" 
and "bom"). Obtained from: 
https://docs.kde.org/stable5/en/applications/katepart/config-variables.html
  - Remove keywords delimiters "*" and "+", because in the 
"RemoveSpacesOptions" list, the keywords "*" and "+" are not highlighted.
  - Fix ";" in the "RemoveSpaces" context ("remove-trailing-spaces" variable). 
The character ";" it is not highlighted as in the rest of the variables, since 
when writing a value, the context ends.
  - For a strange reason, the use of ``, 
in others files, causes the line continuation character ("\") to be highlighted 
as `dsNormal`. This is solved by removing the LineContinue rule in the "Normal" 
context in Modelines. I consider the LineContinue & DetectSpaces rules to be 
unnecessary in the "Normal" context, so I've eliminated them.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  fix-modelines

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

AFFECTED FILES
  autotests/reference/highlight.css.ref
  autotests/reference/review128925-1.css.ref
  autotests/reference/review128925-2.css.ref
  data/syntax/modelines.xml

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