Skip to content

Quote/splice formatting fixes for attributes & comments#137

Merged
jbachurski merged 5 commits intojanefrom
jbachurski/further-quote-splice-fixes
Apr 23, 2026
Merged

Quote/splice formatting fixes for attributes & comments#137
jbachurski merged 5 commits intojanefrom
jbachurski/further-quote-splice-fixes

Conversation

@jbachurski
Copy link
Copy Markdown

@jbachurski jbachurski commented Apr 16, 2026

  • Stops double parenthesising of splices of non-atomic expressions with attributes, e.g. $((int list[@attr])).
  • Adds a bunch of tests for quotes/splices in types, particularly for types with attributes and comments. Quotes/splices in expressions already seem robust but I added a test too.

I briefly parenthesised splices of atomic expressions with pre-fix/post-fix comments (e.g. $((* pre *) int)/$(int (* post *))), but this is actually inconsistent with prefix operators, which should probably format the same as splices in these cases. I added a test showing that.

@jbachurski jbachurski force-pushed the jbachurski/further-quote-splice-fixes branch from 11aef01 to 8786254 Compare April 16, 2026 12:17
@jbachurski jbachurski force-pushed the jbachurski/further-quote-splice-fixes branch from 8786254 to 9032c83 Compare April 16, 2026 14:51
@jbachurski jbachurski requested a review from dvulakh April 20, 2026 13:41
dvulakh added 3 commits April 21, 2026 10:31
also use the standard parenze method for determining whether to
parenthesize and fix the double-parenthisization bug more broadly

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
even though they are syntactically redundant, their absence is often
visually confusing, and this pr is not the place to find a general fix

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Copy link
Copy Markdown
Author

@jbachurski jbachurski left a comment

Choose a reason for hiding this comment

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

@dvulakh's changes look good -- thanks!

@jbachurski jbachurski merged commit 0d0379e into jane Apr 23, 2026
2 of 3 checks passed
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