Hi,

Consider this piece of code.
On a form have a TSynEdit (named: Ed) and a TSpeedButton.

procedure TForm1.SpeedButton1Click(Sender: TObject);
var
  BB, BE: TPoint;
  BackwardsSel: Boolean;
  SelLength: Integer;
const
  Pre = '{';
  Post = '}';
begin
  writeln('===================');
  writeln;
  with Ed do
  begin
    BB := BlockBegin;
    BE := BlockEnd;
    BackwardsSel := IsBackwardSel;
    SelLength := SelEnd-SelStart;
    writeln('BlockBegin  =',bb.x,',',bb.y);
    writeln('BlockEnd    =',be.x,',',be.y);
    writeln('SelLength   =',SelLength);
    writeln('BackwardsSel=',BackwardsSel);
    //BB is always before BE even if IsBackwardSel=TRUE
    //move caret to the end of the selection
    LogicalcaretXY := BlockEnd;
    //SetLogicalCaret clears the selection, so restore it
    BlockBegin := BB;
    BlockEnd := BE;
    BB := BlockBegin;
    BE := BlockEnd;
    writeln('After SetLogicalCaret');
    writeln('BlockBegin  =',bb.x,',',bb.y);
    writeln('BlockEnd    =',be.x,',',be.y);
    //Insert Pre at blockbegin, so in front of the selection: block
moves to the right, blockbegin.x and blockend.x will increase
    SetTextBetweenPoints(BB, BB, Pre, [setMoveBlock], scamIgnore,
smaMoveUp, smNormal);
    BB := BlockBegin;
    BE := BlockEnd;
    writeln('After SetTextBetweenPoints(BB,BB,...)');
    writeln('BlockBegin  =',bb.x,',',bb.y);
    writeln('BlockEnd    =',be.x,',',be.y);
    //Insert Post at blockend, the block does NOT move, so this should
NOT affect the value of blockbegin or blockend
    SetTextBetweenPoints(BE, BE, Post, [setMoveBlock], scamIgnore,
smaMoveUp, smNormal);
    //Now the caret needs to be moved to BlockBegin, but unfortunately
this unselects the selection
    //so we save where it ends, then restore it
    BB := BlockBegin;
    BE := BlockEnd;
    writeln('After SetTextBetweenPoints(BE,BE,...)');
    writeln('BlockBegin  =',bb.x,',',bb.y);
    writeln('BlockEnd    =',be.x,',',be.y);
    if not BackwardsSel then
      LogicalCaretXY := BlockEnd
    else
      LogicalCaretXY := BlockBegin;
    BlockBegin := BB;
    BlockEnd := BE;
    writeln('After SetLogicalCaret');
    writeln('BlockBegin  =',bb.x,',',bb.y);
    writeln('BlockEnd    =',be.x,',',be.y);
  end;
  writeln;
  writeln('===================');
  writeln;
end;

It's intention is to put Pre in front of the current selection and Post after it.
The original selected text should still be selected after it, and the caret must be at the end of the selected text.

Build and run.

Now in an empty synedit type a single character e.g. '1' (at point (1,1)) Run the code.

It works as expected.
The text is now {1} and the 1 is selected.

Now clear the synedit again.
Run the code again.
This time the text will be {} (as expected), but the entire {} will be selected.

Here's the debug output:

Case 1 (1 character at (1,1) selected):

BlockBegin =1,1
BlockEnd =2,1
SelLength =1
BackwardsSel=FALSE
After SetLogicalCaret
BlockBegin =1,1
BlockEnd =2,1
After SetTextBetweenPoints(BB,BB,...)
BlockBegin =2,1
BlockEnd =3,1
After SetTextBetweenPoints(BE,BE,...)
BlockBegin =2,1
BlockEnd =3,1
After SetLogicalCaret
BlockBegin =2,1
BlockEnd =3,1

Case 2 (no text selected)
BlockBegin =1,1
BlockEnd =1,1
SelLength =0
BackwardsSel=FALSE
After SetLogicalCaret
BlockBegin =1,1
BlockEnd =1,1
After SetTextBetweenPoints(BB,BB,...)
BlockBegin =1,1
BlockEnd =2,1
After SetTextBetweenPoints(BE,BE,...)
BlockBegin =1,1
BlockEnd =3,1
After SetLogicalCaret
BlockBegin =1,1
BlockEnd =3,1

