[RDY] Builtin terminal emulation #2076

tarruda · 2015-02-28T15:45:53Z

This PR uses libvterm(big thanks to @leonerd for that project) to implement a full VT220 terminal emulator using Neovim buffers and windows. ~~It will also refactor the calling of shell commands to use this facility, fixing #1044 #1386 #1496 #1716 (all caused by an issue introduced in #1365)~~~(I've decided to not touch the generic os_call_shell for now)

Still a WIP but the :terminal ex command can already be used, here's a quick rundown:

:term [prog/args] will open a new special buffer "connected" to a pseudo terminal running prog/args
any command that would enter insert mode will focus the terminal buffer instead.
when focused a new mode is entered where all keys are passed directly to the program. The <c-\><c-n> combination will return to normal mode.
new terminal-only mappings can be added with the "t" prefix(eg: tnoremap <c-a> <c-\><c-n> exit terminal focus using ctrl+a)

@justinmk An observation about the new Terminal class: It is not coupled to job control so we can use it for implementing #901 and eventually remove all blocking messages(press enter to continue) from nvim(We could have a Terminal instance created at startup and feed all messages to it, for example)

ZyX-I · 2015-02-28T16:04:57Z

Wondering how it deals with https://bugs.launchpad.net/libvterm/+bug/1412093. Also what is the $TERM value (see https://bugs.launchpad.net/libvterm/+bug/1412091 for why this is relevant)?

tarruda · 2015-02-28T16:07:02Z

@ZyX-I I've directed the URL to a fork that fixes these messages: https://github.com/neovim/libvterm/commits/cflags-override( @leonerd will probably merge them once he gets back from holidays)

As for https://bugs.launchpad.net/libvterm/+bug/1412091 , libvterm aims have xterm compliance but its not there yet.

bfredl · 2015-02-28T16:52:30Z

Very nice! Is it possible to somehow paste/send stdin data to the vte ? (perhaps p in "terminal normal mode" could be remapped to "paste to stdin" )

bfredl · 2015-02-28T16:57:05Z

Shouldn't the "real" terminal cursor follow the vte cursor when in "insert" mode ? Otherwise it looks weird when e.g. using input methods

tarruda · 2015-02-28T17:04:37Z

@bfredl I plan to fully integrate this with vimscript job control API(it will be possible to control debuggers, paste data, etc)

Im still not sure about the cursor behavior(perhaps it should simply become invisible when in normal mode?)

ZyX-I · 2015-02-28T17:07:00Z

Invisible cursor looks like neovim doing something. Always invisible cursor looks like neovim hang forever.

tarruda · 2015-02-28T17:09:58Z

@ZyX-I I meant that the vterm cursor should be invisible when in normal mode and also the other way around, so there will only be one visible cursor at a time

bfredl · 2015-02-28T17:21:56Z

@tarruda yes, and when in vterm mode the vterm cursor should be forwarded as ui cursor events. This is required to make the preview buffer of input methods to show up at the right place (that is, at the cursor)

justinmk · 2015-02-28T17:41:21Z

eventually remove all blocking messages(press enter to continue) from nvim

@tarruda Is this in reference to #1802? I'm not attached to that PR, but the purpose of that PR is to give plugins (and core) the option to emit non-blocking messages, not to eliminate them entirely. The next (future) step I was envisioning was for message.c to raise "notification" events which may be optionally handled by UIs instead of just directly writing to the screen (then GUIs could have a notifications area that is not constricted by cmdheight, for example).

We could offer an option to eliminate all "Press ENTER" messages, but I'm not sure this would actually be a good thing for users to do. The "pager" that Vim emulates for messages is quite handy for messages that the user wants to briefly check, and the fact that it can be dismissed by pressing any normal-mode key (which is then executed) is a feature that would be missed.

So, #1802 has a small imperfection that I still need to fix, but perhaps we should discuss whether it is worth continuing. I would really like to wrap it up this weekend one way or other.

bfredl · 2015-02-28T17:43:51Z

Btw, the obligatory nvim-within-nvim test already works (probably not practically useful, but reckon it is a good "stress test" ) 👍

justinmk · 2015-02-28T17:44:20Z

@bfredl Really cool :) Going to try this PR now

jszakmeister · 2015-02-28T17:51:04Z

Is this in reference to #1802? I'm not attached to that PR, but the purpose of that PR is to give plugins (and core) the option to emit non-blocking messages, not to eliminate them entirely. The next (future) step I was envisioning was for message.c to raise "notification" events which may be optionally handled by UIs instead of just directly writing to the screen (then GUIs could have a notifications area that is not constricted by cmdheight, for example).

I agree @justinmk. I don't think it's wise to remove them all, they do serve a purpose and sometimes you really do want to call attention to the user.

