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

Implement lint-as-you-type #35

Closed
weirdan opened this issue Feb 11, 2017 · 36 comments
Closed

Implement lint-as-you-type #35

weirdan opened this issue Feb 11, 2017 · 36 comments
Assignees
Milestone

Comments

@weirdan
Copy link

weirdan commented Feb 11, 2017

vscode-phpcs lints files on save (and load). It's possible to lint unsaved files by feeding file contents to PHPCS on its STDIN. This might provide for more interactive user experience: seeing if you introduced a problem or fixed one without saving the file.

You may borrow some implementation bits from AtomLinter/linter-phpcs that implemented this feature.

@ikappas
Copy link
Owner

ikappas commented Feb 11, 2017

@weirdan Have a look at the latest development branch and see if this works ;)

@ikappas ikappas added this to the 0.8.0 milestone Feb 11, 2017
@ikappas ikappas self-assigned this Feb 11, 2017
@weirdan
Copy link
Author

weirdan commented Feb 11, 2017

It appears to be working. I'm a bit worried though because I don't see any code to throttle linting. Wouldn't it cause phpcs to be spawned for every keystroke?
I also observed some linting errors appearing for lines that are no longer there. I guess this could be caused linting starting when lines are still there, but completing after the lines had been already removed.

As a sidenote, you can use fixed encoding if you're feeding text to PHPCS's stdin, just like Atom's counterpart does: AtomLinter/linter-phpcs#236 (see issue linked from there for rationale). That'll fix #34 with a single line of code.

@ikappas
Copy link
Owner

ikappas commented Feb 12, 2017

@weirdan After doing some tests, I found that change event is throttled at 200ms. See https://github.com/Microsoft/vscode-languageserver-node/blob/master/client/src/main.ts#L534
I have ported the fixed encoding code on the latest development branch. Let me know how this works!

@weirdan
Copy link
Author

weirdan commented Feb 12, 2017

This actually works pretty good for me.

@oojacoboo
Copy link

When will we see 0.8.0 get pushed out? I'd like to see this feature as well. I've currently tweaked my autosave to give me this effect, but it's not preferred.

Also, I might point out, that there are some rules that fix whitespace that cause the cursor to go from an indented state back to the start of the line. If this was doing it every 200ms, this feature would be useless since you'd have to start typing then tab in every single line.

@yukulele
Copy link

how to install this version?

@ikappas
Copy link
Owner

ikappas commented Oct 27, 2017

@weirdan @oojacoboo @yukulele I've pushed some changes in the development branch. Can you test these changes ?

@weirdan
Copy link
Author

weirdan commented Oct 28, 2017

I got this running, it appears to be working. Is there anything specific you would like to get tested?

@oojacoboo
Copy link

oojacoboo commented Oct 31, 2017

@ikappas I'm getting phpcs: Cannot read property 'length' of null. Not sure where to find more information on this error?

Reverted back to 0.7.0 and the issue disappeared and phpcs is analyzing the code properly again.

@weirdan
Copy link
Author

weirdan commented Oct 31, 2017

@oojacoboo , oh, I had that problem too. Had to cleanup all unversioned/ignored files, then rebuilt the ext & server and then it worked.

@oojacoboo
Copy link

Looks like the .gitignore is incorrect.

*/node_modules should be node_modules.

@oojacoboo
Copy link

oojacoboo commented Oct 31, 2017

Same error here though, after that. phpcs: Cannot read property 'length' of null. Maybe I'm doing something wrong in the steps, not certain. It's hard to know what the state of things are with the extension and I'm not very familiar with the debug tool in vscode or running NPM within vscode.

@weirdan
Copy link
Author

weirdan commented Oct 31, 2017 via email

@ikappas
Copy link
Owner

ikappas commented Oct 31, 2017

@oojacoboo Are you following updated instructions found https://github.com/ikappas/vscode-phpcs/blob/develop/README.md ?

@oojacoboo
Copy link

@ikappas yep.

@ikappas
Copy link
Owner

ikappas commented Oct 31, 2017

@oojacoboo win or mac?

@oojacoboo
Copy link

@ikappas mac. It'd be nice if the error output actually told you something helpful. That's a needle/haystack error message output.

@ikappas
Copy link
Owner

ikappas commented Oct 31, 2017

@oojacoboo The error suggests that an array is returned somewhere as null

@oojacoboo
Copy link

@ikappas okay sure. But that doesn't really help much. I don't know if this is within analyzed PHP code, the plugin source, the configuration, etc. It's virtually impossible to track that down, especially since I don't know much about how this plugin works.

