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

Executing a the local phpcbf command does not fix files #38

Closed
marioy47 opened this issue Jun 2, 2020 · 17 comments
Closed

Executing a the local phpcbf command does not fix files #38

marioy47 opened this issue Jun 2, 2020 · 17 comments
Labels
question Further information is requested

Comments

@marioy47
Copy link

marioy47 commented Jun 2, 2020

Hello,

After much struggle I was able to make phpcs lint my WordPress files.

The problem now is, is that phpcbf (The formatter) is not taking the local project configuration. The files get formatted but not to the correct standard.

This is my complete coc-settings.json:

{
  "coc.preferences.formatOnSaveFiletypes": ["php", "json"],
  "diagnostic-languageserver.filetypes": {
    "php": ["phpcs"]
  },
  "diagnostic-languageserver.linters": {
    "phpcs": {
      "command": "./vendor/bin/phpcs",
      "debounce": 100,
      "rootPatterns": ["composer.json", "composer.lock", "vendor", ".git"],
      "args": ["--report=emacs", "-s", "-"],
      "offsetLine": 0,
      "offsetColumn": 0,
      "sourceName": "phpcs",
      "formatLines": 1,
      "formatPattern": [
        "^.*:(\\d+):(\\d+):\\s+(.*)\\s+-\\s+(.*)(\\r|\\n)*$",
        {
          "line": 1,
          "column": 2,
          "message": 4,
          "security": 3
        }
      ],
      "securities": {
        "error": "error",
        "warning": "warning"
      }
    }
  },
  "diagnostic-languageserver.formatFiletypes": {
    "php": "phpcbf"
  },
  "diagnostic-languageserver.formatters": {
    "phpcbf": {
      "command": "./vendor/bin/phpcbf",
      "rootPatterns": ["composer.json", "composer.lock", "vendor", ".git"],
      "args": ["-"]
    }
  }
}
@iamcco
Copy link
Owner

iamcco commented Jun 2, 2020

I do not use phpcbf so I don't know how to do that. Maybe you can add args or something to tell phpcbf to use your local configuration.

@iamcco iamcco added the question Further information is requested label Jun 2, 2020
@marioy47
Copy link
Author

marioy47 commented Jun 2, 2020

I think I made some progress, but still no solution.

I added "intelephense.format.enable": false, to coc-settings.json and changed the phpcbf formater config to

  "diagnostic-languageserver.formatters": {
    "phpcbf": {
      "command": "./vendor/bin/phpcbf",
      "rootPatterns": ["composer.json", "composer.lock", "vendor", ".git"],
      "args": ["%file"],
      "isStdout": false,
      "doesWriteToFile": true
    }
  }

And now the files get formated but no loaded in the buffer.

The breaktrough is that I get the following error:
vim-error

So it looks like the phpcbf fixer is takes longer that normal.

is there a way to increment the timeout so I can verify this?

@mkindred
Copy link

I'm having the same issue. phpcs works great, and phpcbf is making the corrections, but the saved file isn't loaded to the buffer.

210524_coc_phpcbf_issues

@iamcco
Copy link
Owner

iamcco commented May 25, 2021

I'm having the same issue. phpcs works great, and phpcbf is making the corrections, but the saved file isn't loaded to the buffer.

210524_coc_phpcbf_issues

seems like phpcbf just modify the file instead of output the format file to stdout.

@mkindred
Copy link

mkindred commented May 25, 2021

I've tried several combinations of isStdout and doesWriteToFile, and I can't quite get there. Here is my coc-settings.json file:

{
  "coc.preferences.formatOnSaveFiletypes": [
    "javascript",
    "javascriptreact",
    "javascriptreact.html",
    "html",
    "json",
    "php",
    "yaml"
  ],
  "intelephense.format.enable": false,
  "diagnostic-languageserver.filetypes": {
    "php": "phpcs"
  },
  "diagnostic-languageserver.linters": {
    "phpcs": {
      "command": "phpcs",
      "debounce": 300,
      "rootPatterns": ["composer.lock", "vendor", ".git"],
      "args": ["--report=emacs", "-s", "-"],
      "offsetLine": 0,
      "offsetColumn": 0,
      "sourceName": "phpcs",
      "formatLines": 1,
      "formatPattern": [
        "^.*:(\\d+):(\\d+):\\s+(.*)\\s+-\\s+(.*)(\\r|\\n)*$",
        {
          "line": 1,
          "column": 2,
          "message": 4,
          "security": 3
        }
      ],
      "securities": {
        "error": "error",
        "warning": "warning"
      }
    }
  },
  "diagnostic-languageserver.formatFiletypes": {
    "php": "phpcbf"
  },
  "diagnostic-languageserver.formatters": {
    "phpcbf": {
      "command": "phpcbf",
      "rootPatterns": ["composer.lock", "vendor", ".git"],
      "args": ["--standard=./phpcs.xml", "%file"],
      "isStdout": false,
      "isStderr": false,
      "doesWriteToFile": true
    }
  },
  "git.enableGutters": false,
  "prettier.singleQuote": true
}

With the above, I get the option to re-load the corrected file.

But if I use the following, nothing happens.

  "diagnostic-languageserver.formatFiletypes": {
    "php": "phpcbf"
  },
  "diagnostic-languageserver.formatters": {
    "phpcbf": {
      "command": "phpcbf",
      "rootPatterns": ["composer.lock", "vendor", ".git"],
      "args": ["--standard=./phpcs.xml"],
      "isStdout": true,
      "isStderr": false,
      "doesWriteToFile": false
    }
  },

