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

startup: go to buffer 2 if stdin is empty #8576

Merged
merged 2 commits into from Jun 19, 2018

Conversation

Projects
None yet
4 participants
@justinmk
Member

justinmk commented Jun 17, 2018

If stdin is not a TTY we read it into buffer 1, as text. But if the
stdin pipe is empty, Nvim was most likely invoked for some other reason.
DWIM: select buffer 2 (if it exists). Example:

echo file1 | xargs nvim

closes #8560
closes #8561
ref equalsraf/neovim-qt#417

@alok

This comment has been minimized.

alok commented Jun 17, 2018

Would this approach work if, say, we used nvr to open a file from inside the embedded terminal? Wouldn't the empty buffer not have buffer ID 1?

@gibfahn

This comment has been minimized.

gibfahn commented Jun 18, 2018

This will still leave the empty buffer open right? Would it be possible to close it (or not open it at all)?

@justinmk

This comment has been minimized.

Member

justinmk commented Jun 18, 2018

Would this approach work if, say, we used nvr to open a file from inside the embedded terminal?

is that not already working? This isn't going to affect that anyway.

@jamessan

This comment has been minimized.

Member

jamessan commented Jun 18, 2018

This will still leave the empty buffer open right?

Yes

Would it be possible to close it (or not open it at all)?

It has to be opened since that's how we're able to tell the buffer is empty.

This change is simply a heuristic to make nvim foo work as expected when stdin isn't a tty.

