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

when there is an error, proselint --json should output the error to stderr rather than stdout #1307

Open
milanglacier opened this issue Jul 25, 2022 · 8 comments

Comments

@milanglacier
Copy link

milanglacier commented Jul 25, 2022

I am using proselint with null-ls which is a neovim plugin, essentially what this plugin does is cat xxx.md | proselint --json (assuming I am editing a markdown file),
sometimes what proseling output will be not a valid json,
i.e something like this:

output: Error ([Errno 35] Resource temporarily unavailable: '/Users/northyear/.cache/proselint/checks.airlinese.misc.check') opening /Users/northyear/.cache/proselint/checks.airlinese.misc.check - will attempt to delete and re-open.

which may from here,

print('Error (%s) opening %s - will attempt to delete and re-open.' %

i.e from this line. Since it is not a legal json, and then null-ls will parse it with error.
It would be better that when --json is supplied, errors should be outputed to stderr rather than stdout.

@jose-elias-alvarez
Copy link

jose-elias-alvarez commented Jul 26, 2022

To provide a little additional detail here: null-ls is a plugin that spawns external processes like proselint, captures their output, and converts that output into Neovim's diagnostic format for display in the editor. In this case, the issue stems from the error message, which is sent to stdout and therefore breaks the plugin's attempts to parse it as JSON.

Ideally, running proselint with the --json flag would always output valid JSON. I'm not a proselint user myself, so I haven't done research into other integrations, but I imagine they'd run into the same problem if they run under the conditions that create the error message. A simple solution (and one that I've seen other linters use) is to output error messages like these to stderr, which will fix things on our end while still providing feedback to users of the CLI. I'm not a Python expert, but I'm happy to put in a PR if there's a chance of getting this fixed.

Edit: also, if it's of any help, here is the relevant issue on our end, which contains more details (though we haven't been able to figure out what exactly triggers the error in the first place).

@Nytelife26
Copy link
Member

To provide a little additional detail here: null-ls is a plugin that spawns external processes like proselint, captures their output, and converts that output into Neovim's diagnostic format for display in the editor. In this case, the issue stems from the error message, which is sent to stdout and therefore breaks the plugin's attempts to parse it as JSON.

I use null-ls with Neovim on a daily basis, but thank you. It's an honour to speak with you.

This is strange behaviour on part of proselint indeed. I picked up the project a while ago, so I haven't yet squashed all the bugs. As you and @milanglacier have suggested, I'll look into why proselint sometimes fails to produce valid JSON, and also consider throwing to stderr.

@milanglacier
Copy link
Author

I find that if proselint is installed by the python installed by homebrew, then it will continuously give me such warning, i.e it can not open the file in ~/.cache/proselint folder.
However, if I install proselint via a venv in my home and not use the python installed by homebrew (in fact the python from miniforge) and then it works fine and no warning.

@milanglacier
Copy link
Author

It seems that [Errno 35] Resource temporarily unavailable is usually a macOS only problem in many scenarios😂

@Nytelife26
Copy link
Member

It seems that [Errno 35] Resource temporarily unavailable is usually a macOS only problem in many scenarios😂

Odd, I suspected it would be a filesystem issue persistent with most modern UNIX systems. Alas, I should expect nothing less from MacOS 😂 I'll investigate later tonight. Fortunately we have MacOS available in GitHub actions so I can test it that way.

@mcepl
Copy link

mcepl commented Aug 8, 2022

Actually, it doesn’t seem to be reproducible on Linux as well.

Using neovim nightly (commit 62916946), proselint 0.13.0, null-ls actual).

@mabuchner
Copy link

mabuchner commented Feb 27, 2023

I find that if proselint is installed by the python installed by homebrew, then it will continuously give me such warning, i.e it can not open the file in ~/.cache/proselint folder.

I think, it's a permission issue. On my MacOS system the permissions for ~/.cache/proselint were set to drwx--x--x.

After changing the permissions

chmod g+r,o+r proselint

the error went away.

Of course, it would still be nice, if proselint would output the error message to stderr.

Edit: Spoke too soon. The problem still occurs, but in my case less frequently.

@simon-liebehenschel
Copy link

The error happens when proselint is used via pre-commit. Tested on Debian image with a root only user.

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

No branches or pull requests

6 participants