TextSegmentCollection - unexpected behaviour when replacing text #331

New issue

Open

opened 2021-12-23 12:25:40 -06:00 by DoctorVanGogh · 2 comments

DoctorVanGogh commented

2021-12-23 12:25:40 -06:00

(Migrated from github.com)

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

github.com/icsharpcode/AvalonEdit@47f61e9427/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.

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 Text*Markers*) 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: ```cs ... 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.

siegfriedpammer commented

2021-12-27 06:35:48 -06:00

(Migrated from github.com)

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

DoctorVanGogh commented

2021-12-27 09:59:48 -06:00

(Migrated from github.com)

For a prototype implementation of my suggested changes see github.com/DoctorVanGogh/AvalonEdit@db4368916f

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: github.com/DoctorVanGogh/AvalonEdit@2834f6900a

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

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-icsharpcode/AvalonEdit#331

No description provided.

Rows
Columns