Skip to content

[RFC] tui: job-control: use saved termios for pty jobs #6992

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[RFC] tui: job-control: use saved termios for pty jobs #6992

wants to merge 1 commit into from

Conversation

justinmk
Copy link
Member

@justinmk justinmk commented Jul 10, 2017 •
edited
Loading

Related: #6938

The intention is to make :terminal more consistent with the user's actual terminal, to further eliminate "quirks". But this change also affects jobstart(..., {'pty':v:true}), not sure if that's a good idea.

On startup, if running in a terminal, save the termios properties.
Use the saved termios for :terminal and jobstart() pty jobs.

This won't affect nvim spawned outside of a terminal.

questions:

  • This affects :terminal and jobstart(..., {'pty':v:true}).
    Should we be more conservative for jobstart(..., {'pty':v:true}) (e.g.
    pass NULL to forkpty() and let the OS defaults prevail)?
    • Note: iutf8 would not be set in that case.
  • Ultimately we may need to provide options to let users control options on jobstart(..., {'pty':v:true}) jobs, or at least a flag that controls whether termios is "inherited" or uses "sane defaults".

cc @bfredl @oni-link @jdebp @jamessan @hardenedapple

@justinmk
tui: job-control: use saved termios for pty jobs
362fdf0 
On startup, if running in a terminal, save the termios properties.
Use the saved termios for `:terminal` and `jobstart()` pty jobs.

This won't affect nvim spawned outside of a terminal.

questions:

- This affects `:terminal` and `jobstart({'pty':v:true})`.
  Should we be more conservative for `jobstart({'pty':v:true})` (e.g.
  pass NULL to forkpty() and let the OS defaults prevail)?
  - Note: `iutf8` would not be set in that case.
@justinmk justinmk changed the title tui: job-control: use saved termios for pty jobs [WIP] tui: job-control: use saved termios for pty jobs Jul 10, 2017
@justinmk justinmk changed the title [WIP] tui: job-control: use saved termios for pty jobs [RFC] tui: job-control: use saved termios for pty jobs Jul 10, 2017
@marvim marvim added the RFC label Jul 10, 2017
@hardenedapple
Copy link
Contributor

To give an opinion from my use-cases

For me, being able to control jobstart() termios settings sometime in the future sounds pretty cool.
I currently resort to running a bash script containing an stty command before execing into the long-running shell I plan to use, so I know there's at least one user for that functionality.

As a minor opinion, I would prefer a known and documented group of termios settings for jobstart() rather than using the OS defaults or the users settings.
It may be that OS defaults don't vary enough between machines for there to be a practical difference, or it may be that having the same termios settings on different machines would cause some problems, but I guess plugin authors would want a single known behaviour more often than a slightly modified behaviour to fit with the OS.
I could be wrong there, and it's not a strong opinion.

I agree that consistency between the users terminal and :terminal is the most important consequence of the termios settings, jobstart() has a workaround while :terminal doesn't.

good idea!

@justinmk justinmk mentioned this pull request Jul 18, 2017
Closed
justinmk added a commit to justinmk/neovim that referenced this pull request Mar 20, 2018
@justinmk
doc: job/channel, misc
5ee0c17 
document termios defaults
ref neovim#6992
@justinmk
Copy link
Member Author

This was merged in 9acd7bf.

I would prefer a known and documented group of termios settings for jobstart() rather than using the OS defaults or the users settings.

Added some minimal documentation regarding that in #7783

@justinmk justinmk closed this Mar 20, 2018
@justinmk justinmk removed the RFC label Mar 20, 2018
@justinmk justinmk deleted the termios-pty branch March 20, 2018 09:04
justinmk added a commit to justinmk/neovim that referenced this pull request Mar 24, 2018
@justinmk
doc: job/channel, misc
e458273 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request Mar 27, 2018
@justinmk
doc: job/channel, misc
4f94f3f 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request Apr 6, 2018
@justinmk
doc: job/channel, misc
1d3fdaf 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request Apr 15, 2018
@justinmk
doc: job/channel, misc
5201900 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request Apr 22, 2018
@justinmk
doc: job/channel, misc
4b9ce70 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request Apr 25, 2018
@justinmk
doc: job/channel, misc
23b2e25 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request May 1, 2018
@justinmk
doc: job/channel, misc
99b0793 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request May 16, 2018
@justinmk
doc: job/channel, misc
14a5262 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request May 22, 2018
@justinmk
doc: job/channel, misc
4f76558 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request Jun 10, 2018
@justinmk
doc: job/channel, misc
49068a4 
document termios defaults
ref neovim#6992
justinmk added a commit to justinmk/neovim that referenced this pull request Jun 10, 2018
@justinmk
doc: job/channel, misc
f4cfb0d 
doc: termios defaults. ref neovim#6992
doc: :help shell-powershell
doc: provider: Python minimum version is 2.7, 3.4
doc: remove :!start special-case. neovim#5844

mention neovim#7917 change which accepts empty Array for Dictionary parameter
justinmk added a commit to justinmk/neovim that referenced this pull request Jun 10, 2018
@justinmk
doc: job/channel, misc neovim#7783
3c5f681 
doc: termios defaults. ref neovim#6992
doc: :help shell-powershell
doc: provider: Python minimum version is 2.7, 3.4
doc: remove :!start special-case. neovim#5844
doc: mention neovim#7917 change which accepts empty Array for Dictionary parameter
doc: <Cmd> pseudokey
doc: lmap change neovim#5658
doc: -s, -es
justinmk added a commit to justinmk/neovim that referenced this pull request Jun 10, 2018
@justinmk
doc: job/channel, misc neovim#7783
f72f638 
doc: termios defaults. ref neovim#6992
doc: :help shell-powershell
doc: provider: Python minimum version is 2.7, 3.4
doc: remove :!start special-case. neovim#5844
doc: mention neovim#7917 change which accepts empty Array for Dictionary parameter
doc: <Cmd> pseudokey
doc: lmap change neovim#5658
doc: -s, -es
@justinmk justinmk mentioned this pull request Jul 21, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@justinmk @hardenedapple @marvim