REPL/LineEdit to support "undo": part of #8447 #9596

srp · 2015-01-04T06:51:10Z

The REPL now supports undo via Ctrl-^ or Ctrl-_.

This should very closely minic the behavior of other readline/emacs
shells, except it doesn't let the user goto a historical entry (A),
edit it, goto a different historical entry (B), return to the first
(A) and undo the edits.

This change is

rfourquet · 2015-01-05T08:16:50Z

Thanks! I tested it and works fine. I don't know the LineEdit.jl code very well yet, but the patch looks good to me.
WRT the limitation you mention in the commit: would it be difficult to eventually attach an undo_buffers vector to each history entry? (but it's maybe not worth the effort, I don't think I use this functionality in readline).
There is also another difference from readline, which packs together individual characters inserts (i.e. typing "123" and then undo removes those 3 characters), but I don't see this as a problem.

rfourquet · 2015-01-05T08:19:53Z

base/LineEdit.jl

@@ -1512,7 +1538,7 @@ end

 run_interface(::Prompt) = nothing

-init_state(terminal, prompt::Prompt) = PromptState(terminal, prompt, IOBuffer(), InputAreaState(1, 1), length(prompt.prompt))
+init_state(terminal, prompt::Prompt) = PromptState(terminal, prompt, IOBuffer(), (IOBuffer)[], InputAreaState(1, 1), length(prompt.prompt))


Why not IOBuffer[] (w/o parens) ?

rfourquet · 2015-01-05T08:23:47Z

Oh, and special +1 for extensive tests.
If you didn't start yet implementing beginning/end-of-history from #8447, I have it here and will do a PR soon.

srp · 2015-01-05T16:02:57Z

Looking forward to the beginning/end-of-history! My plan was to next tackle kill/yank which doesn't seem to work quite right: m-d and friends don't get added to yank ring, yank ring isn't yet a ring, etc.

rfourquet · 2016-05-15T07:43:38Z

Bump. I'm really missing this functionality!

blakejohnson · 2016-05-16T01:43:03Z

base/LineEdit.jl

@@ -146,16 +148,14 @@ function complete_line(s::PromptState, repeats)
    elseif length(completions) == 1
        # Replace word by completion
        prev_pos = position(s.input_buffer)
-        seek(s.input_buffer, prev_pos-sizeof(partial))


why have the calls to seek been dropped here and elsewhere in this PR?

Probably because edit_replace already is "undo aware", whereas seek is not. When I compile without this change, after an undo the cursor is misplaced (e.g. at the beginning of a (prefix) word after undoing a completion).

rfourquet · 2016-05-16T08:36:26Z

I rebased locally this branch on master, and for some reason the key-binding "^/" is not valid anymore (compilation fails), I don't know why yet.

rfourquet · 2017-07-17T08:26:24Z

@srp would you consider rebasing this work to get it merged?

StefanKarpinski · 2017-07-18T21:23:45Z

Yes, please – this would be a lovely feature to have. Sorry the PR has languished!

tkelman · 2017-07-22T19:22:52Z

base/repl/LineEdit.jl

@@ -1559,7 +1583,7 @@ end

 run_interface(::Prompt) = nothing

-init_state(terminal, prompt::Prompt) = PromptState(terminal, prompt, IOBuffer(), InputAreaState(1, 1), #=indent(spaces)=#strwidth(prompt.prompt))
+init_state(terminal, prompt::Prompt) = PromptState(terminal, prompt, IOBuffer(), IOBuffer[], InputAreaState(1, 1), #=indent(spaces)=#strwidth(prompt.prompt))


this line should probably be wrapped

The REPL now supports undo via Ctrl-^ or Ctrl-_. This should very closely minic the behavior of other readline/emacs shells, except it doesn't let the user goto a historical entry (A), edit it, goto a different historical entry (B), return to the first (A) and undo the edits.

srp · 2017-07-22T22:54:30Z

@rfourquet,@StefanKarpinski: rebased, thanks!

rfourquet

Thanks for your very nice work! None of my comments are blocking, and can be addressed later if needed.
I will merge in a few days if no-one has objections.

rfourquet · 2017-07-23T10:10:16Z

base/repl/LineEdit.jl

@@ -572,15 +592,18 @@ end
 edit_clear(buf::IOBuffer) = truncate(buf, 0)

 function edit_clear(s::MIState)
+    push_undo(s)


maybe you could push_undo only if the buffer is not empty already? Another possibility could also be to not push a new state in push_undo when it's equal to the last pushed state?

rfourquet · 2017-07-23T10:19:48Z

base/repl/LineEdit.jl

 end

 function edit_yank(s::MIState)
+    push_undo(s)


maybe you could push_undo only if !isempty(s.kill_buffer)?

rfourquet · 2017-07-23T10:22:14Z

base/repl/LineEdit.jl

    edit_insert(buffer(s), s.kill_buffer)
    refresh_line(s)
 end

 function edit_kill_line(s::MIState)
+    push_undo(s)


push_undo only if some characters are deleted?

rfourquet · 2017-07-23T10:28:17Z

base/repl/LineEdit.jl

+push_undo(s) = nothing
+
+function pop_undo(s::PromptState)
+    length(s.undo_buffers) > 0 || return false


isempty(s.undo_buffers) && return false

rfourquet · 2017-07-23T10:33:50Z

test/lineedit.jl

@@ -401,3 +401,96 @@ let
        Base.LineEdit.InputAreaState(0,0), "julia> ", indent = 7)
    @test s == Base.LineEdit.InputAreaState(3,1)
 end
+
+# test Undo
+let


Use @testset ?

rfourquet · 2017-07-23T10:37:26Z

doc/src/manual/interacting-with-julia.md

@@ -172,6 +172,7 @@ to do so).
 | `^Y`                | "Yank" insert the text from the kill buffer                                      |
 | `^T`                | Transpose the characters about the cursor                                        |
 | `^Q`                | Write a number in REPL and press `^Q` to open editor at corresponding stackframe or method |
+| `^/`, `^_`          | Undo |


I didn't see you add a key for ^/, how does it work? (when I tried to rebase last year, I had compilation problems with this key...)
Also, your commit message refers to "Ctrl-^", you should change it.

^/ isn't a valid control character (eg after subtracting 64 you get a negative number), however it appears that vt102 terminal emulators map it to emit ^_ for convience, see: https://apple.stackexchange.com/a/227286

From the terminals I tested on this is accurate that pressing ctrl-/ causes "^_" to appear in the console.

rfourquet · 2017-07-23T10:44:39Z

base/repl/LineEdit.jl

+end
+empty_undo(s) = nothing
+
+function push_undo(s::PromptState)


I would prefer the names of the new functions with an ending !, but it seems to be the style of the file, so better to keep as is for now.

Also, this one is a good candidate for short-form function syntax.

These should probably all be refactored to have !. It wasn't clear to me reading through this that it actually modified the PromptState until I saw the definition. The larger refactoring doesn't have to happen in this PR, but if you're adding a function now, might as well use the standard notation, IMO.

KristofferC · 2017-07-23T10:49:51Z

How do I type Ctrl+/ on a European (Swedish) keyboard? Would Ctrl + Shift + 7 work where Shift + 7 gives /?

fredrikekre · 2017-07-23T11:02:20Z

Ctrl + Shift + 7 works for me @KristofferC 🇸🇪
I have to say that it is not a very convenient keyboard short cut for such keyboard layouts though.

srp · 2017-07-23T16:09:21Z

Does Ctrl+_ work on a Swedish keyboard? If so, you might have to stick with that.

fredrikekre · 2017-07-23T16:10:41Z

_ is Shift + -

srp · 2017-07-23T17:04:38Z

Sounds the same as an english keyboard, i have to hit Ctrl + Shift + - to emit ^_

srp · 2017-07-23T21:26:26Z

@rfourquet I plan on pushing an update addressing some of your comments, but in the process noticed that undo doesn't work right (any more?) with bracketed insert. I'm trying to fix that, but so far it's not entirely straight forward.

rfourquet · 2017-08-07T17:22:20Z

@srp, I rebased your branch and tried to fix undo with the bracketed paste mode, would you mind checking out this branch (just do git checkout srp/repl-undo && make after fetching from the julia repo) and verify if this fixes your concerns? You can use this to update your branch (which now has a small conflict). I wish to merge your work soon!
EDIT: I also updated your commit message with a corrected key-binding.

rfourquet · 2017-08-13T16:29:53Z

Bump - I have few features in the pipeline which will increase the work to resolve conflicts here if we don't merge this first...

rfourquet · 2017-09-03T12:35:40Z

