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

Fix parsing of foo// #359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lread
Copy link
Contributor

@lread lread commented Feb 4, 2025

Fixes #357

@lread
Copy link
Contributor Author

lread commented Feb 4, 2025

Ah. Babashka has not yet upgraded to rewrite-clj 1.1.49.
I'll go raise an issue over there.

@lread
Copy link
Contributor Author

lread commented Feb 5, 2025

Fixed over at bababashka by our wonderful @borkdude, we can wait for the next bb release.

@borkdude
Copy link
Contributor

borkdude commented Feb 5, 2025

If you need a development build:

bash <(curl https://raw.githubusercontent.com/babashka/babashka/master/install)  --dev-build --dir /tmp

@lread
Copy link
Contributor Author

lread commented Feb 5, 2025

Good idea! I confirm that with the dev build of bb, cljfmt's bb test passes on my dev box.

@jeaye
Copy link

jeaye commented Feb 28, 2025

@borkdude @lread Do we have an ETA for when this can be merged? I'd love to get standard formatting in the jank monorepo. Blocked on this.

Edit: The dev build is a tough sell, since every dev working on jank would be expected to have it so we can all format the same.

@lread
Copy link
Contributor Author

lread commented Feb 28, 2025

Just waiting for the next bb release with the rewrite-clj update.
After that, cljfmt bb tests will pass.

@jeaye in the meantime:

  • if you running cljfmt from babashka, you can use the bb dev build (see borkdude's comment above) - oh saw your edit. Ok. Fair enough.
  • if you are running cljfmt from jvm clojure you can just override the rewrite-clj dep with v1.1.49.

@borkdude
Copy link
Contributor

I'll make a new release today, thanks for the reminder

@borkdude
Copy link
Contributor

Released!

@lread
Copy link
Contributor Author

lread commented Feb 28, 2025

Cool. I'll force a rebuild here with an empty commit.

@lread
Copy link
Contributor Author

lread commented Feb 28, 2025

Great! Cljfmt CI now passes with new latest babashka.
@weavejester, pending your review, this could be merged.
(do you want me to add a changelog entry?)

@lread
Copy link
Contributor Author

lread commented Feb 28, 2025

@jeaye so long as you are using bb latest, things should work even without this PR:

$ bb -Sdeps '{:deps {dev.weavejester/cljfmt {:mvn/version "0.13.0"}}}'
Babashka v1.12.197 REPL.
Use :repl/quit or :repl/exit to quit the REPL.
Clojure rocks, Bash reaches.

user=> (require '[cljfmt.core :as cljfmt])
nil
user=> (cljfmt/reformat-string "foo//")
"foo//"

@jeaye
Copy link

jeaye commented Feb 28, 2025

Nice! 🎉 Thank you!

@weavejester
Copy link
Owner

Thanks for the commits. Can you remove the empty one?

@lread lread force-pushed the lread-upgrade-rewrite-clj branch from 5778eee to cfe699d Compare March 9, 2025 21:31
@lread
Copy link
Contributor Author

lread commented Mar 9, 2025

Yep, squashed!

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.

Support parsing foo//
4 participants