Lint script from stdin #240
Conversation
|
@RianFuro |
|
@Kuniwak basically a good game plan, I'm not sure though if how this impacts performance if people start loading megabyte-sized vim files into the linter (since you're basically loading everything into memory). Then again, from what I know about python, stdin is just a specialized file object, so if we pass the opened file in instead of just the path we could substitute that with the stdin object without any other part of the system noticing. This would at least give the possibility to keep the linter memory efficient, without yielding too much overhead compared to just using raw text. It's your software though, so if you can't be bothered don't let my inner system architect's nagging stop you; storing the file content in context is most likely a perfectly valid solution 👍 |
|
Sorry for my too late reply. I think we can cache file contents if we feel it is very slow. |
|
|
||
| return violations | ||
|
|
||
| def _should_read_from_stdin(self, env): | ||
| return len(env['file_paths']) == 1 and PosixPath('-') in env['file_paths'] |
There was a problem hiding this comment.
Couldn't this be env['file_paths'] == [PosixPath('-')]?
But then again you might even want to mix it - vint foo.vim - bar.vim could read 3 "files".
There was a problem hiding this comment.
Agree. I feel supporting vint foo.vim - bar.vim is a good way to follow common usage of - such as cat a - b.
| return [parse_error] | ||
| except EncodingDetectionError as exception: | ||
| decoding_error = self._create_decoding_error('stdin', str(exception)) | ||
| return [decoding_error] |
There was a problem hiding this comment.
Would be nice to support --stdin-filename that could be used to configure the name (instead of stdin).
|
@Kuniwak |
|
Yes we should it if we can. I stop to release 0.3.19 until this feature is merged. |
|
Makes sense, thanks. |
|
I'm working at #278. I'm gonna release v0.3.19 if it is merged. |
This would resolve #183, however I'm really not satisfied with how it's looking.
The way I had to sprinkle branching logic basically everywhere - it hurts especially in the policy, which should be user-contributed - suggests that we really should refactor how the linted file is passed around.
Right now the file is passed as just the path in the linting context inside the linter, which makes it really clunky when stdin is introduced. I would say it would make more sense for the linter and the linter context to operate on a file object or even on raw text instead (don't know if the latter one is too memory inefficient with large files though).
Thoughs?