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

Format null-aware elements #1663

Merged
merged 2 commits into from
Mar 5, 2025
Merged

Format null-aware elements #1663

merged 2 commits into from
Mar 5, 2025

Conversation

chloestefantsova
Copy link
Contributor

chloestefantsova and others added 2 commits March 3, 2025 14:36
- Move the tests to tall. The old short style formatter doesn't support
  new syntax and doesn't need to be tested.

- Add support for passing experiment flags with tests. The old way of
  doing this was to just hardcode experiments inside the formatter when
  a new feature was supported. That made sense when the formatter didn't
  know about language version, but now that it does, it should probably
  require experiment flags explicitly and tests for new features should
  enable them.

- Move the code to handle "? into visitMapLiteralEntry(). The AST node
  does have tokens for "?" and that way there's less special-case code
  getting threaded through PieceFactory.

- Add some tests for comments appearing around "?". (Comments tend to be
  a tricky corner of the formatter, so we test them heavily.)
@munificent
Copy link
Member

Thanks for getting this working! I made a handful of changes and pushed a new commit to this PR. I hope that's OK.

Let me know what you think and if you're OK with my changes, I'll merge it.

@munificent munificent requested a review from natebosch March 5, 2025 00:30
@chloestefantsova
Copy link
Contributor Author

Thanks for getting this working! I made a handful of changes and pushed a new commit to this PR. I hope that's OK.

Let me know what you think and if you're OK with my changes, I'll merge it.

Thanks for taking a look and updating the PR! The new commit LGTM.

@munificent munificent merged commit 348a146 into main Mar 5, 2025
5 checks passed
@munificent munificent deleted the format_null_aware_elements branch March 5, 2025 15:28
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.

3 participants