linter icon indicating copy to clipboard operation
linter copied to clipboard

allow linting of new yet unsaved files

Open dirk-thomas opened this issue 8 years ago • 10 comments

Created as a follow up of AtomLinter/linter-xmllint#68.

Currently when a user creates a new file (but doesn't save it yet) and selects a specific syntax for the editor no linter is being triggered. Some linters might not be able to work with an editor which doesn't have a path yet e.g. because they don't support on-the-fly linting or require relative resources. But some linters can perform their task in that case and they should be triggered.

This will require linters which can't handle this case to check the editors path and abort linting when the editor doesn't have a path yet.

dirk-thomas avatar Sep 07 '16 15:09 dirk-thomas

Just a thought, if this change would break many linters why not allow the linter to pass this information (whether it supports on-the-fly or not) to the base linter. This would be false by default so that base linter always do the 'file saved' check if parameter not provided.

gliviu avatar Sep 07 '16 15:09 gliviu

@gliviu That's already how the linter providers work 😉. There is a lintOnFly (soon to be lintsOnChange) flag they set and send to Linter when they are initialized. The only consideration here with regards to that is the edge case of potentially requiring external resources... and how the current file path is determined.

All of the linters that I know of currently either use path.dirname(textEditor.getPath()) (which will return undefined), or atom.project.relativizePath(textEditor.getPath())[0]... with a fallback to the previous. The problem is that this will return null or undefined, which I'm not sure any of them currently handle that situation well.

I'd be fine with this being a required situation to be able to handle in v2, as technically any v1 linter should already be able to handle this, it's just none were written well enough to do so that I know of 😛.

Arcanemagus avatar Sep 07 '16 16:09 Arcanemagus

As for actually implementing this I see a few options for linters that need a path to operate (for things like determining configurations):

  • Just return [] until the file is actually saved and has a path
  • Fall back to atom.project.getPaths()[0], this has the slight potential of being wrong though if the user has multiple projects open in the same Atom window. From what I've seen though this is a pretty rare situation.
  • Use a default configuration

For linters that don't need a path to operate (ie. they have no configuration) this isn't really an issue.

I'd personally probably go with the second choice, since in the rare case that there are more than one projects open in the same window, I'd imagine they are related and would probably have the same configuration anyway. If not, then the configuration will be wrong and there might be some incorrect messages reported, at least until the file is saved.

The technically correct result would be to just return [] if a path is necessary to operate though.

What are people's thoughts on that?

Arcanemagus avatar Sep 07 '16 16:09 Arcanemagus

Some of the current on-the-fly linters already don't care about the getPath() but only about the getText() of the passed editor. Just some examples I am aware of:

  • linter-pep8: https://github.com/AtomLinter/linter-pep8/blob/2a16da551ae9b826a4ca5c231d8153ef38308df3/lib/main.coffee#L38
  • linter-pyflakes: https://github.com/AtomLinter/linter-pyflakes/blob/7bfd6c54db3fdb91560ee37939f93f3825d925b8/lib/init.coffee#L24
  • linter-xmllint: https://github.com/AtomLinter/linter-xmllint/blob/99069c719b6c64fbd9c69e48ec27447ad044561d/lib/init.coffee#L87 (bad example since it currently tries to use the path as the cwd of the external linter which would need to be updated)

I can see atom.project.getPaths()[0] being wrong in various cases. E.g. the linter uses relative resources (e.g. a dtd to validate a xml file). Many linters consider configuration files relative to (or in parent folders of) the processed file. Often the editor path is also used in the error message. I guess each linter has to decide what makes sense.

A linter could also decide to return one message mentioning that the linter could not be run successful and that the file needs to be saved before.

dirk-thomas avatar Sep 07 '16 16:09 dirk-thomas

linter-pyflakes does use the file path, just not in the call to pyflakes. It's used directly as the filePath in the message (which will be required in v2 linters). Same thing with linter-pep8.

And that brings up an excellent problem with this... the v2 Message type currently requires a file in the location, this can never be available in this situation 😕.

Arcanemagus avatar Sep 07 '16 17:09 Arcanemagus

@Arcanemagus that is actually a very good point. If we allow this, we'll have to make providers associate textEditors with messages, because we not only have file scoped providers, we also have indie and project ones

steelbrain avatar Sep 07 '16 17:09 steelbrain

If someone needs a few reasons on why textEditors with messages are bad, try adding a text editor about a file that isn't open yet 😉

steelbrain avatar Sep 07 '16 17:09 steelbrain

Ouch, so we would have to have an active TextEditor associated with each message? I'm not sure that's something we would want to do...

Arcanemagus avatar Sep 07 '16 17:09 Arcanemagus

How about generating an "unsaved-file-path" for unsaved files, which get passed to the linter-provider? when a message file path matches the "unsaved-file-path" associate the message with said Buffer.

So all this stuff discussed here would actually be done mostly in the Linter side and not in the providers, old providers would ignore this information so it wouldn't be a breaking change, everything else stays the same

pablomayobre avatar May 11 '17 05:05 pablomayobre

I'm late to the party, but I'd like to toss out the one case where I'd find this hugely useful. I often copy/paste JSON network responses into a new Atom file to format them for readability. I'll occasionally miss a leading/trailing character, or the JSON will be malformed to start with for some reason. If the linter could be run on this throw-away context to help me make the JSON valid so the pretty formatter can operate on it.

Other similar quick linting on copy/pasted chunks of text without having to create temp files could be very useful for vanilla ES5, XML, etc.

jinglesthula avatar Apr 11 '18 19:04 jinglesthula