-
Notifications
You must be signed in to change notification settings - Fork 581
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
HTMLLintBear: Add settings for individual features #1304
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,9 +28,101 @@ class HTMLLintBear: | |
|
||
@staticmethod | ||
def create_arguments(filename, file, config_file, | ||
enable_doctype: bool=True, | ||
enable_entities: bool=True, | ||
enable_trailing_whitespace: bool=True, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use |
||
enable_tabs: bool=True, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. either |
||
enable_charset: bool=True, | ||
enable_void_element: bool=True, | ||
enable_optional_tag: bool=True, | ||
enable_type_attribute: bool=True, | ||
enable_concerns_separation: bool=True, | ||
enable_protocol: bool=True, | ||
enable_names: bool=True, | ||
enable_capitalization: bool=True, | ||
enable_quotation: bool=True, | ||
enable_indentation: bool=True, | ||
enable_formatting: bool=True, | ||
enable_boolean_attribute: bool=True, | ||
enable_invalid_attribute: bool=True, | ||
enable_void_zero: bool=True, | ||
enable_invalid_handler: bool=True, | ||
enable_http_equiv: bool=True, | ||
enable_extra_whitespace: bool=True, | ||
htmllint_ignore: typed_list(str)=()): | ||
""" | ||
:param htmllint_ignore: List of checkers to ignore. | ||
:param enable_doctype: | ||
Enable or disable doctype checker. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe you should add an example here, it is not quite clear what this setting does |
||
:param enable_entities: | ||
Enable or disable unicode entity checker. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here too... |
||
:param enable_trailing_whitespace: | ||
Enable or disable checker for trailing whitespace. | ||
:param enable_tabs: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be |
||
Enable or disable check for tabs. | ||
:param enable_charset: | ||
Enable or disable utf-8 checking. | ||
:param enable_void_element: | ||
Enable or disable checker for void tags. | ||
:param enable_optional_tag: | ||
Enable or disable checker for optional tags. | ||
:param enable_type_attribute: | ||
Enable or disable checker for default types. | ||
:param enable_concerns_separation: | ||
Enable or disable checker for seperation of concern. | ||
:param enable_protocol: | ||
Enable or disable checker for protocol specification. | ||
:param enable_names: | ||
Enable or disable checker for id or class name delimiters. | ||
:param enable_capitalization: | ||
Enable or disable checker for capitalization | ||
of tags and attributes. | ||
:param enable_quotation: | ||
Enable or disable checker for quotation marks. | ||
:param enable_indentation: | ||
Enable or disable checker for indentation using spaces. | ||
:param enable_formatting: | ||
Enable or disable checker for general formatting. | ||
:param enable_boolean_attribute: | ||
Enable or disable checker for boolean attributes. | ||
:param enable_invalid_attribute: | ||
Enable or disable checker for invalid attributes. | ||
:param enable_void_zero: | ||
Enable or disable checker for javascript void(0). | ||
:param enable_invalid_handler: | ||
Enable or disable checker for Javascript links. | ||
:param enable_http_equiv: | ||
Enable or disable checker for http-equiv. | ||
:param enable_extra_whitespace: | ||
Enable or disable checker for extra whitespace. | ||
:param htmllint_ignore: | ||
List of checkers to ignore. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i feel like, you should instead tell what the setting do instead of telling what is enabled or disabled |
||
""" | ||
param_dict = { | ||
'doctype': enable_doctype, | ||
'entities': enable_entities, | ||
'trailing_whitespace': enable_trailing_whitespace, | ||
'tabs': enable_tabs, | ||
'charset': enable_charset, | ||
'void_element': enable_void_element, | ||
'optional_tag': enable_optional_tag, | ||
'type_attribute': enable_type_attribute, | ||
'concerns_separation': enable_concerns_separation, | ||
'protocol': enable_protocol, | ||
'names': enable_names, | ||
'capitalization': enable_capitalization, | ||
'quotation': enable_quotation, | ||
'indentation': enable_indentation, | ||
'formatting': enable_formatting, | ||
'boolean_attribute': enable_boolean_attribute, | ||
'invalid_attribute': enable_invalid_attribute, | ||
'void_zero': enable_void_zero, | ||
'invalid_handler': enable_invalid_handler, | ||
'http_equiv': enable_http_equiv, | ||
'extra_whitespace': enable_extra_whitespace, | ||
} | ||
|
||
ignore = ','.join(part.strip() for part in htmllint_ignore) | ||
for param, use in param_dict.items(): | ||
if not use and param not in htmllint_ignore: | ||
ignore += ',{}'.format(param) | ||
return HTMLLintBear._html_lint, '--disable=' + ignore, filename |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,3 +27,10 @@ | |
invalid_files=(test_file,), | ||
settings={'htmllint_ignore': 'quotation'}, | ||
tempfile_kwargs={'suffix': '.html'}) | ||
|
||
HTMLLintBearDisableCheckerTest = verify_local_bear( | ||
HTMLLintBear, | ||
valid_files=(test_file,), | ||
invalid_files=(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is no invalid_file given? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For all tests we should have at least an valid and an invalid file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests before don't follow that. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pratyushprakash How does this test check whether disabling a certain setting takes effect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @namanyadav12 could you please view HTMLLintBearTest.py in its entirety and not just the changes I made There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imo testing on this bear is a bit screwy and needs to be improved. This would be separate issue. Ideally we test most of these options, but not possible right now. However since we are trying to improve tests, it would be nice if you verified that the bear was outputting the correct error. |
||
settings={'enable_optional_tag': 'false'}, | ||
tempfile_kwargs={'suffix': '.html'}) |
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 about
check_doctype
? 😅