Recent

Author Topic: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}  (Read 158015 times)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9794
  • Debugger - SynEdit - and more
    • wiki
Quote
But -1, -1 is the only way to identify a highlighter change as senrHighlightChanged is also called many times for other reasons.
Is called to indicate for which lines the highlighting has changed.
-1,-1 should probably be "all", not sure why it was done that way.

Thats why change of HL object, should have its own notify. For now, it may be easy to solve for markup only. (by a method on markup)

Scan ranges, then notifes, once the HL has scanned. (before that, the node info on the HL is incorrect)

I will look at it...

----------------------
The original idea of TextChanged, was that a markup would simply sot a flag (store the min/max changed line); and only when need for painting it will calculate.

Non sure, but textChanged may be called for lines not even visible. Or (if triggered by user app, and without BeginUpdate):
TextChange may be called for the lines that are visible, but then before they ever get painted, the editor could scroll.
In that case Any work done by TextChange would just be a waste of cpu time, since the result in never painted.

Code: Pascal  [Select][+][-]
  1. SynEdit.TextBetweenPoint[Point(1, SynEdit.TopLine),Point(1, SynEdit.TopLine)] := 'foo';
  2. SynEdit.TopLine := SynEdit.TopLine + 200

But dont worry, if all else is ok, I will apply it, even with actions in TextChanged.
« Last Edit: October 07, 2016, 04:23:03 pm by Martin_fr »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9794
  • Debugger - SynEdit - and more
    • wiki
long time, sorry ....

Still reviewing, but the patches are now certainly better than the existing code, so I will commit it (patch 15 is committed to svn), even if it needs more work (review pending).

---------------------------

FNestList2 isn't needed. And used incorectly.
In AddVerticalLine
Code: Pascal  [Select][+][-]
  1.       FNestList2.Line := ANode.LineIndex;
  2.       l := ToPos(FNestList2.NodeEndLine[0]);
  3.  
returns the endline of the most outer node (usually the unit...end / so end of file)

You were looking for
Code: Pascal  [Select][+][-]
  1.       l := ToPos(FNestList2.NodeEndLine[FNestList2.Count-1]);
Though that depends what node you want, if more than one opens on that line.

Or better (ANodeIdx is the index of the ANode, from the nestlist):
Code: Pascal  [Select][+][-]
  1.       l := ToPos(FNestList.NodeEndLine[ANodeIdx]);
  2.  

I did make those changes, before I commited

------------------------
You may want to check in DoTextChanged (line 731)
Code: Pascal  [Select][+][-]
  1.     if sfaOutline in FoldAction then begin
  2.       FNestList.Line := LineIndex;
  3.       l := ToPos(FNestList.NodeEndLine[FNestList.Count]);
  4.  
