Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/text changed event args changed range #215

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

codingdave
Copy link

Short:
I have observed that the Range Start setter also changes the end field. That leads to

  • bugs like that the TextChangedEventArgs.ChangedRange is always reduced to the last range instead of the indended merging of ranges (see FastColoredTextBox:OnTextChanged(TextChangedEventArgs args), 6019-6038)
  • unnecessary assignments because often the code first changes the Start of a Range and then exlicitly sets End property as well

Long

  • The setter of Range.Start modifies Range.End. Start and End are equal. That may lead to unexpected behavior.
  • In the function mentioned above updatingRange.Start is overwritten and such the bottom end of the accumulated range updatingRange.End gets lost. This also leads to updatingRange.GetIntersectionWith(Range) being a noop but much more important: The ChangedRange will always only be line that was changed at last.

I have fixed that issue in this PR. Because it is hard to see where this behavior (Start == End) is required I have added a function SetStartAndEnd that is explicitly doing what it says: Setting Start and End at the same time. Then I went through all the code and have inserted that function whereever Range.Start was assigned and not Range.End was assigned in the next line:
TB.Selection.Start = new Place(0, LineIndex)

Also have left lines untouched that are assigning End to Start:
Start = End;

In the second step I have changed the Range.Start setter to not any longer modify Range.End.

I have also created a sample application that demonstrates that behavior, but it is not part of this PR. You can find it here:
https://github.com/codingdave/FastColoredTextBox/tree/fix/TextChangedEventArgs-ChangedRange-Sample

These are the screenshots of the sample. Green is unchanged text, Red is the ChangedRange and black is the new text that is added and not part of the ChangedRange, so you can say that is the error that this PR fixes.

Before the fix
2020-10-09_15-28-41

After the fix
2020-10-09_15-28-46

One further note: I have left the VB code untouched, as this is not my comfortable language.

@codingdave
Copy link
Author

Hi @PavelTorgashov what do you think about this PR? Do you have any interest in this patch? If so, what is missing for you to accept this PR?

Kind regards

@codingdave
Copy link
Author

Hi @PavelTorgashov any updates on this? Is this project dead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant