Use atom-linter helpers for dealing with temp files

[jschroeder9000]

The atom-linter package intends to implement functionality for dealing with temp files. When that happens, linter-haml should use that instead of custom code.

Depends on: steelbrain/atom-linter#30.

See also: #11.

[steelbrain]

The temp method is there in the latest release.

[jschroeder9000]

After reviewing the tempFile method provided by atom-linter it doesn't seem like a good fit for this linter as is. I need to be able to write multiple files to the same temporary directory.

@steelbrain thoughts?

[steelbrain]

@jschroeder9000 why would you want to write multiple files to a single dir?

[jschroeder9000]

It's a workaround for a limitation of haml-lint. See #2.

[steelbrain]

That is horrible, You should open an upstream bug demanding for stdin support.
Until then just disable lintOnFly on this provider and use the text editor paths for linting, people will understand.

[jschroeder9000]

I'll try to avoid making any demands. Our work is possible because of theirs. :)

I did open an upstream bug (sds/haml-lint#66) which brought about the idea of setting an environment variable with the necessary config location, but it lost steam. We could try to revisit that.

OTOH, I agree it would be ideal if haml-lint could lint from STDIN. I was going to say that's a bit of a difficult proposition given that rubocop does not support it, but after double-checking that that is correct I see a pull request (rubocop/rubocop#2146) was merged a week ago that adds support for it.

Initially that seemed like a boon for adding STDIN support to haml-lint, but after investigating further I don't believe that is the case. Since both are written in ruby, haml-lint calls rubocop code directly; it does not create a new process where rubocop could read from STDIN. Rubocop could theoretically add support for being passed a string, but all that would do is make haml-lint more performant by eliminating disk writes of its own. Neither rubocop accepting input from STDIN nor from a string has any real bearing on haml-lint's ability to accept input from STDIN.

I make disk writes to satisfy haml-lint. Haml-lint makes disk writes to satisfy rubocop. And in spite of that this package performs well for me (granted I've only used it on machines with SSDs).

Each project is making use of edge cases in its dependency that are difficult to justify supporting. In order to eliminate disk writes here, rubocop would have to support being passed a string. But I bet that would get laughed down. That's what I would probably do. Who passes file contents as a string to a CLI program? Nobody. Except haml-lint. But that's what would it would take to eliminate disk writes because haml-lint uses rubocop's CLI class. And that makes sense because it's a stable public API.

In the mean time I intend to support lintOnFly by any reasonable means. While the current workaround is certainly not ideal, I think it is reasonable given the benefit it provides. If I were not the maintainer of this package and it did not support lintOnFly, I would make a fork that did (by any hacky means) because I would find little use for it without. There is already an option to disable the workaround if that is desired.

After all this rambling I intended to close this since it seems that helpers are not as good a fit as originally thought. But now I see that haml-lint actually has implemented support for the environment variable proposed a few months ago (sds/haml-lint@f222bfa). Ugh.

For now I think the best way to move forward is to make use of haml-lint's support for setting an environment variable for the rubocop config location. It requires no additional changes to dependencies and would eliminate the need to copy the rubocop.yml file to the temp dir, which in turn eliminates the need to support multiple files in the same temp dir, which means it is, in fact, possible to go ahead with this proposed change.

It may also be possible to further cut back on disk accesses by remembering the location of the rubocop config file. That could break things if people add/move/remove it, though. Something to think about.

I've also toyed with the idea of a setting that would enable/disable lintOnFly for only this package (while respecting the setting of the main linter package) for people who may not appreciate the IO overhead. It hasn't gone anywhere yet, though.

/ramble

[steelbrain]

John, I respect you and I respect your opinions. but IMO, it's a false argument that we should not demand new features just because we're using someone's project. I know a lot of linters that support stdin and some linters like jshint who rely on a configuration file also support a --filename argument which supports passing the path of the stdin contents.

I am not in the favor of disabling lintOnFly, I like it, I use it everyday, but there are some languages/file types who cost a little more than usual to do on the fly linting, like HackLang or Flow. I don't mind if it doesn't I understand that it's worth it.

The best way of solving this would be to demand support for stdin in haml linter, if they don't agree with you, argument with them and show them the benefits. Not everyone has SSD and even when they have SSD we shouldn't do something that's unnecessary.

[Arcanemagus]

I'm removing the current temporary file setup in #113. It isn't complete enough to cover all the edge cases, and the entire idea just doesn't really work with linters that require several external files. The atom-linter implementation is even less of a fit for this linter for the reasons outlined above. As @steelbrain said above, if on the fly linting is truly desired here, it needs to be solved in haml-lint by supporting stdin input.

I'll leave that open for a few days for comments or ideas to a potential solution.