Skip to content

Update update_data.py: make the space in error comments optional #19546

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

wyattscarpenter
Copy link
Contributor

@wyattscarpenter wyattscarpenter commented Jul 30, 2025

Make the spaces in test error comments optional and more flexible, so instead of #E: whatever you can now write # E: whatever (for example). This will prevent people from erroneously entering error lines which are then ignored. Fixes #19547.

I have written tests for this feature, but have not affirmatively documented it, because I don't want to encourage irregularity in the error comments people write.

Amusingly, this seems to have uncovered several problems with the expected values in some test cases, which I have also fixed. These fixes fell into 3 categories:

  • accidental duplication of an # E: or an # N:
  • accidentally used a type ignore after an E (you can tell it's an accident in this case because it type-ignores the very error the E is looking for), which wouldn't have worked in any case because of type: ignore command not recognized after other comment #19366
  • used python comments instead of test comments for disabling a line with an # E :, so now that that syntax with the interstitial space is valid the E triggers. I turned this one into an xfail instead, based on what it was doing.

@wyattscarpenter
Copy link
Contributor Author

wyattscarpenter commented Jul 31, 2025

Ah, there are a lot of urls with fragments in there. Hmm.

Maybe the whole parser should be changed and simplified...

Or perhaps just match space and beginning of line.

@wyattscarpenter
Copy link
Contributor Author

Why the hell am I still getting test failures like

 Alignment of first line difference:
  E: tmp/foo.pyi:23: note: def f(self, x: int) -> int
  A: tmp/foo.pyi:23: note:     def f(self, x: int) -> int

@wyattscarpenter
Copy link
Contributor Author

wyattscarpenter commented Jul 31, 2025

I added a .strip to it... it's then constructed with a template string... what the hell...

[edit: commit with .strip not pictured in current branch history]

@wyattscarpenter
Copy link
Contributor Author

Oh...

@wyattscarpenter wyattscarpenter marked this pull request as ready for review July 31, 2025 07:58
@wyattscarpenter wyattscarpenter marked this pull request as draft July 31, 2025 08:09
@wyattscarpenter
Copy link
Contributor Author

Amusingly, this seems to have uncovered several problems with the expected values in some test cases, which I will now fix...

@wyattscarpenter wyattscarpenter marked this pull request as ready for review July 31, 2025 08:51
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.

"# E:" lines (etc) in test files are ignored at the beginning of lines
1 participant