It is working properly on 0.7.0, however.

@ikappas
Copy link
Owner

ikappas commented Oct 31, 2017

@oojacoboo can you post your phpcs related settings.json config?

@weirdan
Copy link
Author

weirdan commented Oct 31, 2017

I suspect it's caused by old 'phpcs/server' folder not being cleaned up on build.

@oojacoboo
Copy link

oojacoboo commented Oct 31, 2017

// Optional. The name or path of the coding standard to use. Defaults to the one set in phpcs global config.
"phpcs.standard": "app/vendor/foo/coding-standard/phpcs.xml",

@ikappas
Copy link
Owner

ikappas commented Oct 31, 2017

@weirdan If that is the case then manually delete the server folder and run vscode compile task

@ikappas
Copy link
Owner

ikappas commented Oct 31, 2017

@oojacoboo did you try last suggestion?

@oojacoboo
Copy link

@ikappas I don't know what the "server folder" is. Where is that?

@ikappas
Copy link
Owner

ikappas commented Nov 1, 2017

@oojacoboo phpcs/server

@ikappas
Copy link
Owner

ikappas commented Nov 1, 2017

I just pushed some major new features in develop branch that need testing and include:

  • ability to set phpcs.executablePath
  • ability to set phpcs.showWarnings
  • ability to set phpcs.showSources
  • ability to enable phpcs.trace.server with messages or verbose option for tracing errors.
  • ability to set phpcs.composerJsonPath to something other that workspaceRoot/composer.json

Your feedback is very welcome!

@oojacoboo
Copy link

oojacoboo commented Nov 2, 2017

@ikappas phpcs: ENOENT: no such file or directory, lstat '/Volumes/repos/foo/web/composer.json'

composer.json is located in a sub directory of the project, one level deeper. Not sure why this should be triggering a phpcs issue at any rate.

Also, I think the issue earlier was that I installed 0.8.1 from the Extension Development Host window that was launched, then went back to the existing window with my source code and tried to use that window. This time I loaded the project/source into the newly launched window.

@ikappas
Copy link
Owner

ikappas commented Nov 2, 2017

@oojacoboo Good to hear you got your first error sorted out.

I assume that your workspace root is /Volumes/repos/foo/web/ so by default the plugin looks for a composer.json file there to automatically resolve the phpcs executable path. It should ignore the missing composer.json and move to searching your global path so I'll have a look.

Meanwhile, since your composer is located one level deeper i.e in a folder named bar try to to set the following setting:

"phpcs.composerJsonPath": "bar/composer.json"

@ikappas
Copy link
Owner

ikappas commented Nov 2, 2017

@oojacoboo I fixed the issue when composer.json was not found and pushed the code to develop

@oojacoboo
Copy link

@ikappas tested the latest commits. No issues here.

phpcs.showSources is awesome - much needed to perfect our configs without having to go to the terminal and run phpcs separately.

And, of course, linting as you type is huge!

Let's get this thing deployed!

@jclerc
Copy link

jclerc commented Nov 13, 2017

The new features sound awesome! When will this be deployed?

@ikappas
Copy link
Owner

ikappas commented Nov 13, 2017

@jclerc Working on multi-root support before releasing 1.0.0

@ikappas
Copy link
Owner

ikappas commented Feb 17, 2018

I just pushed multi-root support in develop branch that need testing

@ikappas
Copy link
Owner

ikappas commented Feb 20, 2018

I've just pushed 1.0.0-beta.6 on the development branch adding automatic configuration search. This is the last feature for the 1.0.0 release which needs some testing before 1.0.0 launch.

@ikappas
Copy link
Owner

ikappas commented Feb 21, 2018

I just released vscode-phpcs 1.0.0 which supports the VSCode 1.20+ and includes many enhancements most notable of which are:

  1. multi root support
  2. lint as you type support
  3. automatic configuration search support
  4. ability to set phpcs.executablePath
  5. ability to set phpcs.showWarnings
  6. ability to set phpcs.showSources
  7. ability to set phpcs.composerJsonPath to something other that workspaceRoot/composer.json
  8. ability to enable phpcs.trace.server with messages or verbose option for tracing errors.

I would like to give my thanks to all the people that helped track down issues in the previous betas and hope the new features help you get things done easier.

On that note, I am closing this issue to properly track any new issues in the new release. If you find this issue persists or any other issue please feel free to open a new ticket for v.1.0.0.

@ikappas ikappas closed this as completed Feb 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants