D14978: Add unit test that checks Format data

2018-08-23 Thread Dominik Haumann
This revision was automatically updated to reflect the committed changes. Closed by commit R216:e4d317da92a1: Add unit test that checks Format data (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14978?vs=40349&id=40351 REVI

D14978: Add unit test that checks Format data

2018-08-23 Thread Christoph Cullmann
cullmann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH check-full-formats (branched from master) REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-devel, kde-frameworks-devel,

D14978: Add unit test that checks Format data

2018-08-23 Thread Dominik Haumann
dhaumann updated this revision to Diff 40349. dhaumann added a comment. - Verify Definition::formats() contain all formats from 1...N REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14978?vs=40172&id=40349 BRANCH check-full-formats (branched f

D14978: Add unit test that checks Format data

2018-08-22 Thread Dominik Haumann
dhaumann added a comment. True, agreed. And it was an ugly workaround in the first place. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns,

D14978: Add unit test that checks Format data

2018-08-22 Thread Christoph Cullmann
cullmann added a comment. I would be in favor of removing Alerts_indent. It has no use ATM. Even if the indentation setting would be important, we can't read that value, as no format of that definition is ever ending up in any textline. REPOSITORY R216 Syntax Highlighting REVISION DETAI

D14978: Add unit test that checks Format data

2018-08-21 Thread Dominik Haumann
dhaumann added a comment. The problem likely was that Alerts.xml does not have the indentationBasedFolding attribute set. But it's needed in e.g. Python. dh@eriador:syntax :-) (check-full-formats) $ grep Alerts_indent * alert_indent.xml: coffee.xml: coffee.xml:

D14978: Add unit test that checks Format data

2018-08-21 Thread Dominik Haumann
dhaumann updated this revision to Diff 40172. dhaumann added a comment. - Simplify code REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14978?vs=40167&id=40172 BRANCH check-full-formats (branched from master) REVISION DETAIL https://phabric

D14978: Add unit test that checks Format data

2018-08-21 Thread Christoph Cullmann
cullmann added a comment. Given that hl definition is "eaten" during the include cycle, I guess one can just delete it and directly include the other one. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-de

D14978: Add unit test that checks Format data

2018-08-21 Thread Christoph Cullmann
cullmann added a comment. I think the issue is, that if you have just include rules and a context with include rules, you will skip that one context completely during including. Therefore it is missing in the end result, I assume. REPOSITORY R216 Syntax Highlighting REVISION DETAIL htt

D14978: Add unit test that checks Format data

2018-08-21 Thread Dominik Haumann
dhaumann added a comment. Yes: 1. Why don't we see Alert_indent.xml 2. Is Alert_indent.xml still needed with the new KSyntaxHighlighting framework? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-de

D14978: Add unit test that checks Format data

2018-08-21 Thread Dominik Haumann
dhaumann added a reviewer: vkrause. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann

D14978: Add unit test that checks Format data

2018-08-21 Thread Christoph Cullmann
cullmann added a comment. Still the question is: why is that not seen in includedDefinitions. That hl ist not at all found, thought it is included. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann Cc: kwrite-devel, kde-framewo

D14978: Add unit test that checks Format data

2018-08-21 Thread Dominik Haumann
dhaumann added a comment. From kate.git: commit 6c9ee3c58549d6cad46b9a42cf7407d25cdfa62e Author: Joseph Wenninger Date: Thu Sep 17 23:33:39 2009 + I'm temporarily adding an alert highlighting whic

D14978: Add unit test that checks Format data

2018-08-21 Thread Dominik Haumann
dhaumann added a comment. Interesting: alert_indent.xml has no rules except one IncludeRules (And I am supposed to be the author?!): REPOSITORY R216 Syntax Highlighting REV

D14978: Add unit test that checks Format data

2018-08-21 Thread Christoph Cullmann
cullmann added a comment. Btw., the test is a bit easier to read with: // collect all formats, shall be numbered from 1.. QSet formatIds; for (auto d : qAsConst(includedDefs)) { const auto formats = d.formats(); for (const auto format : formats) { // n

D14978: Add unit test that checks Format data

2018-08-21 Thread Dominik Haumann
dhaumann added a comment. What fails here is Jira.xml: QDEBUG : KSyntaxHighlighting::RepositoryTest::testIncludedFormats() syntaxrepository_test(4584)/(default) ?[31m?[34mKSyntaxHighlighting::RepositoryTest::testIncludedFormats?[0m: QVector(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,

D14978: Add unit test that checks Format data

2018-08-21 Thread Christoph Cullmann
cullmann added a comment. contains 363 Normal Text Alerts_indent is missing. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, s

D14978: Add unit test that checks Format data

2018-08-21 Thread 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 This unit test loads just one definition including its included