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

Keep arguments to nvim before any filenames; fixes #417. #419

Merged
merged 2 commits into from Jun 22, 2018
Merged

Keep arguments to nvim before any filenames; fixes #417. #419

merged 2 commits into from Jun 22, 2018

Conversation

roflcopter4
Copy link
Contributor

@roflcopter4 roflcopter4 commented Jun 20, 2018

As noted in issue #417, Neovim v3.* inexplicably seems to open an empty buffer when option parameters appear after filenames. This could be solved by adding a -- parameter before the filenames, but the easier thing to do is to simply move nvim-qt's additional arguments before any user specified ones and let Neovim sort out this strange bug. Although it may not be 100% reproducible, at least a few people besides myself have noticed this, and this tiny patch seems to fix it without causing any problems I can think of, so it should be harmless at worst.

I also added one small comment (and initially committed with spaces instead of tabs: fixed).

EDIT: I have absolutely no idea why the build failed in the Windows test environment. I didn't change anything that has anything to do with that... I barely changed anything at all.

@justinmk
Copy link
Contributor

Did you try latest Nvim HEAD? As mentioned in #417 (comment) I am wondering if neovim/neovim#8576 (which was merged yesterday) resolves this without the need for a workaround in nvim-qt.

This would be helpful to know because it may affect other GUIs.

@@ -195,12 +195,12 @@ NeovimConnector* NeovimConnector::spawn(const QStringList& params, const QString
{
QProcess *p = new QProcess();
QStringList args;
// Neovim accepts a `--' argument after which only filenames are passed.
// If the user has supplied it, our arguments must appear before.
Copy link
Contributor

@justinmk justinmk Jun 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is definitely true, and if something like this was being constructed before:

nvim -- --embed --headless

Then nvim should have opened files literally named --embed and --headless.

@roflcopter4
Copy link
Contributor Author

roflcopter4 commented Jun 20, 2018

It was certainly worth a shot to recompile nvim (which in Gentoo takes all of emerge neovim to source, compile and install -- I love Gentoo). It didn't help. Nvim-qt with my little patch works, but without it a blank buffer is still opened. The only other Nvim gui that that I can think of that isn't utter garbage is Eovim (which is too buggy to use, and uses EFL which has awful font support) doesn't suffer any problems either way.

The arguments nvim-qt was supplying previously looked like

--cmd "let &rtp.=',/usr/local/share/nvim-qt/runtime'" --cmd "set termguicolors" FILE_GOES_HERE --embed --headless

while this patch changes it to

--embed --headless --cmd "let &rtp.=',/usr/local/share/nvim-qt/runtime'" --cmd "set termguicolors" FILE_GOES_HERE

(I hacked in a debug statement that dumped the arguments to the command line just to be sure). In neither case does -- appear. That's why I consider it a bug in nvim. Both should work without problems. The only explanation I can think of is a problem in neovim's option parsing. Regardless, putting everything before the files is a functional temporary fix.

@justinmk
Copy link
Contributor

Thanks. It definitely would be a Nvim bug, but I can't reproduce it. I tried this:

NVIM_LISTEN_ADDRESS=foo nvim --cmd "let &rtp.=''" --cmd "set termguicolors" README.md --embed --headless

Then connect to the foo server with the python client:

>>> import neovim
>>> n = neovim.attach('socket', path='foo')
>>> n.request('nvim_command_output', 'ls!')
'  1 %a   "README.md"                    line 1'

@roflcopter4
Copy link
Contributor Author

I couldn't say why you can't seem to reproduce this. It doesn't make much sense to me in general anyway. Very strange. Are you using the latest build of Neovim?

@justinmk
Copy link
Contributor

Yes, Nvim HEAD.

@equalsraf equalsraf merged commit fcb99d6 into equalsraf:master Jun 22, 2018
@justinmk
Copy link
Contributor

@equalsraf If you have a chance, would be helpful to get a tagged release which includes this fix, so that Nvim can ship it to Windows users.

@equalsraf
Copy link
Owner

Tag in place but it seems appveyor builds are broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants