Remove broken scopes

[Arcanemagus]

Removes a bunch of scopes that this linter doesn't properly handle.

Fixes #44.
Fixes #45.
Fixes #47.
Fixes #50.

[Arcanemagus]

@johnwebbcole Does htmlhint actually handle anything other than basic HTML? I'll be merging this in a few hours if I don't hear from you since as it currently stands this is just breaking things for people.

[johnwebbcole]

Your removing a lot is scopes that I use.

On Nov 16, 2015, at 3:23 PM, Landon Abney notifications@github.com wrote:

@johnwebbcole Does htmlhint actually handle anything other than basic HTML? I'll be merging this in a few hours if I don't hear from you since as it currently stands this is just breaking things for people.

—
Reply to this email directly or view it on GitHub.

[Arcanemagus]

Does htmlhint properly handle those? Those are all templated scopes and from the looks of it htmlhint is only intended for pure HTML.

[Arcanemagus]

@johnwebbcole almost half of the open issues are related to this linter linting things that it shouldn't be, are any of the scopes that are removed by this actually handled by htmlhint?

[Arcanemagus]

Did some testing/research:

Scope	Supported
text.html.angular	No
text.html.erb	No
text.html.gohtml	No
text.html.jsp	No
text.html.mustache	Yes?
text.html.handlebars	No
text.html.ruby	No

I might consider text.html.angular as it seems that it is somewhat "valid" HTML, it's main issue being custom attributes on the elements which isn't allowed strictly following the spec. The fact that htmlhint doesn't know to check for that is a ding against it, but it means that files of that scope might work here. The same reasoning goes for Mustache.js templates: They are close enough to real HTML that I think they could probably stay for now if absolutely necessary.

The issue with both of them though is that if htmlhint is ever improved those will start becoming invalid as it is only designed for HTML.

Do you want me to add those two scopes back in pending future removal when htmlhint improves, or just remove them now so the users of this aren't confused later?

[johnwebbcole]

I'm using angular, handlebars, and mustache every day. And it works great. Many of the errors reported don't have to do with htmlhint, but the linter helpers.

[Arcanemagus]

@johnwebbcole this is the "linter helper", so I'm not sure what you are talking about there. If you are saying those issues are problems with how this provider is currently written then can you fix those errors so the users of this no longer need to file issues on it?

As I said in the post, the templating languages range from "looks to be valid" to "htmlhint isn't smart enough to see they aren't", but I can put those back in. You need to know that they can break in the future though and should be broken right now.

As the linked issue states, this is already broken for Handlebars, and you should probably be using linter-handlebars for that...

[johnwebbcole]

If there is a specific linter that supersedes htmlhint, then htmlhint should not include that scope. So if there is one for handlebars, then it should be removed from the scope. Keep in mind that adding handlebars was specifically requested.

Looks like erb has a specific linter now too.

The others seem to work ok, while some need a rule or two disabled (but the other rules run fine), which is easily handled with a .htmlhintrc file.

This is a linter plugin, not a helper. There is a linter helper library that this plugin uses. When the main linter gets updated, there have been problems in the past with that helper library. I've found that waiting a few days to make sure the library changes are stable helps a great deal. Many issues reported are resolved on the next linter update, as the same pattern is used by other plugins and they break in a similar way.

Scopes that need one rule disabled should not be removed. A documentation note suggesting a .htmlhintrc setting would be nice though. If there is a linter for that type, we should remove it from the scopes and let the other linter plugin work, if not htmlhint works pretty well still.

[Arcanemagus]

I just looked through the issues and not a single one was related to atom-linter (the helper package I'm assuming you are talking about), other than one that was an Atom bug related to profile paths with non-ASCII characters.

As explained in #44 disabling that rule isn't really a solution as that disables it for the entire file. This linter was never designed to handle templated files.

It looks like you added all these extra scopes in 3715c5d in order to "fix" #26, which you never really got confirmation that that was even the issue for.

I'll take out the removal of Mustache and Angular, but I really don't see how the other scopes should be kept.

[Arcanemagus]

The entire point of this PR is that quite a few of the issues currently open on this provider are due to it attempting to lint files it shouldn't be, and the issues were not being handled.

[johnwebbcole]

We should only remove scopes if there is an alternative linter available. Otherwise, leave them in, and users can configure the rules with .htmlhintrc files and loose partial functionally, not all of it. The only other linter available is linter-tidy, and I think that htmlhint works much better than tidy, and is much more configurable.

[johnwebbcole]

A better way to do this would be to enumerate the text.html.* scopes and provide options to disable them.

[Arcanemagus]

That works well enough, it can be done similar to how linter-eslint does it. I'll see if I have some time to implement that unless you want to.

[johnwebbcole]

Is there a way to git a list of all of the text.html.* scopes?

[Arcanemagus]

I would only list the ones requested so far... but in any case this is the only list I know of: https://github.com/github/linguist/blob/master/lib/linguist/languages.yml

[Arcanemagus]

Closing this, there doesn't seem to be too much interest in it, and the large number of incoming issues seems to have died off anyway.