Skip to content

Support Clojure 1.12 array class tokens #290

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 2 commits into from
Jun 29, 2024
Merged

Conversation

lread
Copy link
Collaborator

@lread lread commented Jun 29, 2024

The new foo/3 was reader-illegal prior to 1.12.

Bring in and customize some tools.reader bits and bobs to support reading this new syntax.

Clojure's symbol allows us to construct technically illegal symbols, we take advantage of this to test and support this new syntax in any version of Clojure via, for example, (symbol "foo" "3").

Closes #279

The new `foo/3` was reader-illegal prior to 1.12.

Bring in and customize some tools.reader bits and bobs to support
reading this new syntax.

Clojure's `symbol` allows us to construct technically illegal symbols,
we take advantage of this to test and support this new syntax in any
version of Clojure via, for example, `(symbol "foo" "3")`.

Closes #279
@lread
Copy link
Collaborator Author

lread commented Jun 29, 2024

@borkdude, when you have some time/interest, I think this PR's approach merits a review.

Recap of options from #279 (with some additions). Support new syntax in rewrite-clj by:

  1. Waiting for and relying upon an upcoming clojure.tools.reader release that reads this new syntax
  2. [this PR] Bringing in necessary support to rewrite-clj itself
  3. Entirely inlining clojure.tools.reader and adapting as necessary (this is the approach you took for clj-kondo). This could have other benefits and would make relaxing parsing rules easier.
  4. Switching from clojure.tools.reader to edamame. You were half-joking, but it might be an interesting experiment someday. Because you control edamame, it could be adapted/extended however we see fit.

@borkdude
Copy link
Collaborator

I'm certainly open to 4 since I have to support the symbol with numeric anyway too. In edamame I also rely on tools.reader btw but only on tools.reader.edn, like rewrite-clj (which was my inspiration initially).

@lread
Copy link
Collaborator Author

lread commented Jun 29, 2024

I had not looked at edamame source code in a while. I kinda like that it is not shy about using clojure.tools.reader impl namespaces. I will keep that in mind for future changes.

I like the idea of exploring using edamame. It would be a bigger change with a (potentially) bigger impact on rewrite-clj users. I guess we could explore edamame separately from this issue/PR. It probably deserves a decision matrix.

@lread
Copy link
Collaborator Author

lread commented Jun 29, 2024

Unless you see a problem with this PR @borkdude, I think I'll go ahead and merge it.

@borkdude
Copy link
Collaborator

LGTM!

@lread lread merged commit a973bd1 into main Jun 29, 2024
@lread lread deleted the clojure-1-12-array-class-tokens branch June 29, 2024 19:18
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.

Adapt as necessary to any new Clojure 1.12 syntax/conventions
2 participants