Recent

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

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
about the files you changed from original.

Does your git has the unmodified versions too, so on could simply compare against it?

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Some of the older changes:

Code: Pascal  [Select]
  1.     FIsCollectingNodeInfo: boolean;
  2.     FFoldNodeInfoList: TLazSynFoldNodeInfoList;
  3.     FCollectingNodeInfoList: TLazSynFoldNodeInfoList; //x2nie not sure if it is needed as here will be 2 TLazSynFoldNodeInfoList
  4.  

There should only be one list.

FIsCollectingNodeInfo should be available through read only property, so pas HL does not need its own.
It then must be set, and cleared (try finally) in the base class.


Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Quote
Code: Pascal  [Select]
  1. LogX2 := LogX1 + L -1;
But...
the MarkupWordGroup seem as wrong on Node.LogXEnd, mean conflict with my idea.
See the "food"~"/food"  vs  "repeat"~"until" screengrab.

IIRC (need to check) end is the begin of next token.
Think of the pos as pointing between two chars, not the start of a char.

Code: Pascal  [Select]
  1. foo bar
foo starts at 1
last char of foo is (starts at) 3
foo ends at 4
space starts at 4

IIRC that is common on many parts of SynEdit.

Keep MWG, by above logic.
Btw, this is also where the caret would be, if it was at the end of foo.


Quote
please ignore my previous question
too late



Quote
Now, the problem is when text/line trailing with tab. The vertical lines drawn improperly.
Either:
1)  they are past end of line. But selection (in column mode) can do it, so there must be a way.

2) logical vs physical http://wiki.lazarus.freepascal.org/SynEdit#Logical.2FPhysical_caret_position
if there are #9, or multybyte utf8 (umlauts äüö, accents, Chinese, ....)

You need to store the phys pos, as that is the same in all lines. logical changes.

Need to check later




Hey, I currently face a real problem ... >:D  !
Quote
This messy visual effect, can be solved by drawing all vertical lines,  straight to "end;" keyword vertically.
We can see how CnPack solves the same problem. (see attachment II )

relates how to the quote of mine that you posted before?

see my comment above.

It should work with the frame for now. There may be shlight changes, but they would still need to go line by line

cnpack probably paints on top of all (and I have seen pictures where the yellow "begin" was painted with an offset to the original....)

But in Lazarus SynEdit composes colors by priority. A user can define another markup (eg selection) to have higher priority. All other markups are evaluated per line, so yours must be too.




Quote
... plus : if it still in 3th depth of folds, comparison will be repeated 3 times again.
That is why this code (searching end line) should be added to nested fold list. there known info can be cached and reused.

Quote
For honest, the longest procedure AFAIK would be only 200 lines including off-of-screen lines.  (sorry for 10K abuse).
But, couldn't we make this real solution more efficient?
Yes, but not by storing on the range.
see above: cache in the list.

I have some ideas there, but I need more time to evaluate and explain.

In the nested list, if you change the line, it can still keep the previous data as cache, and only calculate until it can use the cache.

This cache would be per module, It will work well just for your markup, and it will work well for other markup, but it will not add synergy between markups. For that another cache may be needed, but that can be done later.

Quote
But, because MarkupColorFold will operate per sourcecode's line basis,
the next sourcecode-line will repeat comparing the left most pair of that begin~end (that have already proir calculated)
... plus : if it still in 3th depth of folds, comparison will be repeated 3 times again.
Not if cached in the nested list. Also I have to look at your code again. I need to see if the invalidate issue was truly fixed.
And depending on this other changes may make this partly obsolete.

Quote
Assumed that System.pas has 10K lines, it should have 10K states stored, right? because 1line = 1 states.
As I wrote a few posts back: No.

Example pas HL, it will probably have 1500 range objects for the 10k lines. (and the ratio gets better, if you have 100k lines).
For each line it has a pointer entry, that points to one of those objects.

And that is the reason they must be immutable, because several lines use them, if you changed it, that has side effects on other lines.

Quote
Hence, on file loading, when a 'begin' of procedure block founded, I can store that begin postion.
when 'end' of that procedure founded, I stored that in the shared-folding-block (whatever Range / CodeFoldBLock?).
See above, you need a lot more range objects (and folding blocks, the 2 go together)

You would also need to fix, that very long ago (maybe even before synedit was added to Lazarus) someone decided that range objects are only freed when the HL is freed. They are kept, even if they no longer apply to any line.
Currently this is at best a very minor issue, but the more of them are created, the bigger it gets.

