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

Added encoding configuration #56

Closed
wants to merge 1 commit into from

Conversation

josenicomaia
Copy link

Added encoding configuration #34

@josenicomaia
Copy link
Author

I wasn't able to test the changes. I don't know how to setup the development environment.
The README.md wan't enough.

@josenicomaia
Copy link
Author

I applied the "diff" at ~/.vscode/extensions/ikappas.phpcs-0.7.0 and it worked fine.

@weirdan
Copy link

weirdan commented Aug 1, 2017

@josenicomaia , it may have worked for 0.7, but you're submitting this against develop, so you should really test against that.

develop (future 0.8) is very different to 0.7, so this actually may do more harm there than good.

TextDocument.getText() returns string that's later passed to the stdin of the process started with utf8 IO streams encoding (see options hash in lint method). So I suspect it will be utf8 on PHPCS stdin, no matter the actual file encoding. See AtomLinter/linter-phpcs#235 for very similar problem, and why it makes sense to keep --encoding=UTF-8 as the only encoding.

@ikappas
Copy link
Owner

ikappas commented Oct 27, 2017

@josenicomaia Thank you for submitting a merger request!

As @weirdan accurately explained the way the latest version works on develop is very different compared to 0.7 so I am closing this.

@ikappas ikappas closed this Oct 27, 2017
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