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

[RDY] Builtin terminal emulation #2076

Merged
merged 13 commits into from Mar 26, 2015
Merged

Conversation

@tarruda
Copy link
Member

tarruda commented Feb 28, 2015

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
Copy link
Contributor

ZyX-I commented Feb 28, 2015

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
Copy link
Member Author

tarruda commented Feb 28, 2015

@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
Copy link
Member

bfredl commented Feb 28, 2015

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
Copy link
Member

bfredl commented Feb 28, 2015

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
Copy link
Member Author

tarruda commented Feb 28, 2015

@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
Copy link
Contributor

ZyX-I commented Feb 28, 2015

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

@tarruda
Copy link
Member Author

tarruda commented Feb 28, 2015

@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
Copy link
Member

bfredl commented Feb 28, 2015

@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
Copy link
Member

justinmk commented Feb 28, 2015

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
Copy link
Member

bfredl commented Feb 28, 2015

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

@justinmk
Copy link
Member

justinmk commented Feb 28, 2015

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

@justinmk justinmk added the terminal label Feb 28, 2015
@jszakmeister
Copy link
Member

jszakmeister commented Feb 28, 2015

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
Copy link
Contributor

cweagans commented Feb 28, 2015

This is awesome :)

@Tranquility
Copy link
Contributor

Tranquility commented Feb 28, 2015

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
Copy link

khalidchawtany commented Feb 28, 2015

This is SUPER AWESOME :)

@mudox
Copy link

mudox commented Mar 1, 2015

👍 awesome feature from Neovim.

@rainerborene
Copy link
Contributor

rainerborene commented Mar 1, 2015

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
Copy link
Contributor

jebaum commented Mar 1, 2015

@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
Copy link
Member Author

tarruda commented Mar 1, 2015

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

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);

This comment has been minimized.

Copy link
@oni-link

oni-link Mar 1, 2015

Contributor

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

This comment has been minimized.

Copy link
@tarruda

tarruda Mar 1, 2015

Author Member

👍

linenr_T src_linenr = (linenr_T)term->sb_current + 1;
linenr_T tgt_linenr = (linenr_T)term->sb_current;
char *line = xstrdup((char *)ml_get(src_linenr));
ml_append(tgt_linenr, (uint8_t *)line, 0, false);

This comment has been minimized.

Copy link
@oni-link

oni-link Mar 1, 2015

Contributor

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

This comment has been minimized.

Copy link
@tarruda

tarruda Mar 1, 2015

Author Member

👍

@tarruda
Copy link
Member Author

tarruda commented Mar 1, 2015

@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
Copy link
Member

bfredl commented Mar 1, 2015

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
Copy link
Member Author

tarruda commented Mar 1, 2015

@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
Copy link
Member Author

tarruda commented Mar 1, 2015

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 😄

@tarruda
Copy link
Member Author

tarruda commented Mar 25, 2015

@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
Copy link

ghost commented Mar 25, 2015

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.

tarruda and others added 13 commits Feb 27, 2015
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)
@tarruda tarruda force-pushed the tarruda:builtin-terminal-emulation branch 3 times, most recently from f4e9af1 to 2aa2513 Mar 25, 2015
@tarruda tarruda merged commit 2aa2513 into neovim:master Mar 26, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jszakmeister jszakmeister removed the RDY label Mar 26, 2015
tarruda added a commit that referenced this pull request Mar 26, 2015
@trusktr
Copy link

trusktr commented Mar 26, 2015

@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.

@neovim neovim locked and limited conversation to collaborators Mar 26, 2015
ZyX-I referenced this pull request in ZyX-I/neovim Oct 5, 2015
Current busted output type does not allow determining failing test.
@fwalch

This comment has been minimized.

Copy link
Member

fwalch commented on src/nvim/terminal.c in cdedd89 Nov 3, 2015

@tarruda LeoNerd on Gitter:

please change this line to for(int color_index = 255; color_index >= 0; color_index--) so you'll find the "correct" black

(thought it might get lost otherwise, there's a lot going on on the channel atm).

This comment has been minimized.

Copy link
Member Author

tarruda replied Nov 4, 2015

Thanks

Do you mind creating an issue for this? I didn't follow the gitter discussion but will check it out later

This comment has been minimized.

Copy link
Member

fwalch replied Nov 4, 2015

Sure! (should have done that from the beginning)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.