You still may need to find the end line. it could contain an "invalid flag" or other info. Unless you store a copy of the entire node info record on the fold block. But that would be a lot of data, AND also mean you have to calculate all this data during every scan, even for lines that are currently not visible.
Not tested, but try (forward declaration):
Code: Pascal  [Select]
  1.   type TFoo = class
  2.   // 10 lines
  3.   ;
  4.  
An maybe also "class of".

If the search is improved correctly, then you do only one scan independent on nesting depth, and the data is re-used for the next (and further) line(s).

And all the search will be abstracted in the nested list. So any optimization can be done now or later, it does no longer affect your code. Your code asks the list. the list can get it whatever way, and the way can even be changed.

Quote
If this concept is negative, how can an HL rescan (reparsing) a line randomly?
For rescanning the x pos is not needed. The xpos is only needed outside the HL.

Quote
But it most probably impossible, when range (or foldblock?) is reused rewriten unpredicably.
it is re-used (not unpredictable though).



Overall, I do see what you mean. I don't currently think it is the way to go.
In any case if the problem can be abstracted into a few re-usable helper functions, then it can be changed later if indeed needed.

--------
EDIT
Also adding more data to the HL, one needs to look at who/what benefits.
Some users use the HL only for highlight, no fold, no markup. Then the data is wasted.
Of course then for some HL the fold level is wasted too. But that would be all the more reason not to add to it (pascal needs parts of the fold info, because some symbols change. For examle "of" looks at the foldtype, "case of" means a label is next, but only in  a case block. Also "Cdecl", "final" and others need them.


-----
EDIT2:

I am NOT asking you to write the cache code etc. (you are welcome, but it would be a separate project)

Just add to the TLazSynEditNestedFoldsList:
Code: Pascal  [Select]
  1.     property EndNodeLine[Index: Integer]: Integer read GetEndNodeLine;        
  2.  
that does the loop on the foldlevel.

So that EndNodeLine[n] returns the line that matches the open-node in NodeLine[n]

Ideally, if EndNodeLine[n-x]  is known, continue from there.

then maybe also add
Code: Pascal  [Select]
  1. property HLEndNode[

same as HLNode just the closing.
« Last Edit: December 12, 2015, 02:40:38 am by Martin_fr »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
To start moving some code to svn.
Code: Pascal  [Select]
  1. Also, do you still change the "exit(-1)" to 0 in the nodelist (if not valid).
  2.  
  3. Some code currently relies on -1 (the example foldhl will not fold, if it is 0)
  4.  
  5. I agree the -1 is bad. So it is also possible to add a property IsValid and change all code that depends on it.
  6.  
  7. So one of 2 choices:
  8. 1) Make your code work with -1
  9. 2) add IsValid. This must be a standalone patch (or one separate commit of all involved changes in your git). Then it can be merged as a standalone svn commit.


And LogVertGuideX ?

Then I can extract the code, and merge it.





Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
LogVertGuideX

1) I wouldnt make it part of the getter.
If a HL needs it, then it must override initializeNode (or what the name is).

2) I can see the xml hl needs more than the token.

But does it not also need an end?
Code: XML  [Select]
  1. <foo val=1>
what would be highlighted?

Also the name needs to be more generic. Other markup may use it too.


Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Quote
Hence, on file loading, when a 'begin' of procedure block founded, I can store that begin postion.
when 'end' of that procedure founded, I stored that in the shared-folding-block (whatever Range / CodeFoldBLock?).

In addition to my previous reply. This info would not be useful, not even to your code.

The HL does by design only know logical positions. Changing this would be changing almost the entire design of SynEdit.

But for painting you need the physical pos. To get that you need the actual lines (line with begin and line with end). And to find the lines you still need to do the search on foldlevel.

And no: you cant store the linenumbers of those lines. If you do you must update all ranges if a line is inserted or deleted (at top of file). Currently you only scan the inserted line (in most cases)

EDIT:
In fact it is not possible to have phys pos in HL. if 2 synedit are linked, they share the HL, and the share the same ranges. But each SynEdit can have different tab settings, meaning different phys pos for the same log pos
« Last Edit: December 12, 2015, 07:09:14 pm by Martin_fr »

x2nie

  • Sr. Member
  • ****
  • Posts: 478
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Okay, here is my today update:
* IsCollectingNodeInfo has been set as readonly property (currently no affect to pas HL)
* FNodeInfoList hasbeen deleted, in performing of new name (similar purpose) of FCollectingNodeInfoList,
   I think the new one is more intuitive.
