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

Port macros to inline meta #491

Merged
merged 5 commits into from Oct 5, 2016

Conversation

cb372
Copy link
Contributor

@cb372 cb372 commented Oct 3, 2016

  • Rewrite the "build time" and "git commit" macros as inline/meta
  • Delete the unneeded macros project

Fixes #467

Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! Only minor comments.

import scala.annotation.compileTimeOnly
import scala.meta._

@compileTimeOnly("Gets expanded away")
Copy link
Member

Choose a reason for hiding this comment

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

This message appears when users forget to add the paradise compiler plugin. A clearer error might be:

"@Buildtime not expanded. Have you enabled the scala.meta paradise plugin?"

Same for GitCommit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, so that's what that annotation is for. I'll admit I didn't really understand it.

Fixed.


inline def apply(defn: Any): Any = meta {
defn match {
case defn @ Defn.Val(mods, pats, optType, rhs) if pats.size == 1 =>
Copy link
Member

Choose a reason for hiding this comment

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

I think this might be clearer with a quasiquote, something in this direction:

case q"val $name = ???" => q"val $name = ${System.currentTimeMillis()}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally tried to wrote it with quasiquotes, but I was tripped up by the fact that you have to explicitly type the val name as ${name: Pat.Var.Term} (otherwise it gets typed as Pat, which you can't use when you come to construct the new tree).

Got it working the second time around. I agree that this is more readable.

@cb372 cb372 force-pushed the port-macros-to-inline-meta branch from dd26613 to 1acd9e6 Compare October 5, 2016 07:17
Also changed them from defs to vals (not that it makes much difference
either way)
because scalafmt does not yet support parsing of inline/meta code
@cb372 cb372 force-pushed the port-macros-to-inline-meta branch from 1acd9e6 to e62ce13 Compare October 5, 2016 07:22
@cb372
Copy link
Contributor Author

cb372 commented Oct 5, 2016

@olafurpg Thanks for the review. I've made some fixes and rebased.

@olafurpg
Copy link
Member

olafurpg commented Oct 5, 2016

LGTM 👍

We're still in the very early days of scala.meta macros. In this particular case, macro annotations are a bit awkward because of the = ??? part. Def macros are on the roadmap to fix that :)

Thanks!

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

2 participants