ktexteditor and syntax-highlighting porting

[Dominik Haumann]

Done.

If you find any issues with these patches, please let me know.
Next tag is next Saturday (August 4th). After this day, binary
compatibility holds ;)

Greetings
Dominik


On Sun, Jul 29, 2018 at 10:03 PM, Dominik Haumann <dhaumann at kde.org> wrote:
> 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