Fix end of folding region in rules with lookAhead=true
ClosedPublic

Authored by nibags on Jan 25 2019, 10:00 AM.

Details

Summary

The text length of end/begin of folding region is 0 if the respective rule has lookAhead="true".

For example:

Diff Detail

Repository
R216 Syntax Highlighting
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nibags created this revision.Jan 25 2019, 10:00 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptJan 25 2019, 10:00 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
nibags requested review of this revision.Jan 25 2019, 10:00 AM

Interesting idea.

This changes all C/C++ Preprocessor #if/elsif/endif regions to empty code folding regions.

What I plan on KTextEditor is the following feature: for code folding of e.g. in LaTeX:

\begin{document}
    \begin{center}
    Blaaa
    \end{center}
\end{document}

Consider the cursor is moved in the begin{document} text, then I want to highlight the text \begin{document} and the counterpart \end{document} that define a folding region with a moving range. As you see, it is required to know the offset width of the folding regions then.

This will still work with this patch, since the rules do not use lookAhead. But for Preprocessor if...elsif...endif it will not work anymore.

Question is: is that / will that be a problem?

I think this patch requires a bit manual testing, but in itself it's a good patch.

nibags added a comment.EditedJan 28 2019, 7:52 AM

Some highlighters use endRegion+lookAhead to determine the end of a region when a new one starts, that is, they don't consider the text with endRegion+lookAhead as part of the fold region. This behavior was present before the implementation of KSyntaxHighlighting and, apparently, is no longer from the commit: c004f3f787b2

This problem is noticeable when hiding/showing a region: when a region collapses, the first line of the next region is also included (for example, in the .diff files). By doing a search: (endRegion="[^>]*lookAhead="true"|lookAhead="true"[^>]*endRegion=") in "data/syntax/" I can see the affected files. I leave some examples attached.

Perhaps another solution is better, such as adding some attribute, such as lookAheadEndRegion="true", for the rules that need it. This way you can control the behavior in the C/C++ preprocessor and in other languages.

Ok I see: this kind of reverts Christophs changes in the lookahead case.

What I wonder is whether we need a // TODO KF6: add a bool lookAhead in applyFolding() such that we know whether a folding end region should end at 0 or at length.

@cullmann Your take on this? I think it's an improvement, but a comment could be added that states why this differentiation is done.

cullmann accepted this revision.Feb 2 2019, 3:15 PM

I think that lookAhead rules don't publish their length sounds like a sane improvement.

All in all, I think the folding still has some general issues in how we handle it.

e.g. I have ATM "interesting" highlighted areas for the folding of doxygen comments.

I don't think we need to have an extra bool, one might later internally handle it differently with extra attributes in the hl files, like a lookAheadEndRegion as proposed, if we really don't like the general change.

This revision is now accepted and ready to land.Feb 2 2019, 3:15 PM
This revision was automatically updated to reflect the committed changes.