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

Xml Interpolation #151

Closed
wants to merge 2 commits into from
Closed

Conversation

allanrenucci
Copy link

This PR contains an implementation of a macro based XML interpolator that we developed at EPFL that aims to be a drop in replacement for XML literals.

Additionally to the test suite, we tested it against The Lift Web Framework. We rewrote XML literals occurrences in the projects using Scalafix and successfully ran the test suite.

We decided not to support XML interpolation in pattern position as we don't know how to rewrite:

case <a>{ns @ _*}</a> =>

I was not sure how to integrate it to this repository so I split the build into two subprojects: scala-xml and scala-xml-quote.

cc/ @densh @adriaanm

@ashawley
Copy link
Member

This PR contains an implementation of a macro based XML interpolator that we developed at EPFL that aims to be a drop in replacement for XML literals.

Exciting stuff!

I was not sure how to integrate it to this repository so I split the build into two subprojects: scala-xml and scala-xml-quote.

Is it a subproject so that it is published as a separate library? If so, is it a new library because it has new transitive depndencies (scalafmt, fastparse)? Could it just be included in scala-xml?

Do you expect it will target other platforms, Scalajs or native, or is it just for the JVM?

@allanrenucci
Copy link
Author

Hi @ashawley,

Is it a subproject so that it is published as a separate library? If so, is it a new library because it has new transitive depndencies (scalafmt, fastparse)? Could it just be included in scala-xml?

It is indeed a new library because it has a dependency on fastparse. I can include it to scala-xml if you think it is OK to add fastparse as a dependency to scala-xml.

Do you expect it will target other platforms, Scalajs or native, or is it just for the JVM?

It’s just a macro, it doesn’t have runtime code, then we don’t have to cross-publish.

@SethTisue
Copy link
Member

I can include it to scala-xml if you think it is OK to add fastparse as a dependency to scala-xml

because of the circular dependency between scala-xml and the Scaladoc tool, scala-xml is built during the Scala bootstrap process, so we need to be careful about not breaking that.

(although it would be better, and not that hard, to get rid of the circular dependency — it's just a piece of work that nobody has gotten around to doing)

@allanrenucci
Copy link
Author

scala-xml-quote cannot build with 2.13 (because of its dependency). I suppose, it is a requirement that scala-xml build with 2.13. A separate library seems to be the way to go then

@ashawley
Copy link
Member

2.13 is a preview release, so it's just an attempt to support compiler development. This is a consequence of the circular dependency Seth mentioned. Not being able to build in 2.13 for the moment is not a deal breaker, IMO.

@ashawley
Copy link
Member

Would you be willing to create an example xmlquote project (or projects) that depends on this new library and have it work on the various runtimes (JVM, native, JS) assuming it could. I know it would require sbt publishLocal from this repo on a version 0.1-SNAPSHOT, but that's ok.

I've been meaning to review this by doing the above, but I just haven't gotten around to it.

@SethTisue
Copy link
Member

although it would be better, and not that hard, to get rid of the circular dependency — it's just a piece of work that nobody has gotten around to doing

@adriaanm is now working on this

@SethTisue
Copy link
Member

the circular dependency went away in Scala 2.13

but idk, maybe this repo should stay focused on what it already does, and this new thing should live elsewhere?

@SethTisue
Copy link
Member

maybe this repo should stay focused on what it already does, and this new thing should live elsewhere?

and, that seems like where the discussion stopped. so, closing

@SethTisue SethTisue closed this Dec 5, 2020
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.

None yet

3 participants