* LogVertGuideX has been deleted. Its not really needed, so far.
   As For XML, the difference between tag location and vertical guide, possibly can be adjusted in XML HL,, later.





Also, do you still change the "exit(-1)" to 0 in the nodelist (if not valid).

Some code currently relies on -1 (the example foldhl will not fold, if it is 0)

I agree the -1 is bad. So it is also possible to add a property IsValid and change all code that depends on it.

So one of 2 choices:
1) Make your code work with -1
2) add IsValid. This must be a standalone patch (or one separate commit of all involved changes in your git). Then it can be merged as a standalone svn commit.
This is weekend, I am at home with my LiteZarus (compatible = Lazarus 1.5).
still using with  "-1", the demo is fine without error,
but when integrating with IDE, there are error every time.
I am not sure about it now, probably you can test in your Lazarus 1.7 ?


As for merging my contribution (SynEditHighlighterFoldBase) into lazarus svn ,
1) so far so good. One I worried about is FConfigFold things. I added a guard (check if nil else) but I never test it yet.
2) sorry, I can provide files *.pas, no patches. If you really want to see diff, just replace / copy-paste them into your lazarus codes.

When you were logged in, you can see attachments.
Lazarus Trunk @ Windows7 64bit, XP 32bit, Debian under VirtualMachine

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Quote
2) sorry, I can provide files *.pas, no patches. If you really want to see diff, just replace / copy-paste them into your lazarus codes.

I can work from your git.

But if you add modification to more files, please commit the unmodified copy first, then I can diff against that.

x2nie

  • Sr. Member
  • ****
  • Posts: 478
  • Impossible=I don't know the way
    • impossible is nothing - www.x2nie.com
Quote
2) sorry, I can provide files *.pas, no patches. If you really want to see diff, just replace / copy-paste them into your lazarus codes.

I can work from your git.

But if you add modification to more files, please commit the unmodified copy first, then I can diff against that.
Oh? :o  okay, I will. I just didn't think that it was important for people (other than me).
Also, it was a bit difficult for me (copying unmodified files first). but now I have another idea to automate that. So don't worry, if I did that accidently, git support the "undo commit"  8)  I can resolve them.


Do you still need unmodified files available before any changes made to them? if so, I can "git rebase" to re-flow my commit hystories.  :-\ :D ;D
When you were logged in, you can see attachments.
Lazarus Trunk @ Windows7 64bit, XP 32bit, Debian under VirtualMachine

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
I use a local merge tool now.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #100 on: December 12, 2015, 10:59:53 pm »
while merging I noted a few things:

1) my fault, we need 2 variables for 2 lists, I put them back already

2) TSynCustomFoldHighlighter.CollectNodeInfo
Removing comments,
Removing sfaOpenFold, sfaCloseFold => deprecated
Code: Pascal  [Select]
  1.     if not LevelChanged then
still needed? this should be covered by blockenabled. And not sure if a good idea if only on closing.

3) I need to review IncreaseFoldLevel, and the 2nd depth counter.
More on that later

4) sfaMarkup needs to be added to each foldconfig in GetFoldConfigInstance of each HL.


Code: Pascal  [Select]
  1. procedure TSynCustomFoldHighlighter.CollectNodeInfo(FinishingABlock: Boolean;
  2.   ABlockType: Pointer; LevelChanged: Boolean);
  3. var
  4.   //DecreaseLevel,
  5.   BlockTypeEnabled: Boolean;
  6.   act: TSynFoldActions;
  7.   BlockType: Integer;
  8.   nd: TSynFoldNodeInfo;
  9. begin
  10.   if not IsCollectingNodeInfo then exit;
  11.  
  12.   BlockTypeEnabled := False;
  13.   if (ABlockType <> nil) and (PtrUInt(ABlockType) < FoldConfigCount) then
  14.     BlockTypeEnabled := FFoldConfig[PtrUInt(ABlockType)].Enabled;
  15.  
  16.   //Start
  17.   if not FinishingABlock then
  18.   begin
  19.     act := [sfaOpen, sfaOpenFold]; // todo deprecate sfaOpenFold
  20.     if BlockTypeEnabled then
  21.       act := act + FFoldConfig[PtrUInt(ABlockType)].FoldActions
  22.     else
  23.       act := act + [sfaFold];
  24.     DoInitNode(nd, FinishingABlock, ABlockType, act, True);
  25.   end
  26.   else
  27.   //Finish
  28.   begin
  29.     act := [sfaClose, sfaCloseFold]; // todo deprecate sfaCloseFold
  30.     if BlockTypeEnabled then
  31.       act := act + FFoldConfig[PtrUInt(BlockType)].FoldActions
  32.     else
  33.       act := act + [sfaFold];
  34.     if not LevelChanged then
  35.       act := act - [sfaFold, sfaFoldFold, sfaFoldHide];
  36.     act := act - [sfaFoldFold, sfaFoldHide]; // it is closing tag
  37.     DoInitNode(nd, FinishingABlock, ABlockType, act, LevelChanged);
  38.   end;
  39.  
  40.   FCollectingNodeInfoList.Add(nd);
  41. end;
  42.  

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #101 on: December 12, 2015, 11:03:36 pm »
btw, in your sample:
- indent a "begin" (vert line moves)
- cursor down 10 lines
- undo

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #102 on: December 12, 2015, 11:26:57 pm »
Now here is the issue (and I didnt realize it before)

When you copies PasFoldLevel that was actually right (-ish), but not because the value is needed by the markup.


Now in pas hl we have 2 counters
1) NESTED for all nodes, even if you cant fold the node. (e.g. cfbtIfThen)
2) FOLD only those you can fold

This gives the fold module a slight advance on the search.
If a line contains an "if" and therefore a none foldable node, the 2nd of the 2 levers does not change. The search skips over that line.

The markup needs to seach such lines too, because a markup line may not be foldable.

Now one could argue: Add a MarkupLevel too.
Then that is 3 levels (if all markups share a level, even if they do not share all nodes, otherwise it is even more)

Probably in future other subsets of nodes want there level too. So then 4 or 5 levels.
Each new combination of levels creates a new range. ouch. Not a good way to go.

So for now (until the parsing code relying on it can be fixed: keep the 2 levels.
Actually the (1) "NESTED all" does not matter. It equals the depth of the tree via the parent. So it never creates a range that does not exist already anyway.

EDIT: except the associated "min level" creates new nodes....


Later:
On the long run the "FOLD" can be removed. The search then has to look at the TSynCustomCodeFoldBlock.BlockType. (and the foldconfig). This still works without scanning the line. (even improves current markup search)

If the blocktype is configured for the correct action then the line is a match.
« Last Edit: December 13, 2015, 07:02:49 pm by Martin_fr »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #103 on: December 13, 2015, 07:36:15 pm »
some mem leaks / there are more

Code: Diff  [Select]
  1. diff --git a/syneditmarkupfoldcolors.pas b/syneditmarkupfoldcolors.pas
  2. index bec8f81..72b4e1d 100644
  3. --- a/syneditmarkupfoldcolors.pas
  4. +++ b/syneditmarkupfoldcolors.pas
  5. @@ -338,6 +338,7 @@ begin
  6.        inc(i);
  7.        //dec(i);
  8.    end;
  9. +  Nest.Free;
  10.  end;
  11.  
  12.  procedure TSynEditMarkupFoldColors.PrepareMarkupForRow(aRow: Integer);
  13. diff --git a/syngutterfolddebug.pas b/syngutterfolddebug.pas
  14. index 8844ded..d9f622e 100644
  15. --- a/syngutterfolddebug.pas
  16. +++ b/syngutterfolddebug.pas
  17. @@ -153,6 +153,7 @@ var
  18.  
  19.    finally
  20.      TextDrawer.EndDrawing;
  21. +    nest.Free;
  22.    end;
  23.  
  24.  end;
  25.  
  26.  

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5801
    • wiki
Re: Invite to Colorizing TSynEdit ! {Improving Editor for Editing Complex Codes}
« Reply #104 on: December 13, 2015, 09:36:48 pm »
ok, so I merged TSynCustomFoldHighlighter to svn.

Quote
Revision: 50779
Message:
SynEdit: add FoldNodeInfo to TSynCustomFoldHighlighter / prepare new markups. Patch by x2nie
Author: martin
Date: 13 December 2015 20:36:29
----
Modified : /trunk/components/synedit/synedithighlighterfoldbase.pas

I restored having 2 fields (after I incorrectly said only one was needed), I restored the 2nd foldlevel (as it will be needed for now)

The " if not LevelChanged then" is not needed. Checking the code in pas hl, it should be the same as foldblock.enabled and foldblock.actions.


This will also mean the following can thanks to you get resolved
http://bugs.freepascal.org/view.php?id=23016