External formatter modules? about tslint

[ashwinr]

There's currently no support for external formatters, but it should be straightforward to implement this feature. I'll add it to the TODO.

[Bartvds]

Cool, I look forward to it. I'm a bit busy with other projects otherwise I'd fork and try it myself.

Ideally the reported data would have a similar structure as the other hinters, so with the results split per file. This so the warning report can be grouped. (Note JSHint has the reporter API a bit messy for legacy reasons, while ESLint has a clean approach because they're pretty new).

[ashwinr]

I see. Cool, thanks for the explanation.

[Bartvds]

I have a preliminary implementation for this in a branch of my fork (because I desperately wanted to use it like this).

I'm using it with this custom formatter (not yet published on npm) (the format allows for easy mouse-click navigation in certain editors)

It works as is but there are a few points:

Right now TSLint will first check for it's internal formatter, then try to require() it (so it resolves paths and module names). I played with testing for existence and printing a message but I think it might just error out so the user can see he messed up a module require().

What bothers me slightly is how TSLint prints output by file: JSHint ans ESLint both report all the files at once: their reporters can do stuff like printing a summary at the end of the linting. It is not critical but might be nicer to upgrade that (at a later stage).

Let me know what you think, I'm open for suggestions.

[ashwinr]

Thanks, I'll take a look. I'm working on modularizing all of tslint, so we can make both rules and formatters pluggable.

[ashwinr]

Both formatters and rules are now pluggable and available in the latest npm versions of both tslint and grunt-tslint. I'll add some README later, but for now specify "rulesDirectory: " or "formattersDirectory: " in the Gruntfile. The rule or formatter module is named either "Rule.js" or "Formatter.js" as applicable, and you can even write them in TypeScript! If the module is written in TypeScript, copy over lib/tslint.d.ts and reference it (look at how we do it for the core rules and formatters which are themselves written as pluggable modules). You'd still need to compile the TS file for the core tslint module to find it, but that should be very straightforward. Lemme know if you have any questions or find issues.

[ashwinr]

oh, as a result of this change, I've also renamed tslint rules to be much more readable. breaks with the previous rule configuration. sorry!

[Bartvds]

Nice progress, although I don't really understand why the formatter should be configured like that (with a fixed name and a path to a folder). That works for rules and internal formatters but for external formatter it is less-then-ideal.

For npm modules it'd be a lot cleaner to add a require() based lookup that uses the standard module resolution. So you can just pass the name of the module you want to use as a formatter instead of having to reach deep into a module (like './node_modules/name-of-my-formatter/lib/Formatter.js'). Using this you just need 1 CLI argument (module name) instead of two (folder and name).

I had this working in my fork so it is easy enough to add: I'll re-merge it onto new version and send a PR when it works (I even have test for it somewhere).

[ashwinr]

Cool, send me a PR and I'll merge it in. If formatters are npm modules themselves, your approach's better.

[Bartvds]

I found two problems:

The formatters and rules are compiled to the /build folder that is set to be ignored by git (and not published to npm).

Also I'm pretty sure the CORE_FORMATTERS_DIRECTORY and CORE_RULES_DIRECTORY in formatterLoader.ts and rulesLoader.ts will not resolve correctly when tslint is used in another module (the implied CWD will be off: instead use the module.filename).

[Bartvds]

I have working fixes for both:

• Compile the rules and formatters to ./lib/rules and ./lib/formatters.
• Fix the relative path bug with some path.resolve magic (to resolve from both the ./cli, ./lib and ./build).

To make this work I had to compile the tests directly to ./build (instead of ./build/test), this keeps the internal relative paths at same depth so they work without too much magic. But I think this is fine since the test file has a -tests.js postfix.

I'll add this with the edits I was doing for the external formatter.

[ashwinr]

Formatters and rules are published to npm, they're just not in git (I didn't see a reason to check them in as they're build files). Can you please explain why the rules and formatters are now compiled at lib instead of build? What wasn't working before?

[ashwinr]

So even though .gitignore ignores build/, .npmignore doesn't, which is why they're published to npm.

[Bartvds]

I didn't see the rules and formatters in the checkout and assumed the .gitignore stacks with .npmignore (as it usually does, but apparently that is not the case here).

I expected them to be checked in just like the compiled contents of ./bin and ./lib are also checked in that depend on the rules and formatters (so everything is completely there, for consistency), and it looked as if ./build was to be used only for the tests.

This combined with the way grunt-tslint malfunctioned for that path bug it made me think something got messed up in your last changes, so to get it working I moved them to ./lib (seemed the correct place).

Let me know what to do, I can easily put them back.

[ashwinr]

I think it's cumbersome to check in and maintain all the generated rule and formatter javascript files along with their original TypeScript sources. For projects like eslint, it makes sense because their source is in JS as well. I don't suspect anyone using tslint by copying the binary directly from the repo instead of using npm. I have a task to remove all generated files (except lib/tslint.d.ts) to clean up the repository.

If you could put back the original build/rules and build/formatters, that would be much appreciated!

[Bartvds]

Sure, I'll revert those changes and flatten the rest of commits and repush that.

One could argue the build code in ./lib and ./cli don't have to be there either.

But to be honest my own TypeScript project (rewrite for tsd for DefinitelyTyped) currently also has some build files checked-in, although now I think about it I might remove them: they have no real use being there and it would clean up the commit diffs and stats.

[ashwinr]

Exactly! I have a task to remove the built bin and lib files after I merge in your commit. Thanks for the squash.

Sent from a mobile phone

On Oct 9, 2013, at 14:13, "Bart van der Schoor" <[email protected]mailto:[email protected]> wrote:

Sure, I'll revert those changes and flatten the rest of commits and repush that.

One could argue the build code in ./lib and ./cli don't have to be there either.

But to be honest my own TypeScript project (rewrite for tsd for DefinitelyTyped) currently also has some build files checked-in, although now I think about it I might remove them: they have no real use being there and it would clean up the commit diffs and stats.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/9#issuecomment-25994568.

[Bartvds]

I updated the PR, so the rules and formatters are back in the build folder. See the commit note for info on what is left in the flattened commit.

And to continue the discussion on build files: I did re-discover an argument for checking-in all build files: what I sometimes do to test a npm module is to only push it to github and use the git url as dependency (instead of the npm version number).

For example in my project I link to the git url of my fork of grunt-tslint (which again links to the git url of my tslint fork) so I can test as if it was pushed to npm. Of course this breaks when the npm package has more content then the repos.

Nice complication for the overall consideration.