Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}

[x2nie]

.. please write a brief description on what did you till now and what i can do (i tried reading last pages but it was not clear for me because i wasn't involved)

Sure. Here are what I knew:


* TSynHighligther (aka HL) knows the keywords. it does parsing to know the keywords. So, let it be only who doing parsing.
* TSynEditMarkup (and it's ancestors) is the only correct place of doing additional markup (additional color other than keywords).
* TSynMarkupWordGroup is the simplest markup to learn how HL & markup work together.


* HL shouldn't do drawing it self. never.
* HL provides several signals to be used to draw.
   Such T***Attri for coloring the keyword,comment,symbol by specific colors. It's a legacy from old SynEdit (still founded in Github or Sourceforge).
   and Lazarus's SynEdit provide additional signal via TNodeList.
* TNodeList can be called per-line basis, and can be called several time when needed by markup.
* TNOdeLIst contains several FoldNodeInfo, which each is generally for fold/unfold info (whether it is opening fold or closing one).
* Additionally, each FoldNodeInfo contains set of (
   * sfaMarkup {used by TMarkupWordGroup},
   * sfaOpen,sfaClose {used by TMarkupWordGroup to connect/find paired keyword such begin..end, try/finally/end, procedure/begin/end etc}
   * sfaFold {used by FoldGUtter and CodeBUffer to collapse/expand a block of code, together with sfaOpen&sfaClose}
   * sfaHide {for comments, it is similar as sfaFold, but will collapse to previous line while sfaFold collapse to first-block-line}
   * sfaOutline {it is new, for our colorizing. If any nodeInfo contains "sfaOutline", I got it. If it doesn't contain "sfaOutline" I ignore it}


FoldNodeInfo is generated by  HL, but it depends on Markup class existence to draw.
Well, neither Markup do drawing by itself. Each markup also only provides some instruction of drawing to be done by TSynEdit.
The real drawer is not Markup nor HL. It is because even small individual drawing will cost of time.
Managed drawing will reduce time. Believe it!


Here is a simple diagram of their connectivity:
Form1
  > SynEdit1
        > SynMarkupManager1
              > SynMarkupWordGroup1
              > SynMarkupColororing1
  > SynFreePascalSyn1


some markups has been created automatically inside SynEdit. So, you must add SynMarkupColoring1 manually.


So, what was I really doing?
* The root class of all foldable HL (TSynFoldHighlighterBase) now provide simple sfaOutline for every foldable node.
   But it later depends on ancestor of whether it should foldable or not (and has outline or not) by setting SupportedMode & Mode properties.
* TSynPasSyn (and TSynFreePascalSyn) still never draw any markup by itself.
   It just provide those FoldNodeInfoList which in turn will be used to draw by any markup.
* my TSynMarkupColoring class, for each line, will call HL.NodeInfo[ y ];
   In fact, for each line, TSynMarkupColoring will call that too many time as needed, because I need to get all parent.
 


See? The latest problem which not yet being solved is: it always search all parents, even those parents has been founded for previous line.
Why it is hard? Because it involves some classes, It is not written in a simple function to do that. I need to learn more deeply to achieve that. (and more time)


Don't worry, there is a way for quick jump/  to get be fast involved:


1. search the call of "StartCodeFoldBlock" and "EndCodeFoldBlock" for any foldable HL.
(In TSynPasSyn, these are wrapped in method "StartPascalCodeFoldBlock" and "EndPascalCodeFoldBlock", with different parameter)
Both methods is a chance to provide signals (sfaOpen, sfaFold, sfaHide, sfaOutline, etc.).
So you can detect when it is called by search in whole file (Ctrl+Shift+F).


2. The real nodefoldinfo's generator is method: "DoInitNode". this method is called by above "StartCodeFoldBlock" and "EndCodeFoldBlock".
TSynPasSyn has it's own DoInitNode, because it has 3 group: Neutral/Pascal folding group, {$IFDEF folding group, {%region folding group.
Any other HL seem don't need to override this method.


I highly recomended to learn how TMarkupWordGroup works.
Then you can learn something else, but avoid to learn TPasSynPas too early, because it complexity may confusing you of learning the essential things.


greeting.

[Pascal]

Martin_fr, any plans when this will be implemented? I am looking forward using this feature.
Nice work x2nie.

[Martin_fr]

Last 2 month have been very busy for me, and I still am.

This really is my top 1 priority, as soon as I can make sufficient time.

[Pascal]

Hello Martin_fr,

nice to see you working on this! Looks promising so far.

Regards
Pascal

[Martin_fr]

Well x2nie did the work.
It was just unfortunate that I got real busy for the start of this year.

[x2nie]

Hello,


what I can help further?

[x2nie]

Martin_fr, I've tried your modification.
Anyhow, the speed improvement you made is unbelievable : as fast as without MarkupOutLine.
Congratulation
But, about the visual result of that markup, It was too old, its left far away from my last modification,
Well I believe you did it indeed to control the code quality.





Okay, we have so many improvement beyond that, that you avoid to just apply them at glance.
So now, Let's organize to merge them one by one, since you seem as has time nowadays.


1. I think the good start is to make pas HL more natural.
    It is because people will use pas HL as guidance when they develop their own HL,
    and because pas HL is most comprehensive HL we have.
    In detail,
    a) let we remove the hard to understand "CountPascalCodeFoldBlockOffset ",
        since its not portable/ not reusable, and is actually has another meaning: hidden sfaFold.
    b) let remove InitNode in pas HL, replace with DoInitNode (override virtual).


2. Let's add more happiness to any new HL development, by enabling sfaOutline by-default.
    Using current code, developer should construct the complex fold config when
    they want see the coloring-outline?  it's not fun.
    I think, why being complicated if we can make it easier automatically.
    Once developer ready and satisfied with earlier step, then that is the time to deal with fold config.
    And, it's easy for you too. Just add 2 constants :

Code: Pascal  [Select][+][-]

1.       act := act + [sfaFold, sfaFoldFold, sfaMarkup, sfaOutline];

from

Code: Pascal  [Select][+][-]

1.       act := act + [sfaFold, sfaFoldFold];

What do you think? or you may already have another further plan for it now ?

[Martin_fr]

First of all, I am still merging (some of) your changes. Just done another batch.

Done some small fixes and refactor for those.

For pas-hl I could not use the base class CollectNodeInfo, it breaks the test. It also misses the FAtLineStart for hide action, but even if I apply the fix for it (see jscript, at least I think that should fix it), its still fails several tests from the testcase.

It works now, with the CollectNodeInfo being overridden as empty.

I know its still missing for, while, .... on my list.

I will need to carefully look at the other changes you made. More when I get there.

There still is the issue of missing invalidation.

- go to a begin/end block of at least 10 lines.
- position the caret in the line with the "begin",
   but not touching the begin itself.
   Position the caret in the leading spaces of the line.
- now add some spaces.

The begin will indent, so will the horizontal line, but only on the top 3 text lines of the block. The rest will not be redrawn.

Don't try to fix it just now. I will post some more details, of what happens, and what options to fix.

  a) let we remove the hard to understand "CountPascalCodeFoldBlockOffset ",
        since its not portable/ not reusable, and is actually has another meaning: hidden sfaFold.

Not very urgent at the moment. But yes, eventually will be done. But if I am right, it can be done without adding the FFoldable field.

Currently I do not want to add that field. In fact, I want to reduce FoldLevel and NestLevel into one.

All that can be done, but its a different story. Need to look at the rest first.

b) let remove InitNode in pas HL, replace with DoInitNode (override virtual).

