8000 Fixed unhandled trailing comment in multiline list for OpenSCAD by mkatychev · Pull Request #972 · tweag/topiary · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fixed unhandled trailing comment in multiline list for OpenSCAD #972

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

Merged
merged 9 commits into from
May 12, 2025

Conversation

mkatychev
Copy link
Contributor

Fixed unhandled trailing comment in multiline list for OpenSCAD

Resolves #969

Description

An extraneous trailing comma was previously added to a list if the trailing comma of a list was followed by a comment:

foo = [
  1, // cmt
];

Checklist

Checklist before merging, wherever relevant:

  • CHANGELOG.md updated
  • Documentation (The Topiary Book, README.md, etc.) up-to-date

@mkatychev
Copy link
Contributor Author

Not sure what's causing the tests to fail, will look into it

@mkatychev mkatychev marked this pull request as draft April 14, 2025 04:20
@nbacquey
Copy link
Member

The output of the test is misleadingly verbose, but the actual failure is:

 [2025-04-14T02:14:03Z ERROR topiary] The formatter produced invalid output and
    failed when trying to format twice (idempotence check).

i.e. the rule you wrote produced some idempotency error

@mkatychev

This comment was marked as resolved.

@mkatychev

This comment was marked as resolved.

@mkatychev mkatychev force-pushed the fix/969-wrong-format-list branch from 1ecb4c6 to 4525f8f Compare April 27, 2025 20:06
@mkatychev mkatychev marked this pull request as ready for review April 27, 2025 20:12
@nbacquey
Copy link
Member

There still seem to be idempotency failures in the tests. I'll delay my review until they pass

@mkatychev
Copy link
Contributor Author

@nbacquey should be fixed

Copy link
Member
@Xophmeister Xophmeister left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mkatychev
Copy link
Contributor Author

@Xophmeister heads up, I've probably introduced the same issue in the WIT formatter with (_) prematurely matching a comment in (_) "," (comment)*, this is less likely to come up than in OpenSCAD but we'll need to deal with it at some point.

@nbacquey nbacquey merged commit ae3cf85 into tweag:main May 12, 2025
9 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.

Openscad wrong format for list with ending comments
3 participants
0