I confirmed that running phpcbf test.php from the command line does work, and it correctly picks up the appropriate phpcs.xml.

@iamcco
Copy link
Owner

iamcco commented May 25, 2021

what's doesWriteToFile options? coc-diagnostic doesn't support such option.

you have to make sure phpcbf command does not write change to disk and output format content to stdout or stderr. then set the isStdout or isStderr to true.

@mkindred
Copy link

I found doesWriteToFile under 'formatters field' at https://github.com/iamcco/diagnostic-languageserver

Yes, I'm trying to learn how to direct phpcbf output to stdout.

@iamcco
Copy link
Owner

iamcco commented May 25, 2021

I found doesWriteToFile under 'formatters field' at https://github.com/iamcco/diagnostic-languageserver

ohhhhhhh my fault.

@yaegassy
Copy link
Contributor

yaegassy commented May 25, 2021

@iamcco Is it possible to pass stdin in diagnostic-langserver's "formatter"?

If you can pass it as stdin, this problem may be solved.

$ cat sample.php
<?php

declare(strict_types=1);

function dummy() {
    return "dummy";
}
$ cat sample.php | phpcbf
<?php

declare(strict_types=1);

function dummy()
{
    return "dummy";
}

@iamcco
Copy link
Owner

iamcco commented May 25, 2021

@iamcco Is it possible to pass stdin in diagnostic-langserver's "formatter"?

If you can pass it as stdin, this problem may be solved.

$ cat sample.php
<?php

declare(strict_types=1);

function dummy() {
    return "dummy";
}
$ cat sample.php | phpcbf
<?php

declare(strict_types=1);

function dummy()
{
    return "dummy";
}

Yep, by default it use the stdin.

so it maybe works change the %file to %filepath:

"diagnostic-languageserver.formatters": {
    "phpcbf": {
      "command": "phpcbf",
      "rootPatterns": ["composer.lock", "vendor", ".git"],
      "args": ["--standard=./phpcs.xml", "%filepath"],
      "isStdout": true,
      "isStderr": false,
      "doesWriteToFile": false
    }
  },

PS. it will not use stdin when use %file, it's note in the README of https://github.com/iamcco/diagnostic-languageserver#args-additional-syntax

@yaegassy
Copy link
Contributor

I used %filepath and ran it in the same file as this comment. #38 (comment)

Unfortunately, there seems to be no change in "newText" :(

linter run args: ["/Users/yaegassy/_Dev/php/slim4php8/sample.php"]
[Trace - 17:09:43] Received response 'textDocument/formatting - (4)' in 81ms.
Result: [
    {
        "range": {
            "start": {
                "line": 0,
                "character": 0
            },
            "end": {
                "line": 9,
                "character": 0
            }
        },
        "newText": "<?php\n\ndeclare(strict_types=1);\n\nfunction dummy() {\n    return \"dummy\";\n}\n"
    }
]

@iamcco
Copy link
Owner

iamcco commented May 25, 2021

https://github.com/iamcco/diagnostic-languageserver/blob/master/src/handles/handleFormat.ts#L50

seems like I know what happen. need to fix it.

@yaegassy
Copy link
Contributor

@iamcco I tried to skip that control part in diagnostic-languageserver and it was formatted correctly in phpcbf

setting

// ...snip
    "phpcbf": {
      "command": "phpcbf",
      "rootPatterns": ["composer.json", "composer.lock", "vendor", ".git"],
      "isStdout": true,
      "isStderr": false
    }
// ..snip

log(OK)

linter run args: []
[Trace - 18:13:24] Received response 'textDocument/formatting - (3)' in 84ms.
Result: [
    {
        "range": {
            "start": {
                "line": 0,
                "character": 0
            },
            "end": {
                "line": 9,
                "character": 0
            }
        },
        "newText": "<?php\n\ndeclare(strict_types=1);\n\nfunction dummy()\n{\n    return \"dummy\";\n}\n"
    }
]

@yaegassy
Copy link
Contributor

yaegassy commented May 25, 2021

I changed the diagnostic-languageserver to verify this.

phpcbf seems to return 1 for "code". :(

$ cat sample.php | phpcbf
<?php

declare(strict_types=1);

function dummy()
{
    return "dummy";
}
$ echo $?
1

@iamcco
Copy link
Owner

iamcco commented May 25, 2021

I add ignoreExitCode options support for formatter. it can be true ignore all exit code > 0 or number[] for exit codes you want to ignore. and below config works for me.

  "diagnostic-languageserver.formatters": {
    "phpcbf": {
      "command": "phpcbf",
      "rootPatterns": ["composer.lock", "vendor", ".git"],
      "args": [],
      "isStdout": true,
      "isStderr": false,
      "doesWriteToFile": false,
      "ignoreExitCode": [1]
    }
  },

@yaegassy
Copy link
Contributor

I have confirmed that phpcbf works correctly in my environment with coc-diagnostic "v0.20.0". (add "ignoreExitCode": [1])

@iamcco iamcco closed this as completed May 25, 2021
@mkindred
Copy link

Confirmed working here, as well. Thanks!

marioy47 added a commit to marioy47/dotfiles that referenced this issue May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants