D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

dhaumann updated this revision to Diff 38721. dhaumann added a comment. Update REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14451?vs=38720=38721 BRANCH addFormatGetters (branched from master) REVISION DETAIL

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

dhaumann updated this revision to Diff 38720. dhaumann added a comment. - Update REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14451?vs=38695=38720 BRANCH addFormatGetters (branched from master) REVISION DETAIL

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

vkrause added a comment. The whole format id code is indeed not used here, this was purely added based on the requirements I got from you guys :) So feel free to change whatever is necessary. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14451 To:

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

cullmann added a comment. I think if we have 1. formats() per definition 2. some "includedDefinitions" we can build up the needed format list per highlighting that will allow us to map the syntax-highlighting formats to the KTextEditor::Attribute we need internally and in our

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

dhaumann added a comment. > e.g. for the customization of coloring you want to present the list of all formats a highlighting has, including the included ones. (even if later the color of the included ones is not stored per including highlighting) The included ones behave differently in

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

cullmann added a comment. e.g. for the customization of coloring you want to present the list of all formats a highlighting has, including the included ones. (even if later the color of the included ones is not stored per including highlighting) Otherwise you are right, a global

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

dhaumann added a comment. Btw, thinking more about it: I believe we do want globally unique IDs for a very simple reason. Think of e.g. bracket matching where multiple Definitions are included (e.g.: html { . For the last '}', we want to find the matching attribute '{'. With globally

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

cullmann added a comment. I think an issue is the inclusion of definition in each other. I think ATM the formats are really only per definition, that means if the ids would be definition local, one will have clashs. I think ATM there is no way to get formats for definitions that got

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

dhaumann added a comment. I just had a look into the code - the Format IDs are currently not used at all by KSyntaxHighlighting. Instead, all format lookups are done via the Format name. And that by definition means that the Format looksups are per Definition, since Formats from different

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

cullmann added a comment. I am wondering: Do we really need to have unique IDs over all definitions? Actually I would think it would be much nicer to have all formats for one definition in a vector and having the index as the unique id. The lookup is faster and it is easier to understand.

D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

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 With