Undo does not move to the correct location in the editor window #707

Closed
processing-bugs opened this Issue Feb 10, 2013 · 22 comments

Comments

Projects
None yet
7 participants
@processing-bugs

Original author: mw.mzp...@gmail.com (May 08, 2011 15:07:01)

Jump to the line and column where the done-and-undone modification took place.

Original issue: http://code.google.com/p/processing/issues/detail?id=668

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From f...@processing.org on June 21, 2011 00:02:55
Please provide some details. For starters, what version are you using?

From f...@processing.org on June 21, 2011 00:02:55
Please provide some details. For starters, what version are you using?

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From nth...@gmail.com on November 23, 2011 16:03:56
I'm using 2.0a4 and have this same problem. (I think it's been around for a while) When working with large .pde files with many tabs open undoing behavior is strange.

Example: I make a bunch of changes on one tab, then switch to another tab and make a change.
I then decide I don't like some of the changes I made so I start Ctrl-Zing. I undo all my changes I made in this tab, but I keep pressing Ctrl-Z. Changes are then made in the other tab, but I can't see what changes they are!

This also happens when the changes are in the current tab but are scrolled out of view.

It would be great if the IDE would "snap to" the place where the undo or redo changes are being made. Whether by changing tabs or by scrolling the viewport.

Or somehow keep seperate undo/redo histories for each tab.

From nth...@gmail.com on November 23, 2011 16:03:56
I'm using 2.0a4 and have this same problem. (I think it's been around for a while) When working with large .pde files with many tabs open undoing behavior is strange.

Example: I make a bunch of changes on one tab, then switch to another tab and make a change.
I then decide I don't like some of the changes I made so I start Ctrl-Zing. I undo all my changes I made in this tab, but I keep pressing Ctrl-Z. Changes are then made in the other tab, but I can't see what changes they are!

This also happens when the changes are in the current tab but are scrolled out of view.

It would be great if the IDE would "snap to" the place where the undo or redo changes are being made. Whether by changing tabs or by scrolling the viewport.

Or somehow keep seperate undo/redo histories for each tab.

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From f...@processing.org on November 23, 2011 19:45:23
It should be doing both of those things, so I just need an example of it not working so I can try to fix it.

There have been several fixes made over time to this behavior, so it's probably different in 1.2.1, 1.5.1, and 2.0 alpha releases. So I need some help in determining if it's never worked properly, or there have been specific regressions, or what.

From f...@processing.org on November 23, 2011 19:45:23
It should be doing both of those things, so I just need an example of it not working so I can try to fix it.

There have been several fixes made over time to this behavior, so it's probably different in 1.2.1, 1.5.1, and 2.0 alpha releases. So I need some help in determining if it's never worked properly, or there have been specific regressions, or what.

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From nth...@gmail.com on November 28, 2011 22:23:28
Here's a sketch where the error happens. The example is trivial but it demonstrates the problem.

void setup() {
  ArrayList<Integer> data;

  data.add(round(random(1)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));

}
  1. Paste that into a new Sketch.
    1.a) Resize the window so a scrollbar appears, and you can't see all the add() method calls
  2. Delete the line data.add(round(random(1)));
  3. Move the cursor to the bottom of the sketch, past the last add() or after the } either works.
  4. Ctrl-Z

There won't be any noticeable change, but if you scroll up you'll see the data.add(round(random(1))) has reappeared as we would expect from the Undo call. However the programmer would have had no way of knowing that this is what the Undo did without scrolling to the location itself. Similar behavior happens with multi-tab undos.

I'm running 2.0a4 MacOS 10.6.8 (and I think Java 1.6). Hopefully this example helps some, but if you need something more detailed I can try to come up with something.

From nth...@gmail.com on November 28, 2011 22:23:28
Here's a sketch where the error happens. The example is trivial but it demonstrates the problem.

void setup() {
  ArrayList<Integer> data;

  data.add(round(random(1)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));
  data.add(round(random(100)));

}
  1. Paste that into a new Sketch.
    1.a) Resize the window so a scrollbar appears, and you can't see all the add() method calls
  2. Delete the line data.add(round(random(1)));
  3. Move the cursor to the bottom of the sketch, past the last add() or after the } either works.
  4. Ctrl-Z

There won't be any noticeable change, but if you scroll up you'll see the data.add(round(random(1))) has reappeared as we would expect from the Undo call. However the programmer would have had no way of knowing that this is what the Undo did without scrolling to the location itself. Similar behavior happens with multi-tab undos.

I'm running 2.0a4 MacOS 10.6.8 (and I think Java 1.6). Hopefully this example helps some, but if you need something more detailed I can try to come up with something.

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From f...@processing.org on November 29, 2011 01:04:55
Thanks for the example, very helpful.

If anyone has a chance, can you check whether this does the same (incorrect) thing in 1.2.1 and 1.5.1? This has been "fixed" multiple times by different people, so it seems we're going in circles.

From f...@processing.org on November 29, 2011 01:04:55
Thanks for the example, very helpful.

If anyone has a chance, can you check whether this does the same (incorrect) thing in 1.2.1 and 1.5.1? This has been "fixed" multiple times by different people, so it seems we're going in circles.

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From nth...@gmail.com on November 29, 2011 15:13:17
Ok, I tried in both 1.2.1 and 1.5.1 and both of them seem to be working properly. They do not exhibit the same behavior that I mentioned in 2.0a4

I also tried some of the replicating some of the issues I had seen with undo when using multiple tabs. Both 1.2.1 and 1.5.1 worked great as expected, however I had issues with 2.0a4

I can mock up an example of the multiple tab issues if you'd like, but I'm guessing this was something that had been fixed but then got reverted in 2.0

From nth...@gmail.com on November 29, 2011 15:13:17
Ok, I tried in both 1.2.1 and 1.5.1 and both of them seem to be working properly. They do not exhibit the same behavior that I mentioned in 2.0a4

I also tried some of the replicating some of the issues I had seen with undo when using multiple tabs. Both 1.2.1 and 1.5.1 worked great as expected, however I had issues with 2.0a4

I can mock up an example of the multiple tab issues if you'd like, but I'm guessing this was something that had been fixed but then got reverted in 2.0

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From f...@processing.org on November 29, 2011 15:26:12
Yes, that's what I suspected. I'll check into that, this is enough information for me to work through it.

Unless, of course, someone wants to dig through things in Editor.java and figure out where things regressed... :-)

From f...@processing.org on November 29, 2011 15:26:12
Yes, that's what I suspected. I'll check into that, this is enough information for me to work through it.

Unless, of course, someone wants to dig through things in Editor.java and figure out where things regressed... :-)

@joshgiesbrecht

This comment has been minimized.

Show comment
Hide comment
@joshgiesbrecht

joshgiesbrecht Jun 3, 2013

Contributor

Found the problem and fixed it (on my build anyway). Pull request is at: #1847

Contributor

joshgiesbrecht commented Jun 3, 2013

Found the problem and fixed it (on my build anyway). Pull request is at: #1847

@aengelke

This comment has been minimized.

Show comment
Hide comment
@aengelke

aengelke Jun 22, 2013

Contributor

@joshgiesbrecht In 2.0.1, it isn't working for me (Java mode). Perhaps you should modify the caret location...

Contributor

aengelke commented Jun 22, 2013

@joshgiesbrecht In 2.0.1, it isn't working for me (Java mode). Perhaps you should modify the caret location...

@joshgiesbrecht

This comment has been minimized.

Show comment
Hide comment
@joshgiesbrecht

joshgiesbrecht Jun 22, 2013

Contributor

There are two related things going on ... one was that it wasn't moving the cursor to the undo location at all. The other thing (which I haven't fixed) is that it stores undo events too aggressively. I tried it out today and I think it may even store arrow-key scrolling as an undo event. So that'll make things look weird.

Try the above test case but make sure you use the mouse to scroll down during step 3, after making the edit. That should work. (If not then I've done something wrong.)

Edit: Something is still being slightly weird; it sometimes moves the caret to a couple lines below where the edit happened. Better, but not perfect.

Contributor

joshgiesbrecht commented Jun 22, 2013

There are two related things going on ... one was that it wasn't moving the cursor to the undo location at all. The other thing (which I haven't fixed) is that it stores undo events too aggressively. I tried it out today and I think it may even store arrow-key scrolling as an undo event. So that'll make things look weird.

Try the above test case but make sure you use the mouse to scroll down during step 3, after making the edit. That should work. (If not then I've done something wrong.)

Edit: Something is still being slightly weird; it sometimes moves the caret to a couple lines below where the edit happened. Better, but not perfect.

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar Aug 11, 2015

Contributor

Looks like it is fixed, needs more testing.

Contributor

JakubValtar commented Aug 11, 2015

Looks like it is fixed, needs more testing.

@JakubValtar JakubValtar added revised and removed revision needed labels Aug 11, 2015

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Aug 11, 2015

Member

I think this one isn't, it's been fixed many times but I feel like we're still getting reports about it.

Member

benfry commented Aug 11, 2015

I think this one isn't, it's been fixed many times but I feel like we're still getting reports about it.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Aug 11, 2015

Member

@REAS @shiffman don't you guys have trouble with this one and students?

Member

benfry commented Aug 11, 2015

@REAS @shiffman don't you guys have trouble with this one and students?

@benfry benfry added this to the 3.0 final milestone Aug 11, 2015

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Aug 11, 2015

Member

Or better yet, can someone help us with testing this one? @JakubValtar couldn't get it to reproduce, but we'd like some additional testing before we close.

Member

benfry commented Aug 11, 2015

Or better yet, can someone help us with testing this one? @JakubValtar couldn't get it to reproduce, but we'd like some additional testing before we close.

@JakubValtar JakubValtar removed the revised label Aug 11, 2015

@joshgiesbrecht

This comment has been minimized.

Show comment
Hide comment
@joshgiesbrecht

joshgiesbrecht Aug 11, 2015

Contributor

Just tested it and it's behaving about the same way as I described above. It more-or-less works except sometimes it's leaving the caret a few lines below the edit instead of directly below. It seems to be just the caret position that's off, it still scrolls up enough to see the edited line.
(Win 7 in case that matters)

Contributor

joshgiesbrecht commented Aug 11, 2015

Just tested it and it's behaving about the same way as I described above. It more-or-less works except sometimes it's leaving the caret a few lines below the edit instead of directly below. It seems to be just the caret position that's off, it still scrolls up enough to see the edited line.
(Win 7 in case that matters)

@shiffman

This comment has been minimized.

Show comment
Hide comment
@shiffman

shiffman Aug 11, 2015

Member

The problem I experienced (in the past) was that code would disappear after several undos. I have not noticed this in the recent 3.0 alpha and beta releases and undo seems to be working better. I'll keep a closer eye out.

Member

shiffman commented Aug 11, 2015

The problem I experienced (in the past) was that code would disappear after several undos. I have not noticed this in the recent 3.0 alpha and beta releases and undo seems to be working better. I'll keep a closer eye out.

@joshgiesbrecht

This comment has been minimized.

Show comment
Hide comment
@joshgiesbrecht

joshgiesbrecht Aug 11, 2015

Contributor

I hadn't noticed when #3003 came up. I'll try to look into it soonish, and keep an eye out for the misplaced-caret problem along the way. (Finally got my local build working with 3.0 just recently)

Unless the misplaced caret problem is significant enough to still bother anyone, maybe this one could be closed and the focus can be on #3003?

Contributor

joshgiesbrecht commented Aug 11, 2015

I hadn't noticed when #3003 came up. I'll try to look into it soonish, and keep an eye out for the misplaced-caret problem along the way. (Finally got my local build working with 3.0 just recently)

Unless the misplaced caret problem is significant enough to still bother anyone, maybe this one could be closed and the focus can be on #3003?

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Aug 11, 2015

Member

There's no reason we shouldn't be able to fix the misplaced caret problem—sounds like just a random quirk? I seem to recall that the Undo state information records caret placement, so the code should be in place for handling it.

@joshgiesbrecht Was there a problem with the default build or just something on your machine?

Member

benfry commented Aug 11, 2015

There's no reason we shouldn't be able to fix the misplaced caret problem—sounds like just a random quirk? I seem to recall that the Undo state information records caret placement, so the code should be in place for handling it.

@joshgiesbrecht Was there a problem with the default build or just something on your machine?

@joshgiesbrecht

This comment has been minimized.

Show comment
Hide comment
@joshgiesbrecht

joshgiesbrecht Aug 12, 2015

Contributor

Nah just my local setup. I hadn't cleaned things out and rebuilt since pre-3.0.

Contributor

joshgiesbrecht commented Aug 12, 2015

Nah just my local setup. I hadn't cleaned things out and rebuilt since pre-3.0.

@JukePlz

This comment has been minimized.

Show comment
Hide comment
@JukePlz

JukePlz Aug 12, 2015

I get this and the mentioned dissapearing text a lot. Im pretty sure it's some race condition with automatic markup (ctrl+t) previously to an undo action.
Will try to find better repro steps later.

JukePlz commented Aug 12, 2015

I get this and the mentioned dissapearing text a lot. Im pretty sure it's some race condition with automatic markup (ctrl+t) previously to an undo action.
Will try to find better repro steps later.

@joshgiesbrecht

This comment has been minimized.

Show comment
Hide comment
@joshgiesbrecht

joshgiesbrecht Aug 12, 2015

Contributor

Ok, actually looking at this today. I have a good idea why the caret is off by a line or two. Could definitely use repro steps for anything else.

Contributor

joshgiesbrecht commented Aug 12, 2015

Ok, actually looking at this today. I have a good idea why the caret is off by a line or two. Could definitely use repro steps for anything else.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Aug 19, 2015

Member

Incorporated for 3.0 beta 5, thanks.

Member

benfry commented Aug 19, 2015

Incorporated for 3.0 beta 5, thanks.

@benfry benfry closed this Aug 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment