Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Update to the new Linter API #33

Merged
merged 8 commits into from
Aug 5, 2015
Merged

Update to the new Linter API #33

merged 8 commits into from
Aug 5, 2015

Conversation

Arcanemagus
Copy link
Member

Updates to the new Linter API. Also moves to the XML output, enabling some more advanced information.

DO NOT MERGE
Some issues with the Message format lead me to think this can be improved a bit before release, but I wanted to bring this out for review.

Review on Reviewable

@Arcanemagus
Copy link
Member Author

Specifically, line/column numbers are not showing up in messages.

@steelbrain steelbrain self-assigned this Jul 22, 2015
@steelbrain
Copy link
Contributor

Line/Column are only hidden when you don't provide the filePath parameter.

executablePath:
title: 'CSSLint Executable Path'
type: 'string'
# default: path.join(__dirname, '..', 'node_modules', '.bin', 'csslint.cmd')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's available as an npm module, I highly recommend we use that!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would I include it? As far as I could tell it doesn't provide the "cli wrapper mode" that things like jshint do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use the execNode method of the helpers module and pass in the direct .js path to it (Note: I am not sure if cmd one will work)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On that note, currently on both my systems I had to manually specify the path, since even if you point it at the bundled directory it's not smart enough to use PATHEXT.

I'll look into whether cli.js can be used.

@Arcanemagus
Copy link
Member Author

Ah, knew I was forgetting something, I'll fix that in a sec.

range: [[line, char], [line, char]]
}]
})
return toReturn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EOF please

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done btw.

@timdorr timdorr mentioned this pull request Jul 22, 2015
@steelbrain
Copy link
Contributor

Looks Good, except that I have a question, if csslint is available as an npm module, why not use that directly?

@Arcanemagus
Copy link
Member Author

Haven't had a chance to get that working yet, should have time tonight.

@Arcanemagus
Copy link
Member Author

So update time:
I have a version working with the embedded csslint javascript, which works great, unfortunately it doesn't support .csslintrc files.

So we can use the command line version above (after fixing path detection, since it currently has issues), but will not have lintOnSave functionality... or we can build something to read in the .csslintrc files ourselves as a few other implementations do.

I'm leaning towards the latter as it offers more flexibility in the long run, so I'll commit what is done so far and can implement a .csslintrc loader tomorrow.

Pulls in the `csslint` code and utilizes it directly. Unfortunately this
means we need to pull in and parse the `.csslintrc` files ourselves.
Linting on the fly can be enabled however.
Adds support for a .csslintrc file in the same directory as the file
being linted.
@Arcanemagus
Copy link
Member Author

Sorry this took so long, here is a version that supports pulling in options from a .csslintrc file in the current directory.

It seems this functionality is poorly documented at best in the CSSLint project, as some other linters based on it search in the user's home directory as well. Also the CSSLint project only documents supporting options in the same format as the command line options, yet everything (including CSSLint!) supports a JSON format.

Currently this only supports the JSON format for .csslintrc, as the other linters that even support it only support this format I think this will probably be acceptable.

Move to the AtomLinter/csslint for until `stdin` and `JSON` support are
implemented in the main repository.
Use helpers.execNode to run the linter.
@Arcanemagus
Copy link
Member Author

Okay, utilizing the csslint fork created by @steelbrain (https://github.com/AtomLinter/csslint) that supports stdin input, as well as an additional JSON formatter, this should be ready.

Any further issues?

@steelbrain
Copy link
Contributor

I'll test it as soon as I have some time

steelbrain added a commit that referenced this pull request Aug 5, 2015
@steelbrain steelbrain merged commit 690037f into AtomLinter:master Aug 5, 2015
@Arcanemagus Arcanemagus deleted the new-linter-api branch August 6, 2015 21:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants