Skip to content

Conversation

Galoretka
Copy link
Contributor

Motivation

The function used take_while with a range predicate starting from the first comment in the file. This stops iteration as soon as a comment before pos_lo is encountered, causing the function to almost always return None when earlier comments exist.
Lists relying on this check (e.g., commasep’s has_comments computation) could mis-detect “no comments between”, affecting line breaking and spacing decisions.

Solution

Replace the logic with skip_while(c.pos() <= pos_lo) + take_while(c.pos() < pos_hi) and find the first non-blank. This matches the intended semantics and aligns with has_comment_between’s filtering approach.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

can you pls add a test with the issue this fixes? Thank you

@0xrusowsky
Copy link
Contributor

looks AI generated, but also a legit bug/patch.
i'll try to come up with a repro myself if they don't answer

@Galoretka
Copy link
Contributor Author

looks AI generated, but also a legit bug/patch. i'll try to come up with a repro myself if they don't answer

I will add tests if you don't mind

@0xrusowsky
Copy link
Contributor

@Galoretka great, thanks!
i think that a unit test where u mock several cmnts will probably be easier to setup than finding an actual solidity repro, but do whatever u prefer

@Galoretka
Copy link
Contributor Author

@Galoretka great, thanks! i think that a unit test where u mock several cmnts will probably be easier to setup than finding an actual solidity repro, but do whatever u prefer

I added a test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants