Skip to content

Quote dollar signs when not by themself#31

Merged
mgree merged 3 commits into
binpash:masterfrom
BolunThompson:master
Dec 9, 2024
Merged

Quote dollar signs when not by themself#31
mgree merged 3 commits into
binpash:masterfrom
BolunThompson:master

Conversation

@BolunThompson

@BolunThompson BolunThompson commented Nov 28, 2024

Copy link
Copy Markdown
Contributor

Analogous to binpash/shasta#3. There’s the broader question of whether we can remove the (libdash, presumably) pretty printer and instead use the shasta pretty printer, but this copies the change from the shasta printer to libdash.

@mgree

mgree commented Nov 28, 2024

Copy link
Copy Markdown
Collaborator

This looks good, but for some reason CI hasn't run (and I'd like to confirm that all tests pass before merging!). 🤔

@BolunThompson BolunThompson force-pushed the master branch 3 times, most recently from 44db310 to 09fb8cf Compare November 28, 2024 18:28
@angelhof

angelhof commented Dec 2, 2024

Copy link
Copy Markdown
Member

@mgree could it be that the branch is not a libdash branch? I am struggling to understand why tests didn't run here...

@mgree

mgree commented Dec 2, 2024

Copy link
Copy Markdown
Collaborator

Yeah, no idea. I have "Require approval for first-time contributors" turned on but... 🤔 🤔 🤔

If you can run tests locally and they pass, I'm okay to merge and see if things fail on main.

Signed-off-by: Bolun Thompson <bolunthompson@ucla.edu>
Signed-off-by: Bolun Thompson <bolunthompson@ucla.edu>
Signed-off-by: Bolun Thompson <bolunthompson@ucla.edu>
@BolunThompson

Copy link
Copy Markdown
Contributor Author

After making the equivalent change in the ocaml pretty printer, all the tests pass on my machine: https://gist.github.com/BolunThompson/b073fe50dbd3b4995f26bd564b5cc546

@mgree mgree merged commit b7d9236 into binpash:master Dec 9, 2024
mgree pushed a commit that referenced this pull request Apr 29, 2026
Fix up escaping of `$`; revise tests to support.
---------

Signed-off-by: Bolun Thompson <bolunthompson@ucla.edu>
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