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

Add undo #6570

Merged
merged 1 commit into from Feb 7, 2020
Merged

Add undo #6570

merged 1 commit into from Feb 7, 2020

Conversation

krobelus
Copy link
Member

@krobelus krobelus commented Feb 6, 2020

Add the readline function undo which is bound to \c_ (control + / on
some terminals). Redoing the most recent chain of undos is supported,
redo is bound to \e/ for now.

Closes #1367.
This approach should not have the issues discussed in #5897.

Every single modification to the commandline can be undone individually,
except for adjacent single-character inserts, which are coalesced,
so they can be reverted with a single undo. Coalescing is not done for
space characters, so each word can be undone separately.

When moving between history search entries, only the current history
search entry is reachable via the undo history. This allows to go back
to the original search string with a single undo, or by pressing the
escape key.
Similarly, when moving between pager entries, only the most recent
selection in the pager can be undone.

TODO: (I'm not sure about the first three)

  • decide on the redo hotkey; \e/ does not seem to work on xterm
    (it is tempting to add emacs-style redo but that would probably be less popular)
  • vi mode: find some suitable bindings (u and \cr are taken)
  • vi mode: find out how to bind in insert mode (which should then also work in the pager's search field)
  • update documentation
  • User-visible changes noted in CHANGELOG.md

These areas should be mostly alright based on what I tried but there may be room for improvement:

  • make sure the undo grouping behavior works nicely in practise
  • make sure the interaction with history search feels right
  • make sure the interaction with the pager feels right
  • add some tests (interactive tests are missing - expect-based tests are not very nice to write last time I tried)

I think this also allows us to get rid of cycle_command_line and cycle_cursor_pos because we can obtain those from the history.

Copy link
Member

@ridiculousfish ridiculousfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!! The code is solid 💯.

src/reader.h Outdated Show resolved Hide resolved
src/reader.h Outdated Show resolved Hide resolved
@krobelus krobelus added this to the fish 3.2.0 milestone Feb 7, 2020
Add the input function undo which is bound to `\c_` (control + / on
some terminals). Redoing the most recent chain of undos is supported,
redo is bound to `\e/` for now.

Closes fish-shell#1367.
This approach should not have the issues discussed in fish-shell#5897.

Every single modification to the commandline can be undone individually,
except for adjacent single-character inserts, which are coalesced,
so they can be reverted with a single undo. Coalescing is not done for
space characters, so each word can be undone separately.

When moving between history search entries, only the current history
search entry is reachable via the undo history. This allows to go back
to the original search string with a single undo, or by pressing the
escape key.
Similarly, when moving between pager entries, only the most recent
selection in the pager can be undone.
@krobelus
Copy link
Member Author

krobelus commented Feb 7, 2020

Thanks!
I also made the terminal flash when there is nothing to undo/redo (flash on an empty commandline seems to do nothing here so that's not noticeable for undo).
Merging to master; the current behavior feels reasonable for a start.
vi-mode keybindings and a more portable redo hotkey should be added eventually

@krobelus krobelus merged commit 8a033b9 into fish-shell:master Feb 7, 2020
@krobelus krobelus deleted the undo branch February 7, 2020 16:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for undo
2 participants