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.

Sign up for GitHub

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

restLintBear: Ignore unknown errors #1718

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

restLintBear: Ignore unknown errors #1718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion bears/rest/reSTLintBear.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import re

from restructuredtext_lint import lint

from coalib.bears.LocalBear import LocalBear
Expand All @@ -6,6 +8,18 @@
from coalib.results.RESULT_SEVERITY import RESULT_SEVERITY


def _ignore_unknown(errors):
for error in errors:
msg = error.full_message
res = re.search(
r'No directive entry for "[\w|\-]+"|'
r'Unknown directive type "[\w|\-]+"|'
r'No role entry for "[\w|\-]+"|'
r'Unknown interpreted text role "[\w|\-]+"', msg)
if not res:
yield error


class reSTLintBear(LocalBear):
LANGUAGES = {'reStructuredText'}
REQUIREMENTS = {PipRequirement('restructuredtext-lint', '1.0.0')}
Expand All @@ -14,13 +28,17 @@ class reSTLintBear(LocalBear):
LICENSE = 'AGPL-3.0'
CAN_DETECT = {'Formatting', 'Syntax'}

def run(self, filename, file):
def run(self, filename, file, ignore_unknown_roles: bool=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directives are not roles, so either this is ignore_unknown: bool, or it is split into ignore_unknown_roles and ignore_unknown_directives.

Even if we did plan on deprecating this bear, this PR still makes sense, as it will help us implement the bear, and thus understand the bear ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gah I am not sure if we deprecate the bear, it will be worth doing this.

But now that I have seen rstcheck's way of doing this, I would also like to have two params ignore_directives, ignore_roles, which takes in a list of directives/roles to ignore. And then obviously do the stuff with regex and the helper function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont see the benefit of replicating rstcheck , unless we know of a feature in restlint that is missing from rstcheck...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really sure, there is no feature "unimplemented" in restlint because both uses docutils

However, I feel restlint is the "purer" form of a rst linter, because rstcheck has "batteries" included like sphinx support etc. This is the only argument I have,

That said, I am up for deprecating restlint for the sake of bear uniformity.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c.f. #1723

Also see coala/coala#3044 (comment) , where I think I saw extra error messages from restlint than rstcheck emitted .

I think you might be right about the need for a purer rst linter. https://pypi.python.org/pypi/restructuredtext_lint reminds me ... we need a rst-checker which validates README.rst so that it can be used for PyPI, which has stricter rules than Sphinx, and stricter rules than GitHub's RST parser.

Maybe we can request rstcheck has a strict rst mode. Oddly enough, this coala project seemed like it might have created a more clear separation between docutils mode and sphinx mode.

And if we need a pure rst linter, then we dont want to allow it to validate sphinx rst. It should fail! ;-)
And we should implement restLintBear on any README.rst or other file which is included in the packaging for PyPI, and not other RST files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So close this PR then seems the right way?

"""
Lints reStructuredText.

:param ignore_unknown_roles: Ignore unknown directives and roles.
"""
content = ''.join(file)
errors = lint(content)

errors = _ignore_unknown(errors) if ignore_unknown_roles else errors

for error in errors:
severity = {
1: RESULT_SEVERITY.INFO,
Expand Down
9 changes: 9 additions & 0 deletions tests/rest/reSTLintBearTest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,16 @@
good_file = 'test\n====\n'
bad_file = 'test\n==\n'

good_file2 = ':doc:`here <../Users/Tutorial>`'
bad_file2 = 'test\n==\n'

reSTLintBearTest = verify_local_bear(reSTLintBear,
valid_files=(good_file,),
invalid_files=(bad_file,))


reSTLintBearSettingsTest = verify_local_bear(
reSTLintBear,
valid_files=(good_file2,),
invalid_files=(bad_file2,),
settings={'ignore_unknown_roles': True})