that is the most inner node before-or-on the current line, if any. (or crash if none, but I believe the if is only true if there are nodes.

You really should have the index at which the node was found. But then at the time FNestList may have changed....

----------------------
Why is there
SetCacheCount(100);
in create?

----------------------
your detection for shared buffer did no longer work, since the array always has some length (well it would have the length from the previous buffer anyway?)

I fixed that.
----------------------
need to check, that it does nothing, if not enabled (markup.RealEnabled)

also you probably need override RealEnabled, since you got more colors.... (the var should be FColors, not Colors). Not sure what (if any) is stored in MarkupInfo in your case.

Also in RealEnabled check if the Highlighter has support.....

----------------------
I started a test case.
But its only a dummy, to show how it can be done.
Also add only behaviour that is save not to change...
----------------------
more later......
« Last Edit: October 26, 2016, 03:57:13 am by Martin_fr »

guest58172

  • Guest
Two questions:
  • Is it normal that different colors can be used at the same indentation level ?
  • What options are planned ? (For example I wish to have the lines only on blank chars, also I see strange modifications of attributes). I guess that this not yet done.
I've put a few captures of what looks a bit odd, even if I understand that's this new markup is WIP.

Pascal

  • Hero Member
  • *****
  • Posts: 932
Which Highlighter did you use?
laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

guest58172

  • Guest
Which Highlighter did you use?

This one: https://github.com/BBasile/Coedit/blob/master/src/ce_d2syn.pas

Explanations for the screenshots:
  • The two red asterisks are not normal. A range starts with /** and stops with */. A fold could start but here it's deactivated by an option (1). Another range starts with the open-brace. It looks like the red color comes from this range.
  • Exactly the same scenario but here located inside a scope that has created a fold (for example class A {}) The normal color is then orange but after another scope it becomes green.
  • Same as 2. But I wished to show that the blue line could stop one line before (for example with an option.)

(1): https://github.com/BBasile/Coedit/blob/master/src/ce_d2syn.pas#L503

Pascal

  • Hero Member
  • *****
  • Posts: 932
Later on there will be an extension to the configuration of fold coloring to disable individual nodes from coloring (your comments):
Martin_fr posted a patch on mantis http://bugs.freepascal.org/file_download.php?file_id=24855&type=bug with
this extensions.
In your Highlighter you can decide which nodes should be colored and set options:
See TSynPasSyn.DoInitNode in unit SynHighlighterPas and sfaOutline...-FoldActions.
Remove sfaOutline from aActions to disable coloring.
laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

Pascal

  • Hero Member
  • *****
  • Posts: 932
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #291 on: December 02, 2016, 12:44:46 pm »
Martin,

there is an other bug remainig: array FFoldColorInfos in SynEditMarkupFoldColoring is too small/not dynamic
I put a patch on Mantis: http://bugs.freepascal.org/view.php?id=31049
laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

Pascal

  • Hero Member
  • *****
  • Posts: 932
Martin,

could you please commit this patch?
there is an other bug remainig: array FFoldColorInfos in SynEditMarkupFoldColoring is too small/not dynamic
I put a patch on Mantis: http://bugs.freepascal.org/view.php?id=31049

I would like to continue developement to bring the vertical lines to the else-part too.
laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9794
  • Debugger - SynEdit - and more
    • wiki
Something I noted:
Markups are supposed to do NO work, if they are not enabled (or if there colors are ALL off)

For most markups that is covered by the manager, because the GetNext.... functions are not called.

But TextChanged is still called. Many markup set only a flag in there "FFirstLineChanged:=.... FlastLine.....".
That is ok (not much work)

But yours does a lot of work in
Code: Pascal  [Select][+][-]
  1.      procedure TextBufferChanged(Sender: TSynEditStrings; aIndex, aCount: Integer
  2.     procedure DoTextChanged({%H-}StartLine, EndLine, {%H-}ACountDiff: Integer); override; // 1 based
  3.     procedure SetLines(const AValue: TSynEditStrings); override;
  4.     procedure LinesChanged(Sender: TSynEditStrings; aIndex, aCount: Integer);
  5.     procedure HighlightChanged(Sender: TSynEditStrings; aIndex, aCount: Integer);
  6.  

So they should start with
Code: Pascal  [Select][+][-]
  1.   if not ( Enabled and MarkupInfo.IsEnabled) then exit;

Of course then you need to listen to
  DoEnabledChanged
  MarkupChanged
and if you get enabled, then initialize.


Pascal

  • Hero Member
  • *****
  • Posts: 932
Something I noted:
Markups are supposed to do NO work, if they are not enabled (or if there colors are ALL off)

For most markups that is covered by the manager, because the GetNext.... functions are not called.

But TextChanged is still called. Many markup set only a flag in there "FFirstLineChanged:=.... FlastLine.....".
That is ok (not much work)

But yours does a lot of work in
Code: Pascal  [Select][+][-]
  1.      procedure TextBufferChanged(Sender: TSynEditStrings; aIndex, aCount: Integer
  2.     procedure DoTextChanged({%H-}StartLine, EndLine, {%H-}ACountDiff: Integer); override; // 1 based
  3.     procedure SetLines(const AValue: TSynEditStrings); override;
  4.     procedure LinesChanged(Sender: TSynEditStrings; aIndex, aCount: Integer);
  5.     procedure HighlightChanged(Sender: TSynEditStrings; aIndex, aCount: Integer);
  6.  

So they should start with
Code: Pascal  [Select][+][-]
  1.   if not ( Enabled and MarkupInfo.IsEnabled) then exit;

Of course then you need to listen to
  DoEnabledChanged
  MarkupChanged
and if you get enabled, then initialize.

Okay, i will put this in.
laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

Pascal

  • Hero Member
  • *****
  • Posts: 932
I found an other bug and i have no clue what went wrong in my code.
I attached a sample prog. Start it, place the cursor after {$ifdef DEBUG} and press ENTER.
laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9794
  • Debugger - SynEdit - and more
    • wiki
I found an other bug and i have no clue what went wrong in my code.
I attached a sample prog. Start it, place the cursor after {$ifdef DEBUG} and press ENTER.
I have a look in a bit...

Maybe you did not call "clear" on the nested fold list.

If possible this should be added to the testcase first. So the test will fail now, and the test shows that it is fixed.

Pascal

  • Hero Member
  • *****
  • Posts: 932
Maybe you did not call "clear" on the nested fold list.

Code: Pascal  [Select][+][-]
  1.   y := ToIdx(aRow);
  2.   FNestList.Clear;
  3.   FNestList.Line := y;
  4.  

Calling Clear before setting Line did it. I thought setting Line would also clear the NestList.
laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9794
  • Debugger - SynEdit - and more
    • wiki
Calling Clear before setting Line did it. I thought setting Line would also clear the NestList.

the list re-uses info.
If you are at line n, and it scans back 100 lines to find outer nodes => then if you set line n+1 it can still use the info from the 100 scanned lines.
So that reduces a lot of work.

But if the HL changed (if text changed, and HL scanned) then you need to clear.

You should clear, ONCE, at the first line that is painted. Following lines should only set the number (or it can get a lot slower).

Maybe best to introduce "BeginMarkup" same as "FinishMArkup" but called before the very first line.

Pascal

  • Hero Member
  • *****
  • Posts: 932
You should clear, ONCE, at the first line that is painted. Following lines should only set the number (or it can get a lot slower).

Maybe best to introduce "BeginMarkup" same as "FinishMArkup" but called before the very first line.

I already had it in EndMarkup:
Code: Pascal  [Select][+][-]
  1. procedure TSynEditMarkupFoldColors.EndMarkup;
  2. begin
  3.   {$IFDEF SynEditMarkupFoldColoringDebug}
  4.   //DebugLn('EndMarkup');
  5.   {$ENDIF}
  6.   inherited EndMarkup;
  7.   if not Assigned(FHighlighter) then exit;
  8.   FNestList.Clear; // for next markup start
  9. end;
  10.  

I also tried it in the HighlighChanged handler: no luck.

The place where it seems to work is DoTextChanged. Could this be okay to clear it there?

laz trunk x64 - fpc trunk i386 (cross x64) - Windows 10 Pro x64 (21H2)

 

TinyPortal © 2005-2018