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

Errors with prettierd aren't handled #68

Open
DrunkenToast opened this issue Aug 17, 2023 · 8 comments · May be fixed by #150
Open

Errors with prettierd aren't handled #68

DrunkenToast opened this issue Aug 17, 2023 · 8 comments · May be fixed by #150

Comments

@DrunkenToast
Copy link

When the formatter fails the error gets written in the buffer.

SyntaxError: Unknown keyword or identifier. Did you mean 'import'? (1:1)
�[0m�[31m�[1m>�[22m�[39m�[90m 1 |�[39m mport { defineStore } �[36mfrom�[39m �[32m'pinia'�[39m�[33m;�[39m�[0m
�[0m �[90m   |�[39m �[31m�[1m^�[22m�[39m�[0m
�[0m �[90m 2 |�[39m �[36mimport�[39m type {�[0m
�[0m �[90m 3 |�[39m     �[33mCreateProgramContract�[39m�[33m,�[39m�[0m
�[0m �[90m 4 |�[39m     �[33mPpmProgram�[39m�[33m,�[39m�[0m

used config:

    {
        "nvimdev/guard.nvim",
        event = { 'BufReadPost', 'BufNewFile' },
        config = function()
            local ft = require('guard.filetype')

            ft('typescript,javascript,typescriptreact,vue'):fmt('prettierd')
            --     :lint({
            --     cmd = 'eslint',
            --     args = { '--fix' },
            --     stdin = true
            -- })

            -- Call setup() LAST!
            require('guard').setup({
                -- the only options for the setup function
                fmt_on_save = true,
                -- Use lsp if no formatter was defined for this filetype
                lsp_as_default_formatter = false,
            })
        end
    },
@barrett-ruth
Copy link
Member

Should involve handling exit code of uv.spawn() process for formatters (we currently ignore)

I suggest we also create a config option check_exit_code like null-ls. Otherwise check if exit code is 0.

@DrunkenToast
Copy link
Author

DrunkenToast commented Aug 21, 2023

Fix was pushed. Thanks y'all.

@DrunkenToast
Copy link
Author

This issue resurfaced for me. Same situation, same config. Only updated the plugin.

@DrunkenToast DrunkenToast reopened this Sep 11, 2023
@xiaoshihou514
Copy link
Collaborator

Yeah... previously it was working due to a bug. Some extra logic for catching errors has to be done just for prettierd. For now I would just recommemd using prettier instead.

@DrunkenToast
Copy link
Author

Alright, noticeably slower but does indeed seem to work again. Thanks!
I'll rename this issue to specify prettierd.

@DrunkenToast DrunkenToast changed the title Errors with formatters aren't handled Errors with prettierd aren't handled Sep 12, 2023
@barrett-ruth
Copy link
Member

barrett-ruth commented Sep 13, 2023

@xiaoshihou514 why is this not resolved yet? So this is broken with prettierd because it writes to stdout not stderr. We either need to provide a table of valid_exit_codes or use an optional exit_code function as a default before using your logic.

Forrmatters and linters treat exit codes different and we can't do the logic for both of them together.... We need one of the aforementioned approaches so we can resolve both this issue AND the one you mentioned in #81 with selene.

@xiaoshihou514
Copy link
Collaborator

@barrett-ruth Because now prettierd is the only exception I am still hesitant to add yet another attribute to the tool table. Personally I would prefer providing an api for spawn, update_buffer etc. to empower users to write their own formatting logic. Which would also solve #86, #78 (and potentially more issues about formatting logic). They could also be provided via fn in guard-collection. What is your take on this?🤔

@barrett-ruth
Copy link
Member

Makes sense I guess, maybe I'm just more liberal about the features we should include.

By that reasoning it makes sense for guard-collection to have that feature. If we provide the formatter it should just work and that should be our top priority (we need to merge that ASAP).

In the meantime, I tried to resolve the prettierd issue at its source.

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

Successfully merging a pull request may close this issue.

3 participants