From the perspective of nvim, echo | nvim foo is the same as echo foo | xargs nvim. The user can easily tell the difference between the two, but in the former case deleting the initial buffer is hiding useful information (that the command didn't produce any output) from the user.

startup: go to buffer 2 if stdin is empty
If stdin is not a TTY we read it into buffer 1, as text. But if the
stdin pipe is empty, Nvim was most likely invoked for some other reason.
DWIM: select buffer 2 (if it exists). Example:

    echo file1 | xargs nvim

closes #8560
closes #8561
ref equalsraf/neovim-qt#417
@gibfahn

This comment has been minimized.

gibfahn commented Jun 18, 2018

in the former case deleting the initial buffer is hiding useful information (that the command didn't produce any output) from the user.

If the buffer is only deleted if it's empty, then isn't that information inferable from whether the buffer exists?

In any case that seems like useful debugging information, but having an empty open buffer every time neovim opens is a bit of a pain.

A lot of scripts do <command> | xargs $VISUAL, which means that they will now have the open buffer. For my own shortcuts I could do something like $VISUAL $(<command>), but then you run into the max number of positional arguments limit.

@jamessan

This comment has been minimized.

Member

jamessan commented Jun 18, 2018

A lot of scripts do <command> | xargs $VISUAL, which means that they will now have the open buffer.

Sure, but what harm does the open buffer do?

startup: delete empty stdin buffer if other files were opened
DWIM: avoid empty buffer 1 when stdin was empty. If other files were
specified at startup, we assume that stdin is only accidentally
not-a-TTY: user did not intend to send text from it.

ref #8560
ref #8561
@gibfahn

This comment has been minimized.

gibfahn commented Jun 19, 2018

Sure, but what harm does the open buffer do?

Fair question, I know a lot of people don't keep track of buffers, but if you use a buffer tabline plugin like buftabline, vim-bufferline or wintabs, then it means you always have this annoying extra tab.

This may not seem like a big deal, but it's a big enough annoyance to make me switch back to 0.2.1.

image

@justinmk justinmk force-pushed the justinmk:startup-empty-stdin branch 2 times, most recently from a946b3e to 9625e9d Jun 19, 2018

@justinmk

This comment has been minimized.

Member

justinmk commented Jun 19, 2018

I agree with @jamessan , but my sense is that the empty buffer might cause more confusion that it solves. So latest commit deletes it. We can revert if it causes problems.

it's a big enough annoyance to make me switch back to 0.2.1.

That throws out quite a lot of improvements in 0.3.0, particularly some major job-control bugs were fixed on 0.3.0, as well as performance improvements and many other details.

@gibfahn

This comment has been minimized.

gibfahn commented Jun 19, 2018

That throws out quite a lot of improvements in 0.3.0, particularly some major job-control bugs were fixed on 0.3.0, as well as performance improvements and many other details.

Yeah, that's exactly my point. I know 0.3.0 has lots of great improvements, which is why I was keen to update. But this change causes enough mental distraction for me to make me prefer 0.2.1.

@justinmk justinmk merged commit 74d19f6 into neovim:master Jun 19, 2018

1 of 3 checks passed

QuickBuild Build pr-8576 finished with status FAILED
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@justinmk justinmk deleted the justinmk:startup-empty-stdin branch Jun 19, 2018

justinmk added a commit that referenced this pull request Jul 18, 2018

NVIM v0.3.1
FEATURES:
07499a8 #8709 man.vim: C highlighting for EXAMPLES section
07f82ad #8699 TUI: urxvt: also send xterm focus-reporting seqs
40911e4 #8616 API: emit nvim_buf_lines_event from :terminal
c46997a #8546 fillchars: Add "eob" flag

FIXES:
74d19f6 #8576 startup: avoid blank stdin buffer if other files were opened
4874214 #8737 Only waitpid() for processes that we care about
cd6e7e8 #8743 Check all child processes for exit in SIGCHLD handler
c230ef2 #8746 channel.c: Prevent channel_destroy_early() from freeing uninitialized rpc stuff
0ed8b12 #8681 transstr_buf: fix length comparison
d241f27 #8708 TUI: Fix standout mode
9afed40 #8698 man.vim: fix for mandoc
e889640 #8682 provider/node: npm --loglevel silent
1cbc830 #8613 API: nvim_win_set_cursor: set curswant
bf6048e #8628 checkhealth: Python: fix VIRTUAL_ENV check
3cc3506 #8528 checkhealth: node.js: also search yarn

CHANGES:
b751449 #8619 defaults: shortmess+=F
1248178 #8578 highlight: high-priority CursorLine if fg is set.
01570f1 #8726 terminal: handle &confirm and :confirm on unloading
56065bb #8721 screen: truncate showmode messages
bf2460e #7551 buffer: fix copying :setlocal options
c1c14fa #8520 Ex mode: always "improved" (gQ)
050f397 #7992 options: remove 'maxcombine` option (always 6)

INTERNAL:
463da84 #7992 screen: use UTF-8 representation

UtkarshMe added a commit to UtkarshMe/neovim that referenced this pull request Jul 28, 2018

NVIM v0.3.1
FEATURES:
07499a8 neovim#8709 man.vim: C highlighting for EXAMPLES section
07f82ad neovim#8699 TUI: urxvt: also send xterm focus-reporting seqs
40911e4 neovim#8616 API: emit nvim_buf_lines_event from :terminal
c46997a neovim#8546 fillchars: Add "eob" flag

FIXES:
74d19f6 neovim#8576 startup: avoid blank stdin buffer if other files were opened
4874214 neovim#8737 Only waitpid() for processes that we care about
cd6e7e8 neovim#8743 Check all child processes for exit in SIGCHLD handler
c230ef2 neovim#8746 channel.c: Prevent channel_destroy_early() from freeing uninitialized rpc stuff
0ed8b12 neovim#8681 transstr_buf: fix length comparison
d241f27 neovim#8708 TUI: Fix standout mode
9afed40 neovim#8698 man.vim: fix for mandoc
e889640 neovim#8682 provider/node: npm --loglevel silent
1cbc830 neovim#8613 API: nvim_win_set_cursor: set curswant
bf6048e neovim#8628 checkhealth: Python: fix VIRTUAL_ENV check
3cc3506 neovim#8528 checkhealth: node.js: also search yarn

CHANGES:
b751449 neovim#8619 defaults: shortmess+=F
1248178 neovim#8578 highlight: high-priority CursorLine if fg is set.
01570f1 neovim#8726 terminal: handle &confirm and :confirm on unloading
56065bb neovim#8721 screen: truncate showmode messages
bf2460e neovim#7551 buffer: fix copying :setlocal options
c1c14fa neovim#8520 Ex mode: always "improved" (gQ)
050f397 neovim#7992 options: remove 'maxcombine` option (always 6)

INTERNAL:
463da84 neovim#7992 screen: use UTF-8 representation
@gibfahn

This comment has been minimized.

gibfahn commented Aug 2, 2018

@justinmk just wanted to say thanks for fixing this, works like a charm in v0.3.1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment