Recent

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

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5766
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #270 on: September 24, 2016, 03:24:41 pm »
InitArrays is also the wrong place for
Code: Pascal  [Select]
  1.   if Assigned(FHighlighter) then begin
  2.     FNestList := TLazSynEditNestedFoldsList.Create(Lines, FHighlighter);
  3.  

That code belongs ONLY into TSynEditMarkupFoldColors.HighlightChanged

There is a copy in Create, but I wonder, can create just call HighlightChanged?

--------------
Why does SetLines do FreeAndNil(FNestList); ?

FNestList.Lines needs to be updated in SetLines
« Last Edit: September 24, 2016, 03:31:39 pm by Martin_fr »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5766
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #271 on: September 24, 2016, 03:46:09 pm »
You are using logical positions http://wiki.lazarus.freepascal.org/SynEdit#Logical.2FPhysical_caret_position

That causes the tab misalign.....

You should use Phys x pos.

You need to convert the first none space, into phys, and then on each line use that.
(for keywords (frame) you can use either log, or pos. Since they are for one line only, so the pos can not shift)

(It will not fix all problems, but it is a start)
« Last Edit: September 24, 2016, 04:02:15 pm by Martin_fr »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5766
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #272 on: September 24, 2016, 04:11:03 pm »
Code: Pascal  [Select]
  1.  procedure AddVerticalLine(
  2. ....
  3.     z := Length(FFoldColorInfos);
  4.     SetLength(FFoldColorInfos, z+1);
  5.  
Better to set the lenght to the full needed lengh at start.

That should be "NestCount := FNestList.Count;"?

Well I guess up to NestCount. It can be less.
But then set it to maximum, and reduce at the end.

Or better yet, keep it at maximum (and also keep for next line) and store the real used len in a separate variable.


Pascal

  • Hero Member
  • *****
  • Posts: 832
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #273 on: September 25, 2016, 08:17:30 am »
Code: Pascal  [Select]
  1. procedure TSynEditMarkupFoldColors.InitArrays;
  2. ....
  3.   Lines.AddChangeHandler(senrLineCount, @LinesChanged);
  4.   Lines.AddChangeHandler(senrHighlightChanged, @HighlightChanged);
  5.  

This is called from TSynEditMarkupFoldColors.SetLines (where it is matched by remove calls.)
But also from other locations. (Afaik duplicates are ignored, but still....)

InitArray is only called once It's the fix for cloned Buffers as SetLines isn't called for the clone SynEdit


Also
Code: Pascal  [Select]
  1. constructor TSynEditMarkupFoldColors.Create(ASynEdit: TSynEditBase);
  2. begin
  3.   inherited Create(ASynEdit);
  4.  
  5.   if Assigned(Lines) then begin
  6.     Lines.AddChangeHandler(senrLineCount, @LinesChanged);
  7. ....
  8.  
It can never be assigned.

This code should only be in SetLines and Destroy (a remove call is ignored, if the handler wasn't added.

Okay. So lines is always nil when constructing. I will remove this. This code was there before i began.
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Pascal

  • Hero Member
  • *****
  • Posts: 832
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #274 on: September 25, 2016, 08:21:26 am »
InitArrays is also the wrong place for
Code: Pascal  [Select]
  1.   if Assigned(FHighlighter) then begin
  2.     FNestList := TLazSynEditNestedFoldsList.Create(Lines, FHighlighter);
  3.  

That code belongs ONLY into TSynEditMarkupFoldColors.HighlightChanged

There is a copy in Create, but I wonder, can create just call HighlightChanged?

--------------
Why does SetLines do FreeAndNil(FNestList); ?

FNestList.Lines needs to be updated in SetLines

It is there because TLazSynEditNestedFoldsList.Create(Lines, FHighlighter) depends on both Lines and Highlighter!
It has to be called when one of them changes.
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Pascal

  • Hero Member
  • *****
  • Posts: 832
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #275 on: September 25, 2016, 08:23:37 am »
You are using logical positions http://wiki.lazarus.freepascal.org/SynEdit#Logical.2FPhysical_caret_position

That causes the tab misalign.....

You should use Phys x pos.

You need to convert the first none space, into phys, and then on each line use that.
(for keywords (frame) you can use either log, or pos. Since they are for one line only, so the pos can not shift)

(It will not fix all problems, but it is a start)

Okay. I never used tabs. I wil fix this.
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Pascal

  • Hero Member
  • *****
  • Posts: 832
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #276 on: September 25, 2016, 08:33:50 am »
Code: Pascal  [Select]
  1.  procedure AddVerticalLine(
  2. ....
  3.     z := Length(FFoldColorInfos);
  4.     SetLength(FFoldColorInfos, z+1);
  5.  
Better to set the lenght to the full needed lengh at start.

That should be "NestCount := FNestList.Count;"?

Well I guess up to NestCount. It can be less.
But then set it to maximum, and reduce at the end.

Or better yet, keep it at maximum (and also keep for next line) and store the real used len in a separate variable.

I also thought about this as the array gets copied every time. I will fix this.
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5766
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #277 on: September 25, 2016, 02:14:54 pm »
Quote
InitArray is only called once It's the fix for cloned Buffers as SetLines isn't called for the clone SynEdit
When the buffer was replaced from a shared editor, then there is no need for AddChangeHandler (handlers are copied to the new buffer)

Quote
It is there because TLazSynEditNestedFoldsList.Create(Lines, FHighlighter) depends on both Lines and Highlighter!
It has to be called when one of them changes.

You may have to do an FNestList.Clear
And FNestList.Lines := Lines // Despite lines do not change in todays SynEdit.


And in TSynEditMarkupFoldColors.HighlightChanged
  FNestList.Highlighter := FHighlighter;


Maybe rename it to TextBufferChanged?



Pascal

  • Hero Member
  • *****
  • Posts: 832
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #278 on: September 25, 2016, 02:27:50 pm »
Quote
InitArray is only called once It's the fix for cloned Buffers as SetLines isn't called for the clone SynEdit
When the buffer was replaced from a shared editor, then there is no need for AddChangeHandler (handlers are copied to the new buffer)

So i only have to init the arrays, right? Is there a way to store them with the lines, so that the clone can also use it?
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5766
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #279 on: September 25, 2016, 02:44:32 pm »
So i only have to init the arrays, right? Is there a way to store them with the lines, so that the clone can also use it?

And maybe clear the nestlist, probably not needed, but wont hurt.

Look at unit SynEditTextTabExpander class TSynEditStringTabData

No it is not automatically copied to the new textbuffer (the whole shared buffer needs some improvments.
But it will automatically grow/shrink if lines are inserted / deleted.

Not sure if it is a big advantage right now. Maybe leave it.

Pascal

  • Hero Member
  • *****
  • Posts: 832
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #280 on: September 29, 2016, 02:15:23 am »
Added new patch. Fixed most but not all (Recreation of FNestList is still done on Highlighter change).

Which editor properties did you change to get the range check error? I couldn' reproduce it.
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

Pascal

  • Hero Member
  • *****
  • Posts: 832
I fixed the last points of your test (http://bugs.freepascal.org/file_download.php?file_id=25124&type=bug) and i found a possible problem:

When i change the highlighter it does a ScanRange and notifies all Markups of changed text before they even know about the new highlighter!

Code: Pascal  [Select]
  1. procedure TCustomSynEdit.SetHighlighter(const Value: TSynCustomHighlighter);
  2. begin
  3.     ...
  4.     fHighlighter := Value;
  5.     IncPaintLock;
  6.     try
  7.       // Ensure to free all copies in SynEit.Notification too
  8.       fMarkupHighCaret.Highlighter := Value;
  9.       fMarkupWordGroup.Highlighter := Value;
  10.       FFoldedLinesView.Highlighter := Value;
  11.       FPaintArea.Highlighter := Value;
  12.       FWordBreaker.Reset;
  13.       if fHighlighter<>nil then begin
  14.         fTSearch.IdentChars := fHighlighter.IdentChars;
  15.         FWordBreaker.IdentChars     := fHighlighter.IdentChars;
  16.         FWordBreaker.WordBreakChars := fHighlighter.WordBreakChars;
  17.       end else begin
  18.         fTSearch.ResetIdentChars;
  19.       end;
  20.       RecalcCharExtent;
  21.       ScanRanges; // Todo: Skip if paintlocked <------------------- ScanRanges calls TextChanged for all Markups (see below)
  22.       // There may not have been a scan
  23.       if fHighlighter <> nil then
  24.         FHighlighter.CurrentLines := FLines;
  25.       FLines.SendNotification(senrHighlightChanged, FLines, -1, -1); <--------------- notify about new highlighter
  26.     finally
  27.       DecPaintLock;
  28.     end;
  29.   end;
  30. end;
  31.  

Code: Pascal  [Select]
  1. procedure TCustomSynEdit.ScanRanges(ATextChanged: Boolean = True);
  2. begin
  3.   ...
  4.   // Todo: text may not have changed
  5.   if ATextChanged then
  6.     fMarkupManager.TextChanged(FChangedLinesStart, FChangedLinesEnd, FChangedLinesDiff);  <------- notify Markups of changed Text (FChangedLinesStart is 0 here)
  7.   TopView := TopView;
  8. end;

So maybe the notify should be done before ScanRange !? Or call ScanRange(false).

I could handle this situation as StartLine is 0 in DoTextChanged virtual procedure (as it should be 1 based). So i exit DoTextChanged
when i get a StartLine of 0.
« Last Edit: October 05, 2016, 10:37:59 pm by Pascal »
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)

x2nie

  • Sr. Member
  • ****
  • Posts: 478
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
...

Code: Pascal  [Select]
  1. procedure TCustomSynEdit.SetHighlighter(const Value: TSynCustomHighlighter);
  2. begin
  3. ...
  4.       ScanRanges; // Todo: Skip if paintlocked <------------------- ScanRanges calls TextChanged for all Markups (see below)
  5. end;
  6.  

Code: Pascal  [Select]
  1. procedure TCustomSynEdit.ScanRanges(ATextChanged: Boolean = True);
  2. begin
  3.   if ATextChanged then
  4.     fMarkupManager.TextChanged(FChangedLinesStart, FChangedLinesEnd, FChangedLinesDiff);  <------- notify Markups of changed end;

So maybe the notify should be done before ScanRange !? Or call ScanRange(false).
I think the ScanRange(False);         will solve the potentially a problem.
When you were logged in, you can see attachments.
Lazarus Trunk @ Windows7 64bit, XP 32bit, Debian under VirtualMachine

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5766
    • wiki
Yes it seem ScanRanges(False) should be correct. I cant think of any current markup that would break....

Quote
So maybe the notify should be done before ScanRange !? Or call ScanRange(false).
Looking at the code....

It seems that
Code: Pascal  [Select]
  1. FLines.SendNotification(senrHighlightChanged, FLines, -1, -1);
may never have been meant as "the highlighter was replaced"

It may go with the comment
Code: Pascal  [Select]
  1. // There may not have been a scan
meaning, it simply simulates the event for a full scan of the HL (in case it is needed)
In that case the order is correct, or does at least no matter.

Something else:
Code: Pascal  [Select]
  1.       fMarkupHighCaret.Highlighter := Value;
  2.       fMarkupWordGroup.Highlighter := Value;
  3.  
SynEdit shouldnt need to know individual markups.

One way would be to introduce senrHighlighterClassChanged.
But easier will be to put HighlighterChanged() the base class, and call MarkupManager.HighlighterChanged, that will call all Markups.
Any Markup in need of a HL, can react (the 2 existing Markup can retrieve the HL)

Pascal

  • Hero Member
  • *****
  • Posts: 832
It seems that
Code: Pascal  [Select]
  1. FLines.SendNotification(senrHighlightChanged, FLines, -1, -1);
may never have been meant as "the highlighter was replaced"

But -1, -1 is the only way to identify a highlighter change as senrHighlightChanged is also called many times for other reasons.
 
Something else:
Code: Pascal  [Select]
  1.       fMarkupHighCaret.Highlighter := Value;
  2.       fMarkupWordGroup.Highlighter := Value;
SynEdit shouldnt need to know individual markups.

One way would be to introduce senrHighlighterClassChanged.
But easier will be to put HighlighterChanged() the base class, and call MarkupManager.HighlighterChanged, that will call all Markups.
Any Markup in need of a HL, can react (the 2 existing Markup can retrieve the HL)

You can also modify those markups so that they register a notification handler for senrHighlightChanged (like TSynEditMarkupFoldColors does).
laz trunk - fpc trunk 32bit - Windows 10 Pro x64 (1803)