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

2019-08-15 Thread Christoph Cullmann
cullmann added a comment. I think the current way we handle that is OK. If people start to include keywords between "incompatible" languages, any chosen direction is not really desirable. If somebody has time, for sure it makes sense to document the current status quo. REPOSITORY R216

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

2019-08-15 Thread jonathan poelen
jpoelen added a comment. I don't think using the original word delimiters is a good thing. Let's imagine that c.xml lists the functions of the libc and a language that allows them to be used as `ffi.C.printf` (luajit for the curious). The separators of the 2 languages are not the same and `p

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

2019-08-14 Thread Dominik Haumann
dhaumann added a comment. What just came to my mind is that this patch also has an issue: If you include a keyword list of a.xml in b.xml, and a.xml uses different word delimiters for the keyword list, then the keywords in b.xml will not be highlighted/detected correctly. Did we think a

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

2018-10-27 Thread Christoph Cullmann
cullmann added a comment. I merged that now. I bumped the version + required kate version Git commit f2c29ec618da08ebe9d17ff739e8b12bf3c33fff by Christoph Cullmann. Committed on 27/10/2018 at 15:14. Push

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

2018-10-27 Thread Christoph Cullmann
This revision was automatically updated to reflect the committed changes. Closed by commit R216:83a5c8ce9882: keyword rule: Spport for keywords inclusion from another language/file (authored by jpoelen, committed by cullmann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE htt

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

2018-10-20 Thread Christoph Cullmann
cullmann accepted this revision. cullmann added a comment. This revision is now accepted and ready to land. Then I would say this should go in. We can improve the internals afterwards. The XML files should get a version raise and kateversion raise, I assume, before commiting. REPOSITORY R

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

2018-10-20 Thread Volker Krause
vkrause added a comment. In D14632#346319 , @cullmann wrote: > Volker, have you any objections to have this feature? I'm all for it, seeing how much duplication it removes :) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https:

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

2018-10-20 Thread Christoph Cullmann
cullmann added a reviewer: vkrause. cullmann added a comment. Just played a bit with it. Seems to work reasonable well :) As the only visible change for the outside is the XML addition to the keyword list, I think we might even just improve the internal API later. Nothing of it is expos

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

2018-10-11 Thread Christoph Cullmann
cullmann added a comment. Hi, I still need to test this. For the enum, I have no issue with scoped enums, but I think something like DefinitionData::LoadOptions::OnlyKeywords would be better than a enum that just encapsules a bool. Actually one could create a proper Q_FLAGS to all

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

2018-09-30 Thread jonathan poelen
jpoelen updated this revision to Diff 42628. jpoelen added a comment. - fix the recursion detection and handling of nested included keyword lists REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14632?vs=41872&id=42628 BRANCH kwinclude REVISIO

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

2018-09-26 Thread Christoph Cullmann
cullmann added a comment. I can live with the second kind of loading state, but in any case one needs to fix the recursion detection and handling of nested included keyword lists. e.g. a includes b includes c ATM I think "something" will happen REPOSITORY R216 Syntax Highlighting

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

2018-09-26 Thread Dominik Haumann
dhaumann added a comment. @cullmann Can you make a decision? I trust it will be a good one ;) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: jpoelen, #framework_syntax_highlighting, cullmann, dhaumann Cc: kwrite-devel, vkrause, kde-frameworks-d

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

2018-09-18 Thread Dominik Haumann
dhaumann added a comment. @cullmann If the other solution is easier to maintain then I am fine with this. Still, I would like to avoid that includedDefinitions() for sass returns css just because of keywords. In fact, that was my main motivation for this solution. REPOSITORY R216 Syntax H

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

2018-09-17 Thread Christoph Cullmann
cullmann added a comment. I don't know if the added complexity for loading only keywords is needed, but in any case one needs some recursive keyword resolving with a cycle guard. e.g. if you use a list that again includes another one. REPOSITORY R216 Syntax Highlighting REVISION DETAIL

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

2018-09-17 Thread jonathan poelen
jpoelen added a comment. (I inadvertently edit the operators list in the previous commit) Finally, I added a parameter to `load()` and `loadHighlighting()` rather than a new function because there was a lot of code duplication. REPOSITORY R216 Syntax Highlighting REVISION DETAIL htt

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

2018-09-17 Thread jonathan poelen
jpoelen updated this revision to Diff 41872. jpoelen added a comment. - restore operators list - lazy load keyword lists REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14632?vs=41298&id=41872 BRANCH kwinclude REVISION DETAIL https://phab

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

2018-09-16 Thread Dominik Haumann
dhaumann added a comment. Yes, and calling loadKeywords would only do work the first time. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: jpoelen, #framework_syntax_highlighting, cullmann, dhaumann Cc: kwrite-devel, vkrause, kde-frameworks-deve

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

2018-09-16 Thread jonathan poelen
jpoelen added a comment. `resolveIncludeKeywords()` would then use `loadKeywords()` instead of `load()` ? This should be enough. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: jpoelen, #framework_syntax_highlighting, cullmann, dhaumann Cc: kwr

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

2018-09-16 Thread Dominik Haumann
dhaumann added a comment. To be clear, currently we have two states: - load metadata only - load full file Instead, we could have three states: - load metadata only - load keyword lists (in addition to metadata) - load full file This is my preferred solution. ;) REPOS

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

2018-09-16 Thread Dominik Haumann
dhaumann added a comment. This was also my interpretation: only include the Definitions that use itemDatas / colors. What we currently do: we already support loading the language metadata without loading the rest. Then, we support loading the full XML file. What we could do: add the

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

2018-09-15 Thread jonathan poelen
jpoelen added a comment. Sorry to answer so late, I had trouble logging in. I think I misinterpreted the use of `includedDefinitions()`. For me, this function lists the languages ​included by the rules and which influence the colors. While the inclusion of a keyword list has no influence

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

2018-09-13 Thread Christoph Cullmann
cullmann added a comment. If you have no time for the wanted refactoring, I can do that this weekend and commit this, the syntax highlighting format changes are good as is ;=) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: jpoelen, #framework_

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

2018-09-10 Thread Christoph Cullmann
cullmann requested changes to this revision. cullmann added a comment. This revision now requires changes to proceed. Please unite the includedDefinitions() Beside that, I think it would make sense to have the resolveIncludeKeywords() part inside the KeywordList class, then we don't need tha

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

2018-09-09 Thread Christoph Cullmann
cullmann added a comment. The properties##CSS variant is really just like what I wanted, too! Nice. Only nitpick API wise: I would not expose the difference between includedDefinitions and includedKeywordDefinitions, I would just like to have all included definitions in includedDefinitio

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

2018-09-09 Thread jonathan poelen
jpoelen updated this revision to Diff 41298. jpoelen added a comment. include rule in keyword list to import those from another file REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14632?vs=39138&id=41298 BRANCH kwinclude REVISION DETAIL ht

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

2018-09-02 Thread Christoph Cullmann
cullmann added a comment. For the keywords stuff, one must keep in mind I altered the implementation of keyword lists a lot in: https://phabricator.kde.org/D15217 REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: jpoelen, #framework_syntax_h

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

2018-08-12 Thread 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 integrati

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

2018-08-06 Thread jonathan poelen
jpoelen added a comment. I just thought of `Definition::keywordLists()` which does not list the keywords used by this new syntax. - Should we add the lists used as and when parsing? - Move the functionality on the tag ``? (``, ` name##language `) - Other? REPOSITORY R216 Synt

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

2018-08-06 Thread jonathan poelen
jpoelen edited the summary of this revision. 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, demskin

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

2018-08-06 Thread Christoph Cullmann
cullmann added a comment. Great ;) I assume we can handle the missing things during that week for a real switch. ATM the basics already work, there are just some missing extras. (and the color configuration is a mess) If you have some time to take a look at the syntax-highlighting

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

2018-08-06 Thread Volker Krause
vkrause added a comment. In D14632#304149 , @cullmann wrote: > Volker, are you at the conference next week? Sure, I'll be there for the entire week. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D146

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

2018-08-06 Thread Dominik Haumann
dhaumann added a comment. @jpoelen When adding such functionality, could you also also explain in the log message *why* this change is required or useful? I think we should have real-world use cases before integrating this. But in general, looks good. REPOSITORY R216 Syntax Highlighting R

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

2018-08-06 Thread Christoph Cullmann
cullmann added a comment. I think this needs to wait with merging until we finalized the usage of the syntax-highlighting framework in ktexteditor. Volker, are you at the conference next week? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: j

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

2018-08-06 Thread Volker Krause
vkrause added a comment. Good idea IMHO, and now that KTE is moving to the KSyntaxHighlighting engine we can actually look at feature extensions again :) The engine and language changes look good to me, but I'll leave the final decision to Dominik/Christoph as I'm not sure about the impl

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

2018-08-05 Thread jonathan poelen
jpoelen edited the summary of this revision. jpoelen edited the test plan for this revision. 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,

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

2018-08-05 Thread jonathan poelen
jpoelen created this revision. jpoelen added reviewers: Framework: Syntax Highlighting, cullmann, dhaumann. Restricted Application added projects: Kate, Frameworks. Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. jpoelen requested review of this revision. REVISION SUM