lucc / nvimpager

Use nvim as a pager to view manpages, diffs, etc with nvim's syntax highlighting
Other
372 stars 20 forks source link

Indicating a file as a parameter should ignore stdin #28

Closed marioortizmanero closed 3 years ago

marioortizmanero commented 4 years ago

I was trying to use nvimpager as a previewer for nnn and ranger, but turns out that if there's no stdin (because nvimpager is ran within nnn/ranger), nothing is shown. I'm guessing the command is equivalent to this:

nvimpager -c example.ini < /dev/null

outputs:

image

I think it'd be more appropiate to first check the arguments, and falling back to stdin if none were passed.


I might be completely wrong, but it seems to be caused by highlight, which is called in cat_mode after nvim.nvim_command('next'). The first highlight outside the for shouldn't matter, but once it iterates the arguments something should be output. In this line, it will end when the buffer is empty, so it doesn't actually get to read the parameter file here.

It might actually be neovim's fault, here's the same problem with neovim:

image

It might be related to this issue.

lucc commented 4 years ago

I do not think that neovim is at fault because nvimpager always starts it with a tty on stdin: https://github.com/lucc/nvimpager/blob/73e0de4c148749dc7c5779939a879deb216faedf/nvimpager#L114

I think the issue is that nvimpager ignores any file arguments if stdin is not a tty (important here is the precedence in the if): https://github.com/lucc/nvimpager/blob/73e0de4c148749dc7c5779939a879deb216faedf/nvimpager#L90-L93

I have to figure out what would be the best way to handle that situation. Discussion is welcome! I will go and check what less or so do.

For ranger specifically we had an issue before that might help you to work around the problem there (#1). Maybe we should start a wiki page for usage of nvimpager with some special programs or document this somewhere else which is more visible than an issue.

lucc commented 4 years ago

less does ignore stdin if a file argumet is present, just like cat. But both accept - as a special file argument and stdin is put in the argument list at that point. Maybe we should aim for this.

marioortizmanero commented 4 years ago

Thanks for the link to that issue, it's exactly the same as what I'm experiencing. But in this case I can't use the workaround you provided because the file previewer I'm using (pistol) doesn't launch a sub-shell, only the command, so </dev/tty won't work.

Using - as a parameter is a great idea but I don't think it's exactly what this issue is about. Maybe it could be done that way if it's much easier, but I would try to imitate what less does, since this is often used as its replacement.

I'm a bit confused about the </dev/tty usage here when you launch nvim. What's that for? I'm trying to understand how that script works. I'll clone the repo and try for myself, but I'm not sure if I'll be of help, because I don't know much about lua or the nvim api.

lucc commented 4 years ago

The </dev/tty was introduced in 19f8133.

I think that bash does not change stdin for a subprocess automatically. So if the nvimpager script reads from stdin into a file (lines 90+) then nvim would get an empty pipe on stdin. Maybe that helps too understand the commit message.

I think we can duplicate the logic from cat/less. But I have no time to do it today, hopefully during the weekend.