AvalonEdit icon indicating copy to clipboard operation
AvalonEdit copied to clipboard

TextSegmentCollection - unexpected behaviour when replacing text

Open DoctorVanGogh opened this issue 4 years ago • 2 comments

I've come across an issue which I suspect is caused by the internal implementation of TextSegmentCollection.ReplaceText, specifically this section

https://github.com/icsharpcode/AvalonEdit/blob/47f61e9427b0cb2653f83e8c303b9ea94c8ef60c/ICSharpCode.AvalonEdit/Document/TextSegmentCollection.cs#L200-L203

A segment gets removed from the collection, then manipulated, then re-added to the collection.

If there is any logic reacting to the in-between manipulation, it may behave unexpectedly, since the segment is not a member of the collection (at that time).

My problem were Segments (specifically TextMarkers) which got replaced in their entirety, but would claim not to be members of their corresponding collection during a TextSegment.OnSegmentChanged invocation, only to "magically" reappear during later calls (specifically redraws). See https://github.com/icsharpcode/AvalonEdit/discussions/327

There may be some hacky ways to work around the problem (sadly there are no "added/removed" triggers for the segment collection.. and it's sealed...). One could post some handler via Dispatcher to "recheck" a segment... but that's... messy.

A clean solution to my mind would be adding BeginChange() / EndChange() methods to TextSegment (similar to what exists for TextDocument) which suppress all OnSegmentChanged invocations and rework the ReplaceText method to use those methods:

...
segment.BeginChange();
RemoveSegment(segment); 
segment.StartOffset = offset + change.RemovalLength; 
segment.Length = Math.Max(0, remainingLength); 
AddSegment(segment); 
segment.EndChange();
...

That would defer any OnSegmentChanged invocation until the segment is once again in an expected state.

DoctorVanGogh avatar Dec 23 '21 18:12 DoctorVanGogh

See https://github.com/icsharpcode/AvalonEdit/discussions/327#discussioncomment-1873085 as GitHub does not add a reference to the discussion apparently.

siegfriedpammer avatar Dec 27 '21 12:12 siegfriedpammer

For a prototype implementation of my suggested changes see https://github.com/DoctorVanGogh/AvalonEdit/commit/db4368916ffdb37729956224996f6edc55a4e200

It keeps the "replace text" operation atomic in regards to any attached (overridden) logic on TextSegment.OnSegmentChanged.


Edit: Also created a unit test to show my failure case: https://github.com/DoctorVanGogh/AvalonEdit/commit/2834f6900ab9178286efd0472fecc326d57a265e

DoctorVanGogh avatar Dec 27 '21 15:12 DoctorVanGogh