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

Add support for disabling checks via comments #1315

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Holzhaus
Copy link

In some cases, there is a false positive or the writer knowingly wants
to ignore a failing check in a specific section of the text, but not
the entire document.

With this change, checks can be enabled and re-enabled at any time using
special comments:

Here, all checks are used.
% proselint: disable
No checks are enabled here.
% proselint: enable
All checks have been reenabled.

It's also possible to disable only some checks:

Here, all checks are used.
% proselint: disable=nonwords.misc,weasel_words.very
Here, the \texttt{nonwords.misc} and \texttt{weasel\_words.very}
checks are disabled.
% proselint: enable=nonwords.misc
At this point, the \texttt{nonwords.misc} check has been reenabled.

This aims to be markup-language agnostic by allowing any printable
non-word ASCII character in front of `proselint: instead of maintaining
a list of allowed comment styles. Note that there mustn't be any word
characters, so toggling checks in the middle of a line is not possible,

This is a very minor performance improvement, because line and column
are not used if the error is quoted.
In some cases, there is a false positive or the writer knowingly wants
to ignore a failing check *in a specific section of the text*, but not
the entire document.

With this change, checks can be enabled and re-enabled at any time using
special comments:

    Here, all checks are used.
    % proselint: disable
    No checks are enabled here.
    % proselint: enable
    All checks have been reenabled.

It's also possible to disable only some checks:

    Here, all checks are used.
    % proselint: disable=nonwords.misc,weasel_words.very
    Here, the \texttt{nonwords.misc} and \texttt{weasel\_words.very}
    checks are disabled.
    % proselint: enable=nonwords.misc
    At this point, the \texttt{nonwords.misc} check has been reenabled.

This aims to be markup-language agnostic by allowing any printable
non-word ASCII character  in front of `proselint: instead of maintaining
a list of allowed comment styles. Note that there mustn't be any word
characters, so toggling checks in the middle of a line is not possible,
@Holzhaus
Copy link
Author

Ping, this would come in handy at times.

@@ -230,6 +231,48 @@ def line_and_column(text, position):
return (line_no, position - position_counter)


def find_ignored_checks(available_checks, text):
ignored_checks = collections.defaultdict(dict)
for match in re.finditer(r"^[!-/:-@[-`{-~\s]*proselint: (?P<action>disable|enable)(?P<checks>(?:=(?:[\w\.,]*)))?", text, flags=re.MULTILINE):
Copy link
Member

@Nytelife26 Nytelife26 Apr 6, 2023

Choose a reason for hiding this comment

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

Consider the computational cost of this. RegExes are expensive and moderately slow, so running one on every line just to determine whether or not we can disable some checks feels inefficient.

I am not quite sure what the solution to this would be yet, but in the meantime any ideas are welcome. They may not be quite as simple or elegant, but proselint's performance is lacking as it stands, and implementing this would be a major blow in the wrong direction.

Update: Upon further thought, the changes to how proselint works at its core are not going to be here for some time. I would be willing to accept this as a temporary solution.

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

Successfully merging this pull request may close these issues.

2 participants