===================

When text is selected, SetTextBetweenPoints will move the block to the right if we insert text before it and adjust BlockBegin and BlockEnd accordingly.

However when no text is selected, BlockBegin. remains unchanged, but BlockEnd is incremented, which IMO is wrong: BlockBegin.X should be incremented by the amount dicated by the contents of Pre (in this case BlockBegin.X should be incremented by 1).

Is this a bug (in SetTextBetweenPoints?) or do I need to adjust my code for the case where no text is selected?

Additionally, if you run this code on a SynEdit that has never had any text in it (e.i. create it at run time, Lines.Count will be zero), then SetTextBetweenPonits will NOT adjust BlockBegin and BlockEnd at all: they will remain (1,1) at all times.

I added 2 fixes (one for the other mail / empty edit). Please test.

The first fix can be merged, the 2nd should get some more testing as the BlockBegin points to a none existing line (and that should not be allowed). Also do not rely on the current new implementation, that will make sure BB.y is never 0.

If there is no selection, then BB and BE should end up after {}, as both inserts are treated as before the selection.

PM Martin Frb wrote

I added 2 fixes (one for the other mail / empty edit). Please test.

I think the fix for empty selection is allright.
It will not completely solve my issue (the second SetTextBetweenPoints also moves BlockEnd), which makes sense I guess, since in essence I want BlockEnd to "move" with the first call, but remain with the second call, and in each call BlockBegin=BlockEnd.

I adjusted my code to deal with that (if no text is selected I restore BB and BE as needed).

The situation with Lines.Count=0 even got worse: it now places Post in front of Pre, so you get '}{' instead of "{}'.
As a side effect of that, the caret is in between the } and { (which is where I wanted them to be ;) )

Here's the debug output for that situation:

BlockBegin =1,1
BlockEnd =1,1
SelLength =0
BackwardsSel=FALSE
Lines.Count =0
After SetLogicalCaret (LogicalCaretXY := Blocken, which is 1,1) BlockBegin =1,0 <<====== ???
BlockEnd =1,1
After SetTextBetweenPoints(BB,BB,...)
BlockBegin =1,1
BlockEnd =1,1
After SetTextBetweenPoints(BE,BE,...)
BlockBegin =2,1
BlockEnd =2,1
After SetLogicalCaret
BlockBegin =2,1
BlockEnd =2,1

Well, yes. I tested with all the SetCaret and SetBlock.... disabled. I will have to look into that.

The block pos is limited to existing lines. And if line 1 does not exist..... argh

PM Martin Frb wrote

The block pos is limited to existing lines. And if line 1 does not exist..... argh

TSynEditSelection.SetStartLineBytePos should not allow values of X/Y < 1?

The problem is that lines.count = 0 should not exist. But I am not opening that can of worms right now.

I added a few more workarounds. Please test.

Ok, looking at your code.

This should be all you need. Caret, Selection and IsBackwardSel are all kept

   with SynEdit1 do
   begin
     BB := BlockBegin;
     BE := BlockEnd;
     writeln('BlockBegin  =',bb.x,',',bb.y);
     writeln('BlockEnd    =',be.x,',',be.y);
     writeln(SynEdit1.IsBackwardSel);

    BB := BlockBegin;
    SetTextBetweenPoints(BB, BB, Pre, [setMoveBlock], scamAdjust, smaMoveUp, smNormal);
  // scamAdjust => Move the caret with the block.     BE := BlockEnd;
    SetTextBetweenPoints(BE, BE, Post, [setMoveBlock], scamIgnore, smaMoveUp, smNormal);
  // setMoveBlock for the 2nd call,  is to KEEP the selection as it is. So to keep caret => scamIgnore

     BB := BlockBegin;
     BE := BlockEnd;
     writeln('After SetTextBetweenPoints(BE,BE,...)');
     writeln('BlockBegin  =',bb.x,',',bb.y);
     writeln('BlockEnd    =',be.x,',',be.y);
     writeln(SynEdit1.IsBackwardSel);
   end;

Also  SelEnd / SelStart are really slow (they have to iterate over all lines up to the selection, each time you call them. SynEdit operates with x/y locations.

AM Martin Frb wrote

This should be all you need. Caret, Selection and IsBackwardSel are all kept

...

Yes, this seems to work OK, even when Lines.Count = 0.

So, now I need to $ifdef my workaround for Lazarus 2.0. What ifdef to use?

Also SelEnd / SelStart are really slow (they have to iterate over all lines up to the selection, each time you call them. SynEdit operates with x/y locations.

I know, see the diff in
https://sourceforge.net/p/flyingsheep/code/324/tree//trunk/EPlus/editorpagecontrol.pp?diff=4faa6364b9363c207f000850:323

+ //Using SetTextBetweenPoints is faster (and according to Martin Friebe) more reliable
+ //than using SelStart and SelLength (like the old code did)

That code was by you, so you got me into this mess in the first place :-)

{$IF LCL_FullVersion < 2010000}

This does not work for smColumn mode. Even if the column only has a single line. But trying to do a {} around a column selection is of limited use anyway. It would only add on opening on the very first line, and a closing on the very last line.

Btw, I have not tested that code for persistent selections.

It relies on that the 2nd  SetTextBetweenPoints does not move the caret (that will be the same with persistent selection)
and that if the block had been moved (in case of on empty selection), the caret will reset the block.

That is because the selection must end (or begin) at the caret. Which is not the case after the empty selection did move.
A persistent selection does not need to end at the caret. So I do not know if it will be reset. (Maybe it does, maybe not)

If it is not, then it will still at first look ok: selection still empty, and caret at correct location.
But this may affect how ctrl-K,B / ctrl-K,k  (IIRC) work. That is, if you set the block-end, the begin (even though empty) may have been moved.

Hovewer come to think of it:
Unless the selection begin has just been set, the begin of a persistent selection is not at the caret. So using BlockBegin/End for an empty persistent selection will not get you what you want. So if there is no selection (and if persistent selection could be enabled) then (not tested) something like this may help

If not SelAvail then begin
    p := LogicalCaretPos;
    f :=    BlockBegin = p;
    SetTextBetweenPoints(p, p, Post, [setMoveBlock],
scamIgnore,smaMoveUp, smNormal);
    SetTextBetweenPoints(p, p, Pre, [setMoveBlock],
scamAdjust,smaMoveUp, smNormal);
    if f then BlockBegin := p;
end
else
   // the other code

Selection will be moved, if it is not at the caret pos. Otherwise
- insert the end "}" first, but do not move caret.
- insert begin (will insert before end, because caret did not move),     will move the caret (so it is after "{"), and will reset selection to be empty at caret
- reset empty selection to caret.

BlockBegin/BlockEnd are faster than SelStart/SelEnd.

BlockBegin/End and SetTextBetweenPoints are comparable. But SetTextBetweenPoints should be less work to use.

That code was by you, so you got me into this mess in the first place :-)

In that case sorry.

Hi,

In my real app I don't have persistent blocks, so that should not be a problem.

BlockBegin/BlockEnd are faster than SelStart/SelEnd. BlockBegin/End and SetTextBetweenPoints are comparable.

Speed is not of concern in my use case. The computer will always be faster than any human typing in my editor ;-)
My old code uses SelStart etc (ported form a Delphi 3 app), but then I could not get it to work properly if the selection spanned multiple lines.
You then suggested I'lld use SetTextBetweenPoints. This did not have that drawback.

But SetTextBetweenPoints should be less work to use.

Apparently not for me <big grin>

IIRC then at some point in time I ran into problems (probably related to new SetTextBetweenPoints API ???) with backwards blocks and empty blocks.
Since my app is mostly private for me (I create all my htm pages with it and use it instead of notepad whenever possible), I did not bother too much.
However I seem to have a habit of selecting text from right to left, so the bug triggered more often then not.
Once I was frustrated enough with it I investigated again and asked for your help (again).
ATM I have it working, for which I thank you.
In my app, I solve the Lines.Count = 0 situation by simply adding an empty line. This makes sense, in the way that the function will always insert text in the editor, so linecount would have been altered in that case anyway.

One more question:
The current solution creates 2 "redo" points.
I have to press ^Z 2 times to undo the process.
Is there a way to make it so that the whole process can be undone in one step (by invoking the default undo-mechanism of TSynEdit)?

Again, thanks for your help.

Bart

SynEdit1.BeginUpdate(); / EndUpdate

or BeginUndoBlock.

PM Martin Frb wrote

SynEdit1.BeginUpdate(); / EndUpdate

Thanks.
Should have figured that out myself, shouldn't I ;-)