Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using an HTMLLinter ? #1801

Closed
gaurav21r opened this issue Mar 3, 2016 · 21 comments
Closed

Consider using an HTMLLinter ? #1801

gaurav21r opened this issue Mar 3, 2016 · 21 comments

Comments

@gaurav21r
Copy link

With the rise of new technologies like WebComponents, as developers use more and more HTML in their projects (more tags, more nesting, more attributes) I think it should be important to add (and extend) an HTML Linter.

According to our research at rcorp/standard-project-structure#4,
HTMLhint is the best one out there. How about a standard .htmlhintrc file ?

@quantumpacket
Copy link

How would the rules used be chosen? Some of the standard rules are really personal preference. For example:

https://github.com/yaniswang/HTMLHint/wiki/Attr-value-double-quotes both are perfectly valid HTML5 and I don't think this boilerplate should be choosing what style should be the default.

https://github.com/yaniswang/HTMLHint/wiki/Tag-self-close both are valid HTML5, and the latter is actually the more modern way to do it as the prior is for XHTML and is allowed in HTML5 for backwards compatibility when migrating from XHTML to HTML5 as far as I am aware.

https://github.com/yaniswang/HTMLHint/wiki/Attr-value-not-empty both are valid and the latter is what most people use, but it's being considered the bad way.

A lot of these rules are dubious and would cause parts of the boilerplate to be marked as errors or warnings. Due to that, I'm against adding this and would rather such preferences be left up to the developer and not have it bundled up with this project. Maybe add a HTML linter section to the docs and provide links to various linters?

@gaurav21r
Copy link
Author

@quantumpacket As you mentioned tag-self-close there is always at least some justification for choosing one over the other and isn't here where the community can weigh in just like they can over the contents of the boilerplate? Where there is no justification, we simply do not add that rule. For reference if we look at all the code style's for JS rcorp/standard-project-structure#6 especially AirBNB, we can see that there are a lot of rules but each of them has a reasoning which the developer will appreciate.

I totally get your point, but these are just rules. The ones we don't choose aren't enforced. So it can be a very lenient approach. and a boilerplate for developers to add / remove rules from .htmlhintrc

@roblarsen
Copy link
Member

roblarsen commented Aug 22, 2016

@gaurav21r Sorry for letting this linger. I think that this would be a good idea. (did you want to propose one with a PR? I'll flag this help wanted for now.)

As for the which values to choose, the starting point will be evident from the project itself. In the case of the examples @quantumpacket shared- we use double quotes, use empty attributes and don't self-close. From there, I'm sure people will have opinions.

@gaurav21r
Copy link
Author

@roblarsen We'd love to contribute with a PR. We've already selected a good starting base for the ruleset. rcorp/standard-project-structure#13

Its used by a couple of big orgs. I'll see if it matches the boilerplate and is not too opinionated. Rest we can discuss in the PR itself!

@roblarsen roblarsen modified the milestone: 6.0.0 Sep 30, 2016
@AlexxNica
Copy link

Any progress on this issue?

@roblarsen
Copy link
Member

@gaurav21r Any update? If no, anyone else want to take a stab at this?

@AlexxNica
Copy link

@roblarsen If it happens that @gaurav21r is not able to proceed I'll be happy to do it.

@roblarsen
Copy link
Member

@AlexxNica sounds good. We always love the help.

@gaurav21r
Copy link
Author

Sorry for the very late response! @roblarsen @AlexxNica Will try to do this by 15th. If not, will be happy to help whoever is!

@roblarsen
Copy link
Member

no worries!

@roblarsen
Copy link
Member

The 15th has come and gone! This is open to whoever wants to try to do it!

@AlexxNica
Copy link

Are we sure HTMLHint is the best option out there?

@roblarsen
Copy link
Member

@AlexxNica I have zero experience on this front, so I'm along for the ride. Whoever wants to take this on can open some discussion here and I'll do my best to provide feedback as warranted, but this is definitely something that's in the community's hands.

@AlexxNica
Copy link

AlexxNica commented Mar 18, 2017

We'll have to get a good look at this before adding more code into the project. Things to be sure before we take any action:

· Is it helpful enough to justify the need to implement?
· Is it easy to dispatch and maintain?
· Did it get tested by someone/us?
· Do we see real improvement over the actual model?
· Did we run the tests against our code to see if we'll not have to change anything to be compliant to the analyzer? (so users don't get stuck with more errors on the screen than their monitor can display)
· Will it have a long-term presence? (I don't think it's a good idea to implement something that won't be existing for a solid amount of time)
· What is its ease of use? Won't users be stuck with more stuff to manage that they can't understand?

@AlexxNica
Copy link

AlexxNica commented Mar 22, 2017

@roblarsen Can you consider changing the labels on this? I think "awaiting feedback" and (maybe) "HTML" would be more appropriate.

@roblarsen
Copy link
Member

I added those. It's still help-wanted and still a new feature. I'm not going to touch this personally, so it's definitely something the community will have to run with.

@roblarsen roblarsen removed this from the 6.0.0 milestone Jul 11, 2017
@coliff
Copy link
Member

coliff commented Jul 22, 2017

I think there's two questions here:

  • Should we include a .htmlhintrc in the src/dist folders for users?
    HTMLHint is only really intended to be used to lint full, compiled HTML pages (for example, be default it checks that every file has a <title> tag so It is not really suitable for most server side or templating languages. Therefore I think it’s of limited value as so many users are using Angular, React, PHP, ASP or templating languages like Liquid where it will either give many errors or not work at all. A user could edit all the rules to get it not display errors but then that kind of defeats the purpose of including it. And all users would need to install a plugin for their IDE to be able to use it. I just don’t think enough users are coding web pages in pure HTML without includes to warrant its inclusion (I personally only use HTMLHint when coding HTML emails - its great for finding bugs) - and we don’t include any JS or CSS linters/hinter config files.

  • Should we include a HTMLHint in the build system to check for errors?
    Yes, I think this could be a good idea. Our build system is currently using Gulp and there is a NPM package for it to be integrated: https://www.npmjs.com/package/gulp-htmlhint. I'll look into this over the next few days. This would check for HTML errors when compiling and could be beneficial for finding issues prior to deploying a site.

@ArmorDarks
Copy link

Inability to use html linters with templating languages is something that always prevented us from using it. React is a bit another story, since jsx can be effectively linted with ESLint.

Linting build seems to be beneficial for project, but not that for end user. But seems to be better than nothing, at least it will prevent occasional errors in source of boilerplate.

Btw, I don't see reasons to use Gulp for such trivial tasks. It is always easier and more reliable to avoid additional layers for linters and call them directly from npm test (see example).

@XhmikosR
Copy link
Member

XhmikosR commented Oct 1, 2017

IMO, you should stay away from non-standard tools.

It's like you use no validator at all, and I'm speaking from experience since on Bootstrap v4-dev they had switched to htmlhint which basically didn't catch any real errors.

My suggestion would be to use the official validator vnu.jar even though it requires Java. You can have a wrapper script which would run it if Java is available.

I made a similar change recently in Bootstrap (see twbs/bootstrap#24149) because I was annoyed by the fact that we were basically without any validation with htmlhint.

@coliff
Copy link
Member

coliff commented Mar 6, 2018

HTMLHint is unfortunately no longer actively maintained (https://github.com/yaniswang/HTMLHint) and as @XhmikosR says - there are issues with so I vote to close this issue.

@roblarsen
Copy link
Member

Crickets + the above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants