com-lihaoyi / scalatags

ScalaTags is a small XML/HTML construction library for Scala.
https://com-lihaoyi.github.io/scalatags/
MIT License
758 stars 117 forks source link

Include more implicit conversions in implicits #134

Closed julienrf closed 6 years ago

julienrf commented 8 years ago

This change makes more implicit conversions available in the implicits object.

julienrf commented 8 years ago

Don’t know how this relates to #119. Any thought, @stewSquared?

lihaoyi commented 8 years ago

This looks fine to me; could you add a previously-failing unit test that verifies we actually fixed something?

stewSquared commented 8 years ago

Yes! The implicits object definitely needs to extend Cap. Also, don't forget to make the corresponding change in shared/src/main/scala/scalatags/Text.scala.

The problem, as you'll notice when you start fiddling with tests, is that the utility of this import is limited as long as tags, attrs, styles, etc. still extend Cap, because you can only import underscore from one Cap without losing some functionality due to ambiguous implicits. For example, to fix the test, you'll have to change import tags._ to import tags.{p, div, img}

In short, it's a step in the right direction, but you still can't mix and match with implicits the way you might like to.

stewSquared commented 8 years ago

On second thought, I don't think this change should be made until the corresponding change is made to tags (removing Cap/Util). As it stands now, at least you can import underscore from both tags and implicits.

julienrf commented 8 years ago

I think that tags, attrs, styles should not provide implicit defs. (Maybe that’s what your last comment was about?)

lihaoyi commented 8 years ago

I don't actually remember what's going on, but if tags, attrs, styles is providing implicits on import, we should fix that and remove the implicits exported by them. I don't think there's any reason why you'd import things from e.g. tags alone without correspondingly importing scalatags.implicits._ or scalatags.all._

lihaoyi commented 8 years ago

I'm still waiting for tests to appear before merging this