Remove printers abstractions from library #10162
Conversation
nicolasstucki
force-pushed
the
make-tasty-printer-work-with-quote-context
branch
3 times, most recently
from
4449a1a to
0403880
Compare
nicolasstucki
force-pushed
the
make-tasty-printer-work-with-quote-context
branch
2 times, most recently
from
41c514f to
195a996
Compare
| package scala.tasty | ||
| package reflect | ||
| package scala.quoted | ||
| package reflection.printers |
There was a problem hiding this comment.
reflection -> reflect?
If we have both names, it's easy to get confused and make mistakes. It would be nice to just use the same name through out.
There was a problem hiding this comment.
They have a different name to avoid confusion. I tried that but noticed that using reflect is more confusing because you would have qctx.reflect and scala.quoted.reflect in the same scope. It also clashes with scala.reflect which is annoying. scala.quoted.reflection is the one that will be used the less often hence the longer name.
There was a problem hiding this comment.
If it has to be different, what about choosing a name that does not contain reflect*?
There was a problem hiding this comment.
What name could be used? It should be clear that it is about reflection.
There was a problem hiding this comment.
I also wanted to distance the name from tasty to avoid confusion with the file format
There was a problem hiding this comment.
Can it be just scala.quoted.printers?
There was a problem hiding this comment.
We will also need to move scala.tasty.Reflection and scala.tasty.reflect.* into this package.
scala.quoted
- Expr, Type, ...
- reflection (needs better name)
- Reflection (or rename to Reflect to align with qctx.reflect)
- printers
- SourceCode, Extractors and SyntaxHighlighting
- reflect (needs new name)
- TreeMap, TreeAcumulator, TreeTraverser
There was a problem hiding this comment.
Yes, naming is always hard. What about scala.quoted.trees or scala.quoted.ast?
There was a problem hiding this comment.
I like ast, but now I realize that we may no even need to keep this logic in the library.
There was a problem hiding this comment.
Now I moved to printers out of the library. We never used them in the library anyway, only in the implementation of the QuoteContext. This will also allow us to rewrite them using the compiler trees directly for efficiency if we need it in the future.
nicolasstucki
force-pushed
the
make-tasty-printer-work-with-quote-context
branch
from
195a996 to
0805d53
Compare
|
Rebased |
nicolasstucki
force-pushed
the
make-tasty-printer-work-with-quote-context
branch
from
ca90ff7 to
96bd7d0
Compare
nicolasstucki
changed the title
| if ctx.settings.color.value == "always" then | ||
| qctx.reflect.TreeMethodsImpl.extension_showAnsiColored(tree) | ||
| else | ||
| qctx.reflect.TreeMethodsImpl.extension_show(tree) | ||
| } |
There was a problem hiding this comment.
Does the name change showTree -> showDecompiledTree suggest that the method only works for decompiled trees?
There was a problem hiding this comment.
This method is only used internally for decompilation. I renamed it to make it clear what it should do to avoid using it somewhere else afterward and changing something inside to adapt to the new use. It also differentiates it from the other general-purpose showTree methods.
This way we avoid ambigouities been `qctx.reflect` and `scala.reflect`.
nicolasstucki
force-pushed
the
make-tasty-printer-work-with-quote-context
branch
from
65b8689 to
da8f531
Compare
|
Rebased |
SourceCodePrinter,ExtractorPrinter,SyntaxHighlight)show(SyntaxHighlighting)withshowAnsiColored(kept normal B/W show as is)