[docs] Generate linting docs from source#17189
[docs] Generate linting docs from source#17189jugglinmike merged 1 commit intoweb-platform-tests:masterfrom
Conversation
sideshowbarker
left a comment
There was a problem hiding this comment.
I suggest the existing descriptions from the docs/writing-tests/lint-tool.md file — rather than just being dropped — should be migrated over to the tools/lint/rules.py file, to replace the corresponding existing embedded descriptions in that file. Rationale: The existing descriptions in the docs/writing-tests/lint-tool.md file in most cases seem more helpful than the corresponding existing embedded descriptions in the tools/lint/rules.py file. That said, of course the descriptions should be evaluated case-by-case rather than just being replaced without reviewing them.
|
|
||
| if rule["to_fix"]: | ||
| definition += nodes.strong(text="To fix:") | ||
| definition += WPTLintRules._parse_markdown(rule["to_fix"]) |
There was a problem hiding this comment.
So we parse the to_fix as Markdown, but not the description?
There was a problem hiding this comment.
Yup, that's a mistake. Good catch!
|
@sideshowbarker @gsnedders the descriptions are now parsed as Markdown, and the website's text has been preserved in all cases where it was more useful. Additionally, the list is lexicographically sorted now. |
tools/lint/rules.py
Outdated
| class InvalidTimeout(Rule): | ||
| name = "INVALID-TIMEOUT" | ||
| description = "Invalid timeout value %s" | ||
| description = ( |
There was a problem hiding this comment.
Why aren't we just using a triple-quoted string here, as we did in earlier versions of the PR?
There was a problem hiding this comment.
Multi-line strings are fine when rendering markup, but they aren't appropriate for the CLI. Since multi-line strings are easier to maintain, I experimented with normalizing the values at runtime using Python's standard library (specifically textwrap.dedent and inspect.cleandoc). That takes care of indentation, but the interstitial newlines are still a problem.
Rather than further increase complexity with custom formatting code, I opted for the uglier syntax.
|
@gsnedders do you support my decision about multiline strings? If so, would you mind merging this? |
|
@jugglinmike Can you add a comment or something at the top justifying this? I feel like this is something we're highly likely to regress in future, though. And probably add a parametrised test that goes through all the rules and checks the description/to_fix doesn't contain any newlines? |
|
@gsnedders I suggested the requirement to author text without newlines as a compromise to limit complexity. In some ways, writing tests cuts against that goal. It's not that I'm against testing, but I'd rather test something that supports a more Pythonic contribution experience. That is, instead of complaining when folks naturally reach for multi-line strings, I'd rather try to support that pattern and test the logic which enables it. Do you think it's better to treat this as an output sanitization problem? A utility function like this would be easy to test: def prep_for_command_line(text):
return inspect.cleandoc(text).replace('\n', ' ') |
|
@jugglinmike we need more aggressive whitespace normalisation that that, don't we? or we'll just end up with lots of consecutive whitespace? (or does that work fine because it's interpreted as Markdown and whitespace is otherwise collapsed?) |
|
I think the combination of import inspect
strings = [
'''a
b
c''',
'''
a
b
c''',
'''
a
b
c
''',
'''a
b
c
'''
]
for string in strings:
print inspect.cleandoc(string).replace('\n', ' ') |
|
@jugglinmike I got side-tracked and forgot to check what |
|
@gsnedders I now remember another wrinkle: the tests inspect the Python objects directly (as opposed to the CLI output). That means we can't clean these values as we write them out--they have to be stored in sanitized form. I've added a function named Another solution might be perform the Whatever we decide, future contributors will be made aware of extra whitespace without any new tests because the current tests assert the |
d7f1f3b to
c5d18fb
Compare
|
@gsnedders There was a strange failure in Taskcluster that seemed to be unrelated to the change logs...except that it was consistent. I've rebased to try to address that (the prior version of this branch is available here). Also fixed a problem flagged by the type system. Would you mind taking another look at this? |
|
@gsnedders gh-19952 improved the feedback we get for documentation patches. I've merged |
When new rules have been added to WPT's "lint" tool, the corresponding documentation has not always been updated [1] [2] [3]. The static list of rules currently describes only 22 of the 53 available rules. Automatically generating documentation from source code helps avoid this state and the confusion it can cause contributors. Rely on the previously-implemented source code structure [4] during documentation generation to automatically create a listing of all available linting rules. Although the Sphinx documentation generator includes a built-in extension for generating documentation from Python source code, the output of that extension is designed to document Python primitives such as functions and classes. Such a format is inappropriate for this case because the users of the linting tool do not interact with the internals in this way. Define a custom docutils directive to tailor the documentation to the needs of its audience. [1] web-platform-tests#5299 [2] web-platform-tests#10501 [3] web-platform-tests#11479 [4] web-platform-tests#16268
c002098 to
29c9630
Compare
|
I've rebased this as per the recent advice to contributors. The prior version of this branch is available here. @gsnedders Last call :) |
When new rules have been added to WPT's "lint" tool, the corresponding
documentation has not always been updated [1] [2] [3]. The static list
of rules currently describes only 22 of the 53 available rules.
Automatically generating documentation from source code helps avoid this
state and the confusion it can cause contributors.
Rely on the previously-implemented source code structure [4] during
documentation generation to automatically create a listing of all
available linting rules.
Although the Sphinx documentation generator includes a built-in
extension for generating documentation from Python source code, the
output of that extension is designed to document Python primitives such
as functions and classes. Such a format is inappropriate for this case
because the users of the linting tool do not interact with the internals
in this way. Define a custom docutils directive to tailor the
documentation to the needs of its audience.
[1] #5299
[2] #10501
[3] #11479
[4] #16268
As noted in the commit message, this nearly doubles the number of documented
linting rules. Over time, we can further improve it by separating out explicit
"to fix" instructions (and maybe even require that new rules do the same).