You already had that done, and I just merged it.

Let's add more happiness to any new HL development, by enabling sfaOutline by-default.

Let me first finish merging. And see if other bugs turn up that need fixing.

Better to make it avail via Object inspector.

On the long run, SynEdit should tell the HL, if any module needs the info. And the HL can ignore unneeded, yet enable config. I have some ideas there. But that is for when all else is working.
This will be important, if the HL can actually skip work, if some conf is disable or un-needed.

Also the IDE will need to provide options to the user.

Lets first get it working in the IDE.

[Martin_fr]

If I am right, the only outstanding change for merging from your git, are in SynPasSyn?

[Martin_fr]

Ok, about the invalidation issue.

First basics.
1) the goal is to invalidate (and then paint) as little as possible. A line should only be invalidated, if something actually changed. (except, currently it is always full lines (but with or without gutter)

2) minimum enclosing rectangle. If more than 1 point is invalidated, the invalidation will cover the minimum enclosing rectangle.
Invalidate line 1, and line 5, and the lines in-between will be repainted too. (but you can not rely on this)

3) Invalidation must be done, before paint.

(2)  is why you may not see the missing invalidation of your vertical line, if other code (word brackets like begin/end) cause invalidation.

(3) means that in GetMarkupForRow and similar, it is to late to invalidate, because those are called only during paint.

(1) means that you need to know the old state, in order to know if the new state is different, and if you need to repaint.

Example:

Code: Pascal  [Select][+][-]

1. Program a;
2.    {$mode objfpc}
3.     begin // some comment
4.     |
5.     |  repeat
6.     |  |
7.     |  until false;
8.     end.
9.  

Lets assume a SynEdit with all other markup disabled, if a line is edited, then only that line is invalidated.

So if the begin was moved, then you need to invalidate all lines down to the end.

Yet if the line with the begin is edited, then you do not know if the begin actually moved, maybe someone edited the comment, or the line starts with a tab, and a space was added before the tab in such way that it did not move anything after the tab.
And if the begin did not move, you are not allowed to invalidate the other lines.

It could also be that the begin or end where deleted. Or changed their level/color as another begin end was added around them.

