Skip to content

feat: support noqa before multiline strings#318

Draft
tacerus wants to merge 1 commit intowarpnet:mainfrom
tacerus:multiline-noqa
Draft

feat: support noqa before multiline strings#318
tacerus wants to merge 1 commit intowarpnet:mainfrom
tacerus:multiline-noqa

Conversation

@tacerus
Copy link
Copy Markdown

@tacerus tacerus commented Jan 5, 2024

Allow rules applying to lines inside multiline strings inside lists to be skipped by using the "noqa" statement in the line of the multiline start marker.

Allow rules applying to lines inside multiline strings inside lists
to be skipped by using the "noqa" statement in the line of the
multiline start marker.

Signed-off-by: Georg Pfuetzenreuter <mail@georg-pfuetzenreuter.net>
@tacerus
Copy link
Copy Markdown
Author

tacerus commented Jan 5, 2024

See #317.

@roaldnefs roaldnefs self-requested a review April 27, 2026 17:27
Copy link
Copy Markdown
Member

@roaldnefs roaldnefs left a comment

Choose a reason for hiding this comment

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

As-written the patch silently breaks all existing noqa usage. Easy fixes; should not be marked ready until both bugs are fixed and a TestSkipRule case mirroring issue #317 is added.

Two blocking bugs:

  1. Operator precedence — > in any line propagates skips: if last['line'].startswith('-') and '|' in last['line'] or '>' in last['line'] parses as (startswith AND '|') OR '>'. So a Jinja line like {{ x > 5 }} after a list-item-with-noqa inherits the skip. Fix by parenthesizing the or.
  2. Current-line noqa is now ignored: skips is built only from last['skips']; the final check if self.id in skips no longer consults rule_id_list. Every existing key: value # noqa: 204 on a non-multiline line stops working. Fix by building a combined list that includes rule_id_list.

Other notes:

  • No tests. tests/unit/TestSkipRule.py is the obvious place; tests would have caught both bugs immediately.
  • not previous_skip in rule_id_listprevious_skip not in rule_id_list (pylint C0117).
  • Stray blank line at L84
  • No CHANGELOG.md entry

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