I rebased again and updated the branch "srp/repl-undo" that I linked to above. I plan to merge by tomorrow. But I cannot update the PR directly, and I'm afraid that if I merge it on the command line, github won't recognize this PR has been merged (it happened to me already), someone would have a hint as to how to do it properly?

rfourquet · 2017-09-05T14:33:16Z

Thanks again @srp !

tkelman added the REPL Julia's REPL (Read Eval Print Loop) label Jan 4, 2015

rfourquet reviewed Jan 5, 2015
View reviewed changes

srp force-pushed the repl-undo branch 2 times, most recently from 7e82a32 to da54cf6 Compare January 5, 2015 15:59

srp force-pushed the repl-undo branch 2 times, most recently from 307d525 to cf5dfc8 Compare January 15, 2015 03:58

tkelman mentioned this pull request Aug 2, 2015

Doc hard, and with a vengeance #11943

Merged

blakejohnson reviewed May 16, 2016
View reviewed changes

srp force-pushed the repl-undo branch from cf5dfc8 to 8bc4b00 Compare July 22, 2017 16:52

tkelman reviewed Jul 22, 2017

View reviewed changes

srp force-pushed the repl-undo branch from 8bc4b00 to fe77221 Compare July 22, 2017 20:06

rfourquet approved these changes Jul 23, 2017

View reviewed changes

rfourquet mentioned this pull request Aug 15, 2017

REPL: tab/backspace aligns to 4 #22939

Merged

rfourquet mentioned this pull request Aug 21, 2017

Missing emacs-like key-bindings (available in readline) #8447

Closed

7 tasks

rfourquet mentioned this pull request Sep 5, 2017

REPL: enable "redo" after "undo" #23590

Merged

rfourquet merged commit fe77221 into JuliaLang:master Sep 5, 2017

rfourquet added a commit that referenced this pull request Sep 5, 2017

Merge pull request #9596 from srp/repl-undo

0e1eb8b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REPL/LineEdit to support "undo": part of #8447 #9596

REPL/LineEdit to support "undo": part of #8447 #9596

srp commented Jan 4, 2015 •

edited

Loading

rfourquet commented Jan 5, 2015

rfourquet Jan 5, 2015

rfourquet commented Jan 5, 2015

srp commented Jan 5, 2015

rfourquet commented May 15, 2016

blakejohnson May 16, 2016

rfourquet May 16, 2016

rfourquet commented May 16, 2016

rfourquet commented Jul 17, 2017

StefanKarpinski commented Jul 18, 2017

tkelman Jul 22, 2017

srp Jul 22, 2017

srp commented Jul 22, 2017

rfourquet left a comment

rfourquet Jul 23, 2017

rfourquet Jul 23, 2017

rfourquet Jul 23, 2017

rfourquet Jul 23, 2017

rfourquet Jul 23, 2017

rfourquet Jul 23, 2017

srp Jul 23, 2017

rfourquet Jul 23, 2017

rfourquet Jul 23, 2017

ararslan Aug 15, 2017

KristofferC commented Jul 23, 2017

fredrikekre commented Jul 23, 2017

srp commented Jul 23, 2017

fredrikekre commented Jul 23, 2017

srp commented Jul 23, 2017

srp commented Jul 23, 2017

rfourquet commented Aug 7, 2017 •

edited

Loading

rfourquet commented Aug 13, 2017

rfourquet commented Sep 3, 2017

rfourquet commented Sep 5, 2017

REPL/LineEdit to support "undo": part of #8447 #9596

REPL/LineEdit to support "undo": part of #8447 #9596

Conversation

srp commented Jan 4, 2015 • edited Loading

rfourquet commented Jan 5, 2015

Choose a reason for hiding this comment

rfourquet commented Jan 5, 2015

srp commented Jan 5, 2015

rfourquet commented May 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfourquet commented May 16, 2016

rfourquet commented Jul 17, 2017

StefanKarpinski commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srp commented Jul 22, 2017

rfourquet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC commented Jul 23, 2017

fredrikekre commented Jul 23, 2017

srp commented Jul 23, 2017

fredrikekre commented Jul 23, 2017

srp commented Jul 23, 2017

srp commented Jul 23, 2017

rfourquet commented Aug 7, 2017 • edited Loading

rfourquet commented Aug 13, 2017

rfourquet commented Sep 3, 2017

rfourquet commented Sep 5, 2017

srp commented Jan 4, 2015 •

edited

Loading

rfourquet commented Aug 7, 2017 •

edited

Loading