Fix sort_reexports code mangling#2283
Merged
DanielNoord merged 4 commits intoPyCQA:mainfrom Jan 9, 2025
Merged
Conversation
2 tasks
Contributor
Author
|
It seems a bit unfair that DeepSource blocks this PR for cyclomatic complexity while it was already this high :) |
|
👋 This is great, thanks for the fix! Any chance this could be released in any of the next versions? I agree that it's a bit mad to hold this off on the basis of complexity given how complex the original function is. |
Contributor
Author
|
We'd have to find an active maintainer to ping or something, given that there's 50 open pull requests |
Contributor
Author
|
@hugovk @timothycrosley sorry for the noise, I'd like to get some traction on this since it's been pending quite some time. |
Contributor
|
@Helveg I don't have merge rights here, perhaps @staticdev can help. |
DanielNoord
approved these changes
Jan 9, 2025
Member
|
Thanks! Going to merge this :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2193 and fixes #2252,
The logic behind
sort_reexportswas keeping track of a point in theoutput_stream, and seeked to that point before writing the sorted reexports back in, in order to compensate for the first line of this code sorting section being written into the stream already.The original code tried to track the position of the point with every iteration, but since the code is very branched, and contains ~10
.writestatements, this fails, and this approach would never be easily maintained. So I opted to simplify the approach by instead rolling back the write position in the stream by the length of the segment that was written too much.Additionally, when the input code sort section is longer than the sorted output (e.g., when there is a lot of trimmed whitespace), some previously written garbage remained, which I fixed by truncating the stream after writing to it during
sort_reexportoperations.