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

fix(#68): handle formatter errors #75

Merged
merged 1 commit into from
Aug 20, 2023
Merged

fix(#68): handle formatter errors #75

merged 1 commit into from
Aug 20, 2023

Conversation

barrett-ruth
Copy link
Contributor

#68 warrants a bit more discussion about the exit code. While formatters exit with 0 (usually) on success, some linters like shellcheck exit with non-zero exit codes if they find errors. This means the following code, if put in spawn.lua, will not work if placed in function spawn() for some linters, but will resolve issue #68 by catching the exit code:

    if exit_code == 0 then
      coroutine.resume(co, table.concat(chunks))
    else
      on_failed(('process %s exited with non-zero exit code %s'):format(opt.cmd, exit_code))
      coroutine.resume(co)
      return
    end

As a result, I think a check_exit_code is necessary. I don't like to introduce complexity but it's either add the function or somehow distinguish between linters and formatters before spawn(), which sounds like it could get messy. thoughts @glepnir @xiaoshihou514 ?

BTW sorry for messing up previous PR - rebase skills are improving.

@glepnir
Copy link
Member

glepnir commented Aug 19, 2023

usually exit code non 0 or stdout.len() < 999 is error. so if 0 is correct non 0 just on_failed. check_exit_code why need this function ?

@barrett-ruth
Copy link
Contributor Author

Non-zero exit code does not indicate a failure always.

That logic falls apart for the linter shellcheck:
output is sent to stdout
non-zero exit code is returned when the linter runs correctly

How do you overcome this without check_exit_code function?

@xiaoshihou514
Copy link
Collaborator

xiaoshihou514 commented Aug 20, 2023

@glepnir proposed the following

if (ret~=0 and #stderr>0) then
    -- determine that there was an error
else
    ...

which should cover most cases. Unless the tool is really messing with the convention, in which case the user should make use of the fn feature.

@barrett-ruth barrett-ruth marked this pull request as ready for review August 20, 2023 03:26
@barrett-ruth
Copy link
Contributor Author

Ok - is this what you mean? @xiaoshihou514 .

@xiaoshihou514 xiaoshihou514 merged commit de02de7 into nvimdev:main Aug 20, 2023
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.

3 participants