Skip to content

Fix a few undo-related bugs#4310

Merged
benfry merged 3 commits into
processing:masterfrom
joelmoniz:bugfixes-undo
May 9, 2016
Merged

Fix a few undo-related bugs#4310
benfry merged 3 commits into
processing:masterfrom
joelmoniz:bugfixes-undo

Conversation

@joelmoniz
Copy link
Copy Markdown
Member

This PR attempts to resolve #4280, fix #4302 and close #4303.
#4303 was mostly an issue with not considering Replace All as a compound edit. I have left Replaces to be individually undo-able, since technically each Replace i a distinct operation (unlike Replace Alls, which all happen at once).

I had to modify the JEditTextArea a little for #4280, because the replacement happens from within there, and there's no Editor object accessible (and hence, the Editor's compound edit methods can't be called). The SyntaxDocument has compound edit methods, but they were left blank, and never overridden, so I now have.
#4302 was a little tougher to crack, but primarily boiled down to each insert and deletion being considered a separate compound edit, with replacements (i.e., entries in Insert mode) being modeled as a delete+insert, which kept triggering endCompoundEdit() and starting a new one. Another issue was that a compound edit ends whenever the text in the Editor is the same before and after a key press, which could happen when the user replace a line like int k = 4; with `int x = 5;' (several character overlap here, and the set of characters between each overlapping "block" would be undone at a go, and a block could be as small as a single character).

@benfry There are changes in here to a few of the methods, and how they are called, but I have tried to keep things as neat as possible. Could you please let me know if these changes are ok, if any modification to how changes have been incorporated need to be made, or if further changes are required? Thanks!

@benfry
Copy link
Copy Markdown
Contributor

benfry commented May 8, 2016

@JakubValtar Can you review?

@JakubValtar
Copy link
Copy Markdown
Contributor

Reviewing right now.

@JakubValtar
Copy link
Copy Markdown
Contributor

#4280 is not fixed

Other two are fixed, but I find Undo still terribly broken when more tabs are involved.

When Undo modifies more tabs, the code in Editor is updated, but they are not marked as modified (orange strip), and when I save without visiting the tabs, changes made by undo are not saved (something to do with SketchCode having both Document and String program). This is quite dangerous and confusing as hell.

I was also fighting with this with Rename and I temporarily fixed it in some places (JavaBuild, PreprocessingService) by re-reading documents from all tabs instead of using SketchCode's program.

Also when Undo affects more tabs, you can undo only from one of them, others don't have any Undo available.

I think Undo needs to be fixed before 3.1. The situation with rewriting text from code is already quite convoluted and maybe it would be better to clean it up instead of adding more complexity by merging this. If you want I can look at it tomorrow.

@benfry
Copy link
Copy Markdown
Contributor

benfry commented May 9, 2016

Hm, so that would delay 3.1 for a week. Are things any worse with Undo than they have been in the past? Or is it just a matter of it being a high priority?

@JakubValtar
Copy link
Copy Markdown
Contributor

@benfry Let's merge this and I'll work on Undo and look at #4280 when it's merged

@benfry benfry merged commit 0bcf878 into processing:master May 9, 2016
@benfry
Copy link
Copy Markdown
Contributor

benfry commented May 9, 2016

Ok, done.

@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Replace" and "Replace All" do not undo in a single step Undo is a little "jerky" in insert mode Undo misbehaving

3 participants