ktexteditor and syntax-highlighting porting

[Dominik Haumann]

On Sun, Jul 29, 2018 at 4:11 PM, Volker Krause <vkrause at kde.org> wrote:
> On Sunday, 29 July 2018 13:35:23 CEST Dr.-Ing. Christoph Cullmann wrote:
>> Hi,
>>
>> > On Sun, Jul 29, 2018 at 12:18 PM, Dr.-Ing. Christoph Cullmann
>> >
>> > <cullmann at absint.com> wrote:
>> >> Hi,
>> >>
>> >>> On Sun, Jul 29, 2018 at 9:53 AM, Dominik Haumann <dhaumann at kde.org>
> wrote:
>> >>>> [...]
>> >>>>
>> >>>> What we can add:
>> >>>> 1. QHash<uint16_t, Format> Definition::formatHash() const;
>> >>>> 2. Format Definition::format(uint16_t) const;
>> >>>> 3. QHash<uint16_t, Format> Repository::formatHash() const;
>> >>>> 4. Format Repository::format(uint16_t) const;
>> >>>
>> >>> I see an issue with 3) because Definitions are lazy-loaded.
>> >>> So the number of Format items may grow over time if more Definitions are
>> >>> loaded. So I think we should not add this.
>> >>
>> >> as commented in the phabricator request: I would actually not do
>> >> repository
>> >> global id's at all.
>> >> Or is that needed for embedded highlightings?
>> >
>> > From what you wrote, I had the impression that you WANT global IDs and
>> > you WANT to just have one lookup function for all IDs. In fact, wasn't
>> > that what we discussed at that time also with Volker? Globally unique
>> > IDs?
>>
>> I think I wanted just unique id's per definition including all included
>> definitions. ;=) But given that is 1-2 years ago, I might be confused ;=)
>>
>> At least I think in our current code it is that way, that we build a unique
>> id per highlighting (including all included ones).
>>
>> After inspecting the code, I think the issue is, that we need the global
>> unique id, as the inclusion works differently than in our code and the
>> formats are not duplicated into the definition that includes an other one.
>>
>> That makes it harder to get a per highlighting vector, as we would need one
>> that includes all included formats, too.
>>
>> But perhaps I misread the code.
>
> No, I think that's how it works from what I remember. The format id code was
> added based on what KTextEditor needed, it's not used anywhere else AFAIK. So
> feel free to adjust this to whatever you need. But it's almost 2 years since I
> touched this, memory might be a bit fuzzy ;-)

It's exactly as described above a Repository-global unique ID. We
decided against Definition-local IDs, since we then would have
different IDs for IncludeRules-included Definitions, which imho
currently sucks in KTextEditor: In KTextEditor, if you change a color,
you have to do it for *every* syntax highlighting that uses a
Definition via IncludeRules. This is not feasible.

Two years back, we discussed this, and Volker did the necessary
adjustments. I think we should adapt the way it works in KTextEditor -
and currently I think there is no reason why this should not work.

See my review requests:
- Add Definition::formats()
- Add Definition::includedDefinitions()

These two together allow to build the ID list you want. Or do I miss anything?

Best regards
Dominik