When changes to the text were made, and before the painting begins, you must find what to invalidate.
You can get the current positions and levels of any needed markup. But you also need the previous, to compare them.

You dont want to store data for 10000 lines of text, only for the current screen. But even then, you can not just go like:
topline = line 1183
bottomline = line 1241
because with folding top and bottomline can be many thousand lines appart.

You want to number the screen lines from 0 to 41 (example). (You then need to listen for resize/zoom, to adjust the space, but more later.)

On each of them you can store the position and

Code: Pascal  [Select][+][-]

1. 0: [],  //Program a;
2. 1: [],  //   {$mode objfpc}
3. 2: [(x: 2, l:1)],  //    begin // some comment
4. 3: [(x: 2, l:1)],  //    |
5. 2: [(x: 2, l:1), (x:4, l: 2)],  //    |  repeat
6.  

That allows you to invalidate correctly.

only, if the screen scrolls, so must your array. Otherwise you would risk to invalidate the entire screen when it scrolls. (And that must not happen)

Also lines may be inserted, or deleted (in text or via fold). One way to detect that is to store with each screen line, what line in the text it is.

If your "program" line is at line 1183 in the text, and scrolled so it's on top of the screen.

Code: Pascal  [Select][+][-]

1. 0: [1183],  //Program a;
2. 1: [1184],  //   {$mode objfpc}
3. 2: [1185, (x: 2, l:1)],  //    begin // some comment
4. 3: [1186,(x: 2, l:1)],  //    |
5. 2: [1187,(x: 2, l:1), (x:4, l: 2)],  //    |  repeat
6.  

A good place to build the list, and do the invalidate is
markup.TextChanged

SynEdit has
    function ScreenRowToRow(ScreenRow: integer; LimitToLines: Boolean = True): integer;
    function RowToScreenRow(PhysicalRow: integer): integer;

to go through all screen lines (including the half visible at the bottom)

Code: Pascal  [Select][+][-]

1.   for i := 0 to markup.linesInWindow do begin
2.     realLine := TSynEdit(markup.SynEdit).ScreenRowToRow(i);
3.     // real line may increase by more than 1, if a folded section is skipped
4.     ...
5.   end.
6.  

---
store the old value of TopLine and LinesInWindow, and check in TextChanged if the values for TopLine or LinesInWindow have changed.

However if no text changed, then TextChanged will not be called. So when GetMarkupFor.... is called you need to check too. In that case invalidation will have been done by whatever caused the change, you only need the data for the current paint

[Pascal]

Martin_fr,

your last commit (52184) breaks build. Additional parameter in TSynCustomFoldHighlighter.StartCodeFoldBlock is not added in TIDESynPasSyn.

Regards
Pascal

[x2nie]

If I am right, the only outstanding change for merging from your git, are in SynPasSyn?

Apparently (yes).


I've big pain when dealing with pas HL's unit test, and you did it. It helps me a lot when you done with unit-test part.



About invalidation (screen update), you've told me three time (or more).
Previously, you mix it with undo/redo. That's why it so hard for me, because learning undo/redo system is a bit out of topic.
But, today, it is being more clear and clean to understand for me.
Therefor, I will try my best again for this.
Also, this part wouldn't disturb what you are doing.

[x2nie]

Let's add more happiness to any new HL development, by enabling sfaOutline by-default.

Let me first finish merging. And see if other bugs turn up that need fixing.

Better to make it avail via Object inspector.

Wait ! do you mean that in future, Object Inspector can be used as Code Explorer ?
if yes, OI would be very useful for both form and code exploration.
I love this idea

[x2nie]

There still is the issue of missing invalidation.

- go to a begin/end block of at least 10 lines.
- position the caret in the line with the "begin",
   but not touching the begin itself.
   Position the caret in the leading spaces of the line.
- now add some spaces.

The begin will indent, so will the horizontal line, but only on the top 3 text lines of the block. The rest will not be redrawn.

I am dejavu.
Yes you are right I can reproduce what you said,
but I forgot that I did fixed it months ago. I just (forget that I) disabled them.


I've committed,
but please ignore any files except the SynEditMarkupFoldColoring.pas
and please ignore all lines except 1 line:

Code: Pascal  [Select][+][-]

1. begin
2.   if EndLine < 0 then exit; //already refreshed by syn
3.   exit;//debug // <------------ I forgot that I disabled it.
4.  
5.  
6.   y := Caret.LineBytePos.y;

becoming this

Code: Pascal  [Select][+][-]

1.  
2.  
3. begin
4.   if EndLine < 0 then exit; //already refreshed by syn
5.  
6.  
7.   y := Caret.LineBytePos.y;

Now, it's not an issue any more.
But it's my old changes, maybe you need to review inside the sub procedures.

[Martin_fr]

Martin_fr,
your last commit (52184) breaks build. Additional parameter in TSynCustomFoldHighlighter.StartCodeFoldBlock is not added in TIDESynPasSyn.

Ups forgot that file in the commit, fixed