cweagans · 2015-02-28T19:17:31Z

This is awesome :)

Tranquility · 2015-02-28T22:08:20Z

Awesome! I really don't know how I would use this feature but it's impressive how well it works.

Some things I noticed:

long running processes keep running after vim closes and print error messages because the lnums are invalid
moving word wise with alt-keys does not work
if I am in normal mode and the cursor is below the vterm cursor (someplace without text) I need capital i to focus and i only prints E21: Cannot make changes, 'modifiable' is off
in command mode the vim cursor jumps into the buffer if something changed. it looks like it's no longer in command mode although typing gets the cursor back

khalidchawtany · 2015-02-28T22:57:15Z

This is SUPER AWESOME :)

mudox · 2015-03-01T01:08:41Z

👍 awesome feature from Neovim.

rainerborene · 2015-03-01T04:22:46Z

Just one thing, when I type <C-l> the whole buffer gets empty instead of clearing and show only one line like on most shells. I think this might be related with the size of the pseudo terminal when you have more than one buffer opened.

Fucking awesome feature.

jebaum · 2015-03-01T04:34:02Z

@rainerborene on my system I don't see either of the bugs you describe with nvim -u NONE. you should try again with that to see if something in your nvimrc is causing an issue

tarruda · 2015-03-01T04:36:57Z

Also when you run :terminal without arguments nvim crashes.

Whats the value of your sh option?(when running without arguments it will run your shell). But it should not crash anyway so I will investigate

oni-link · 2015-03-01T12:01:42Z

src/nvim/terminal.c

+    if (linenr <= term->buf->b_ml.ml_line_count) {
+      ml_replace(linenr, (uint8_t *)line, false);
+    } else {
+      ml_append(linenr - 1, (uint8_t *)line, 0, false);


We need a free(line); here. ml_append seems to copy line.

tarruda · 2015-03-01T12:44:55Z

@justinmk / @jszakmeister The states where user feedback is required can be easily emulated with the new terminal module. For example, here's how I plan to enhance bang commands with this new feature while still maintaining vimscript compatibility:

Bang commands will open a terminal buffer to run the shell
If the current buffer is modified, run the terminal buffer in a split window, if not it will replace the current buffer.
When the program completes, pressing enter will return the cursor to the previous buffer.

Note that the keypress sequence is still the same, the differences are:

the bang command will no longer block the user until complete if the user escapes the window
If this happens the bang command will simply return prematurely and the process is still running like :terminal was invoked directly.(since this is not even possible right now, there wont be compatibility problems)
the user can access the command output after it completes

A similar concept can be applied to message output:

When nvim starts, a hidden terminal buffer is created(without a controlling job, of course)
When the user must be notified of something, the message is appended to the hidden terminal buffer and then different actions can be taken:
- If the message fits in a single line and not an error(an echo command for example), the behavior can persist the same: show the message in the command line
- If its a message that would normally require feedback from the user(a :highlight command for example), a split window is opened with the message appended and the "press RETURN" prompt which will close the window bring the user back to the previous buffer.

The advantage of using this approach may not be obvious but here are some:

We'll get rid of a lot of code in message.c. Implementation of message paging is trivial with the new module.
The user will be able to copy the text from previous messages from the scrollback(:messages will simply open a split with the terminal buffer)
We'll get rid of of the HITRETURN, ASKMORE, CONFIRM and EXTERMCMD states. The new TERMINAL_FOCUS state can be used to emulate any of them.
Neovim will be able to handle deferred events in more situations(no problem in handling vim_eval requests while in the wait_return() prompt for example.

bfredl · 2015-03-01T12:48:43Z

Neovim will be able to handle deferred events in more situations(no problem in handling vim_eval requests while in the wait_return() prompt for example.

👍

tarruda · 2015-03-01T12:58:04Z

@Tranquility

Awesome! I really don't know how I would use this feature but it's impressive how well it works.

With time, the numerous use cases for this feature will be clear, but here's one idea: This allows another level of integration between text mode programs and Neovim. For example, we'll be able to make gdb respond to nvim mappings and change file/line numbers when a breakpoint is hit(Technically this might be possible with tmux but not without numerous hacks)

long running processes keep running after vim closes and print error messages because the lnums are invalid

👍 needs to be fixed

moving word wise with alt-keys does not work

Not sure what you mean, but when you leave terminal focus(Ctrl+w, esc) you can move around the scrollback just like any other buffer)

if I am in normal mode and the cursor is below the vterm cursor (someplace without text) I need capital i to focus and i only prints E21: Cannot make changes, 'modifiable' is off

Thats expected, the terminal buffer cannot receive direct modifications from the user: The key presses are forwarded to the underlying program which outputs redraw instructions. This is how any terminal behaves(you cannot delete lines from tmux panes for example)

in command mode the vim cursor jumps into the buffer if something changed. it looks like it's no longer in command mode although typing gets the cursor back

👍 Its just a redraw glitch, should be easy to fix.

tarruda · 2015-03-01T13:04:47Z

Btw, the obligatory nvim-within-nvim test already works (probably not practically useful, but reckon it is a good "stress test" ) 👍

I know, this was one of the first things I tried with 5 levels of nesting 😄

ghost · 2015-03-25T19:09:17Z

@tarruda do you plan on documenting this soonish? I assume you have a lot to do already, but I can't think of a better person to document this than the person who wrote it. If I can help in any way just ask.

tarruda · 2015-03-25T19:11:21Z

@tarruda do you plan on documenting this soonish? I assume you have a lot to do already, but I can't think of a better person to document this than the person who wrote it. If I can help in any way just ask.

Definitely. I didnt do it yet because I will create another PR after this one that will contain changes to the job API, so I'd rather document everything in one step.

ghost · 2015-03-25T19:20:50Z

Definitely. I didnt do it yet because I will create another PR after this one that will contain changes to the job API, so I'd rather document everything in one step.

👍 Thanks for clarifying.

This is required to avoid event loop recursion due to indirect calls to os_breakcheck by screenalloc

This ifndef causes problems when including fileio headers.

Most internal functions to modify buffers operate on the current buffer and require temporary switchs. This macro is a temporary workaround until a cleaner refactoring of the internal API is performed.

This commit integrates libvterm with Neovim and implements a terminal emulator with nvim buffers as the display mechanism. Terminal buffers can be created using any of the following methods: - Opening a file with name following the "term://[${cwd}//[${pid}:]]${cmd}" URI pattern where: - cwd is the working directory of the process - pid is the process id. This is just for use in session files where a pid would have been assigned to the saved buffer title. - cmd is the command to run - Invoking the `:terminal` ex command - Invoking the `termopen` function which returns a job id for automating the terminal window. Some extra changes were also implemented to adapt with terminal buffers. Here's an overview: - The `main` function now sets a BufReadCmd autocmd to intercept the term:// URI and spawn the terminal buffer instead of reading the file. - terminal buffers behave as if the following local buffer options were set: - `nomodifiable` - `swapfile` - `undolevels=-1` - `bufhidden=hide` - All commands that delete buffers(`:bun`, `:bd` and `:bw`) behave the same for terminal buffers, but only work when bang is passed(eg: `:bwipeout!`) - A new "terminal" mode was added. A consequence is that a new set of mapping commands were implemented with the "t" prefix(tmap, tunmap, tnoremap...) - The `edit` function(which enters insert mode) will actually enter terminal mode if the current buffer is a terminal - The `put` operator was adapted to send data to the terminal instead of modifying the buffer directly. - A window being resized will also trigger a terminal resize if the window displays the terminal.

- Free memory allocated for job data when the job table is full.

Since all reads are queued by the event loop, we must also queue the exit event, or else the process_close function can close the job streams before received data is processed.

- Modify tty-test to allow easier control over the terminal - Add a new directory with various terminal tests/specifications - Remove a pending job/pty test. - Flush stdout in Screen:snapshot_util() (avoid waiting for the test to finish) - Replace libuv sigwinch watcher by a sigaction handler. libuv randomly fails to deliver signals on OSX. Might be related to the problem fixed by @bbcddc55ee1e5605657592644be0102ed3a5f104 (under the hoods, libuv uses a pipe to deliver signals to the main thread, which might be blocking in some situations)

trusktr · 2015-03-26T03:37:43Z

@ZyX-I

Command-line can easily be colored using the other means

I wouldn't want to use all those other means because those means are already part of Neovim, and we can use them on the terminal buffer. It's so convenient.

Current busted output type does not allow determining failing test.

marvim added the WIP label Feb 28, 2015

tarruda mentioned this pull request Feb 28, 2015

[RFC] job control: allow not sending final newline in jobsend #2073

Merged

This was referenced Feb 28, 2015

Upcoming Newsletter neovim/neovim.github.io#82

Merged

Terminal emulator inside of vim #51

Closed

justinmk added the terminal label Feb 28, 2015

oni-link reviewed Mar 1, 2015
View reviewed changes

tarruda and others added 13 commits March 25, 2015 18:57

syntax: Fix possible invalid read in hl_combine_attr

5cf3dec

map: Add map_clear method

05c030c

map: Add Map(int, int)

51c69b8

ui: Schedule screen refreshs to run in the event loop

05d60c4

This is required to avoid event loop recursion due to indirect calls to os_breakcheck by screenalloc

api: Move switch_to_win_for_buf/restore_win_for_buf to api helpers

071aca9

vim.h: remove #ifndef EINTR

2b90aed

This ifndef causes problems when including fileio headers.

buffer: Add WITH_BUFFER macro to simplify global buffer modification

8b6b06c

Most internal functions to modify buffers operate on the current buffer and require temporary switchs. This macro is a temporary workaround until a cleaner refactoring of the internal API is performed.

buffer: Move b_p_ma(modifiable) checks into the MODIFIABLE macro

b16c5bf

event: Ensure the loop is stopped when a event is pushed.

6f471fa

job: Fix memory errors

727f524

- Free memory allocated for job data when the job table is full.

job: Close the process in a queued event handler

710002c

Since all reads are queued by the event loop, we must also queue the exit event, or else the process_close function can close the job streams before received data is processed.

tarruda force-pushed the builtin-terminal-emulation branch 3 times, most recently from f4e9af1 to 2aa2513 Compare March 26, 2015 00:58

tarruda merged commit 2aa2513 into neovim:master Mar 26, 2015

jszakmeister removed the RDY label Mar 26, 2015

tarruda added a commit that referenced this pull request Mar 26, 2015

Merge PR #2076 'Builtin terminal emulation'

a6e53a3

tarruda mentioned this pull request Mar 26, 2015

[RDY] Refactor/enhance job api #2247

Merged

grtlr mentioned this pull request Mar 26, 2015

build failed, ninja: build stopped: subcommand failed. #2248

Closed

michaldarda mentioned this pull request Mar 26, 2015

Add neovim terminal strategy vim-test/vim-test#21

Closed

This was referenced Mar 26, 2015

wildmenu causes the statusline to appear temporarily #2255

Closed

<C-Left> and <C-Right> are handled incorrectly #2254

Closed

neovim locked and limited conversation to collaborators Mar 26, 2015

ZyX-I referenced this pull request in ZyX-I/neovim Oct 5, 2015

travis: Use gtest for busted output type

99aef30

Current busted output type does not allow determining failing test.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RDY] Builtin terminal emulation #2076

[RDY] Builtin terminal emulation #2076

tarruda commented Feb 28, 2015

ZyX-I commented Feb 28, 2015

tarruda commented Feb 28, 2015

bfredl commented Feb 28, 2015

bfredl commented Feb 28, 2015

tarruda commented Feb 28, 2015

ZyX-I commented Feb 28, 2015

tarruda commented Feb 28, 2015

bfredl commented Feb 28, 2015

justinmk commented Feb 28, 2015

bfredl commented Feb 28, 2015

justinmk commented Feb 28, 2015

jszakmeister commented Feb 28, 2015

cweagans commented Feb 28, 2015

Tranquility commented Feb 28, 2015

khalidchawtany commented Feb 28, 2015

mudox commented Mar 1, 2015

rainerborene commented Mar 1, 2015

jebaum commented Mar 1, 2015

tarruda commented Mar 1, 2015

oni-link Mar 1, 2015

tarruda Mar 1, 2015

tarruda commented Mar 1, 2015

bfredl commented Mar 1, 2015

tarruda commented Mar 1, 2015

tarruda commented Mar 1, 2015

ghost commented Mar 25, 2015

tarruda commented Mar 25, 2015

ghost commented Mar 25, 2015

trusktr commented Mar 26, 2015

[RDY] Builtin terminal emulation #2076

[RDY] Builtin terminal emulation #2076

Conversation

tarruda commented Feb 28, 2015

ZyX-I commented Feb 28, 2015

tarruda commented Feb 28, 2015

bfredl commented Feb 28, 2015

bfredl commented Feb 28, 2015

tarruda commented Feb 28, 2015

ZyX-I commented Feb 28, 2015

tarruda commented Feb 28, 2015

bfredl commented Feb 28, 2015

justinmk commented Feb 28, 2015

bfredl commented Feb 28, 2015

justinmk commented Feb 28, 2015

jszakmeister commented Feb 28, 2015

cweagans commented Feb 28, 2015

Tranquility commented Feb 28, 2015

khalidchawtany commented Feb 28, 2015

mudox commented Mar 1, 2015

rainerborene commented Mar 1, 2015

jebaum commented Mar 1, 2015

tarruda commented Mar 1, 2015

oni-link Mar 1, 2015

Choose a reason for hiding this comment

tarruda Mar 1, 2015

Choose a reason for hiding this comment

tarruda commented Mar 1, 2015

bfredl commented Mar 1, 2015

tarruda commented Mar 1, 2015

tarruda commented Mar 1, 2015

ghost commented Mar 25, 2015

tarruda commented Mar 25, 2015

ghost commented Mar 25, 2015

trusktr commented Mar 26, 2015