-
Notifications
You must be signed in to change notification settings - Fork 576
bears/hypertext: Add HTMLHintBear #1987
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
Conversation
CC @Makman2 |
bears/hypertext/HTMLHintBear.py
Outdated
@linter(executable='htmlhint', | ||
output_format='regex', | ||
output_regex=r'(?P<filename>.+):(?P<line>\d+):(?P<column>\d+):\s*' | ||
r'(?P<message>.+). \[(?P<severity>error|warning)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should add the closing \]
to the regex ;)
bears/hypertext/HTMLHintBear.py
Outdated
@linter(executable='htmlhint', | ||
output_format='regex', | ||
output_regex=r'(?P<filename>.+):(?P<line>\d+):(?P<column>\d+):\s*' | ||
r'(?P<message>.+). \[(?P<severity>error|warning)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the dot for after <message>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That marks the end of the sentence after which the severity begins.
Example:
xyz.html:2:8: The attribute name of [ TYPE ] must be in lowercase. [error/attr-lowercase]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to escape it then with a \
. Also match the whole string not just until error/warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current .
represents any character
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it look good now @SanketDG ?
bears/hypertext/HTMLHintBear.py
Outdated
disallow_inline_style: bool=True, | ||
enforce_relative_links_in_href: bool=None, | ||
prohibit_unsafe_characters: bool=True, | ||
disallow_inline_script: bool = False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove spaces around =
operator
bears/hypertext/HTMLHintBear.py
Outdated
:param allow_attribute_value_in_double_quotes: | ||
Allow attribute values to be enclosed in double quotes. | ||
For example: If set to ``True``, prefer ``<a href="" title="abc">`` | ||
over ``<a href='' title=abc>``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allow
sounds like both are allowed, single and double quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to require_attribute_value_in_double_quotes
bears/hypertext/HTMLHintBear.py
Outdated
:param prohibit_empty_attribute: | ||
Disallow empty values for attributes. | ||
For example: If set to ``True``, prefer | ||
``<input type="button" disabled="disabled">`` over |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use a different attribute name, that's not the same as its value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to prohibit_empty_value_for_attribute
bears/hypertext/HTMLHintBear.py
Outdated
``<input type="button" disabled="disabled">`` over | ||
``<input type="button" disabled>``. | ||
:param prohibit_attribute_duplication: | ||
Disallow the defining of the same attribute more than once in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--> Disallow defining the same attribute more than ...
bears/hypertext/HTMLHintBear.py
Outdated
:param enforce_self_close_empty_tag: | ||
Enforce the empty tag to be closed by self. | ||
For example: If set to ``True``, prefer ``<br />`` over ``<br>``. | ||
:param allow_escaped_special_characters: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again: You don't allow this case together with other cases, so you need to rename this too require_...
bears/hypertext/HTMLHintBear.py
Outdated
``<span>aaa>bbb<ccc</span>`` over | ||
``<span>aaa>bbb<ccc</span>``. | ||
:param enforce_unique_attribute_id: | ||
Require the ID attributes to be unique in the document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not call the setting require_...
if you already meantion it inside the description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool 👍
bears/hypertext/HTMLHintBear.py
Outdated
Disallow the defining of the same attribute more than once in | ||
a tag. For example: If set to ``True``, prefer | ||
``<img src="a.png" />`` over ``<img src="a.png" src="b.png" />``. | ||
:param use_doctype_at_beginning: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again you can use require
bears/hypertext/HTMLHintBear.py
Outdated
``<div id="id1"></div><div id="id1"></div>``. | ||
:param require_title_tag: | ||
Require the ``<title>`` to be present in the ``<head>`` tag. | ||
:param disallow_script_in_head: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--> prohibit...
bears/hypertext/HTMLHintBear.py
Outdated
:param disallow_script_in_head: | ||
Prohibit the use of the ``<script>`` tag in the ``<head>`` tag. | ||
:param require_alt_attribute: | ||
Require alt attribute when using images(``img`` tag) and links |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing space before (
bears/hypertext/HTMLHintBear.py
Outdated
:param disallow_script_in_head: | ||
Prohibit the use of the ``<script>`` tag in the ``<head>`` tag. | ||
:param require_alt_attribute: | ||
Require alt attribute when using images(``img`` tag) and links |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should enclose alt
in double quotes
bears/hypertext/HTMLHintBear.py
Outdated
|
||
:param enforce_id_class_naming_convention: | ||
Possible values are ``underline``, ``dash``, ``hump`` and ``false`` | ||
to disable the rule. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't mention false
, not using the setting should disable it.
bears/hypertext/HTMLHintBear.py
Outdated
For example: If set to ``underline``, prefer | ||
``<div id="aaa_bbb">``. | ||
For example: If set to ``dash``, prefer ``<div id="aaa-bbb">``. | ||
:param disallow_inline_style: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--> prohibit
bears/hypertext/HTMLHintBear.py
Outdated
will raise a warning. | ||
:param enforce_relative_links_in_href: | ||
If ``True``, enforce relative links in the ``href`` attribute and | ||
if ``False``, enforce absolute links. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--> require
bears/hypertext/HTMLHintBear.py
Outdated
For example: If set to ``True``, | ||
``<li><a href="https://vimeo.com//56931059\u0009">2012</a></li>`` | ||
will raise a warning. | ||
:param disallow_inline_script: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--> prohibit
@yash-nisar it should be |
Oh, missed that. Will fix it 👍 @Makman2 review possible ? |
bears/hypertext/HTMLHintBear.py
Outdated
@linter(executable='htmlhint', | ||
output_format='regex', | ||
output_regex=r'(?P<filename>.+):(?P<line>\d+):(?P<column>\d+):\s*' | ||
r'(?P<message>.+)\. \[(?P<severity>error|warning).+\]') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually I'm wondering why you don't match the period too for the message :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
bears/hypertext/HTMLHintBear.py
Outdated
elif require_relative_links_in_href is False: | ||
options['href-abs-or-rel'] = 'abs' | ||
else: | ||
options['href-abs-or-rel'] = 'false' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm maybe inline it above doing like this:
'href-abs-or-rel': 'rel' if require_relative_links_in_href else
'abs' if require_relative_links_in_href is False else
'false'
As the assignment is repeated here, we should try to avoid that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also maybe change the if
s to this to make it more intuitive (but optional, both do the same):
'false' if require_relative_links_in_href is None else
'rel' if require_relative_links_in_href else
'abs'
<!DOCTYPE html> | ||
<SPAN> | ||
</SPAN> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whitespacing on purpose?
{ | ||
"inline-script-disabled": true, | ||
"inline-style-disabled": true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no newline on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've faced this issue a lot of times. Should I open an issue for a NewlineBear
or similar ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SpaceConsistencyBear
already supports this ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should enable that feature on all the files ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you like ;) however sometimes test files shall contain such inconsistencies (not in your case though)
@@ -0,0 +1,3 @@ | |||
<!DOCTYPE html> | |||
<a href="test.html">test1</a> | |||
<a href="http://www.alibaba.com/">test1</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also on purpose this missing newline?
Updated @Makman2 |
@@ -0,0 +1,2 @@ | |||
<!DOCTYPE html> | |||
<input type="submit" value="Submit" type="submit1"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leading space on purpose?
@@ -0,0 +1,2 @@ | |||
<!DOCTYPE html> | |||
<input TYPE="submit" value="Submit"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leading space on purpose?
Add `HTMLHintBear` that was worked upon by @AsnelChristian. This PR is a continuation of @AsnelChristian's PR with some modifications. Closes coala#635
Done @Makman2 |
ack b49553a |
@rultor merge |
Add
HTMLHintBear
that was worked upon by @AsnelChristian.This PR is a continuation of @AsnelChristian's PR with some
modifications.
Closes #635
For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!
Checklist
them.
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!
After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.
Please consider helping us by reviewing other peoples pull requests as well:
cobot mark wip <URL>
to get it outof the review queue.
The more you review, the more your score will grow at coala.io and we will
review your PRs faster!