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

Regression about undo/redo #1421

Closed
lhmouse opened this issue Jul 15, 2019 · 21 comments
Closed

Regression about undo/redo #1421

lhmouse opened this issue Jul 15, 2019 · 21 comments
Labels

Comments

@lhmouse
Copy link
Contributor

lhmouse commented Jul 15, 2019

Version

d14e227

Steps to Reproduce

  1. Create an empty file.
  2. Type abcdefghijlkmn.
  3. Press Ctrl-Z repeatedly to undo the input.
  4. Press Ctrl-Y repeatedly to redo the input.

Behavior Got

Undo/redo is now character-wise. Additionally, in order to undo/redo each character other than the last one, the combination key has to be pressed twice.

Behavior Expected

The sequential keystrokes should be considered as an integral operation. A single Ctrl-Z should suffice to undo it. So is Ctrl-Y.

@hpwamr
Copy link
Collaborator

hpwamr commented Jul 15, 2019

Hello @RaiKoHoff ,
Tested Final Notepad3 v5.19.630.2381: Ctrl+Z and Ctrl+Y consider abcdefghijlkmn as a unit.

@RaiKoHoff
Copy link
Collaborator

Feel free to test beta version _5.19.715.2393_BETA (beta channel access see: #1129).

@lhmouse
Copy link
Contributor Author

lhmouse commented Jul 15, 2019

LGTM. Thanks for the fix.

@lhmouse lhmouse closed this as completed Jul 15, 2019
@leeoniya
Copy link

leeoniya commented Jul 15, 2019

@RaiKoHoff

seems like now it has the opposite problem, where everything is considered as a single undo transaction?

EDIT: this only seems to be the case when authoring new content:

  1. type some text
  2. wait 1 sec
  3. press enter (new line) type some more text
  4. wait 1 sec
  5. press enter (new line) type some more text
  6. press ctrl+z undoes everything above.

i would expect there to be some timeout between closing the undo transaction. 1 sec seems reasonable to me.

@lhmouse
Copy link
Contributor Author

lhmouse commented Jul 15, 2019

I fail to see what's wrong here:

  1. Create an empty file.
  2. Type aaaaaaaaaaa followed by Enter
  3. Type bbbbbbbbbb followed by Enter
  4. Type cccccccccccc followed by Enter
  5. Select the first line by left-clicking on the line number column, copy it and paste it at the end.
  6. Select the second line by left-clicking on the line number column, copy it and paste it at the end.
  7. The text area now contains 5 lines. Pressing Ctrl-Z undoes the second paste; pressing it again undoes the first paste; pressing it once more undoes the first three lines because they are a whole.

@leeoniya
Copy link

leeoniya commented Jul 15, 2019

pressing it once more undoes the first three lines because they are a whole.

that's kinda the crux of this for me. if there is sufficient time between user activity, then each is a separate transaction in my view. this seems to only affect exclusively appending new data by keyboard entry.

SublimeText and LibreOffice Writer works more like i described. VScode seems to rely on some combination of a timeout and whitespace (or complete non-whitespace tokens). Notepad++ works like Notepad3 (probably since both scintilla).

so while not a bug, per se, it would be quite useful if there was a minimal amount of smarts around doing partial undos of long-lived append-only transactions.

@lhmouse
Copy link
Contributor Author

lhmouse commented Jul 15, 2019

If the caret is moved elsewhere after inserting some text, but moved back before inserting more (so the insert position doesn't change despite the move), the two insert operations are still considered a single unit. Probably this is undesired, but I am fine with either.

@RaiKoHoff
Copy link
Collaborator

I vote against an undo split based on some timeout settings, and do not change Scintilla's undo/redo default in this case.

@hpwamr
Copy link
Collaborator

hpwamr commented Jul 16, 2019

I second your vote... 😃

@leeoniya
Copy link

the nays have it!

@RaiKoHoff
Copy link
Collaborator

@leeoniya : to track this discussion, just create a new issue (change request) for this.

@leeoniya
Copy link

i don't feel strongly enough about it, so if you guys don't see enough reason to add it, then that's fine.

@RaiKoHoff
Copy link
Collaborator

@leeoniya: see #1548 (comment)

@RaiKoHoff RaiKoHoff reopened this Aug 27, 2019
@leeoniya
Copy link

thanks, will test later today!

@leeoniya
Copy link

from prelimnary testing, 5.19.828.2604 BETA behaves as i'd expect. 👍

@RaiKoHoff
Copy link
Collaborator

We still have this bug (Point 1 of #1548 (comment)), so I suggest to keep this issue open until we have a solution for it.

@RaiKoHoff
Copy link
Collaborator

New development beta version (_5.19.828.2605_BETA) available.

@ltguillaume
Copy link
Contributor

Also see #1599

@RaiKoHoff
Copy link
Collaborator

Thank you for testing and issue reporting 😄 - Feel free to test dev beta ver _5.19.904.2617_BETA.

@hpwamr
Copy link
Collaborator

hpwamr commented Sep 27, 2019

Hello @lhmouse ,
As far as I'm concerned, I think you (as requester) can close this issue...

@lhmouse
Copy link
Contributor Author

lhmouse commented Sep 27, 2019

oh ok.

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

No branches or pull requests

5 participants