lucidsoftware / xtract

A library to make it easy to deserialize XML to user types in scala
Apache License 2.0
60 stars 20 forks source link

Support Scala 2.13 #23

Closed fmeriaux closed 4 years ago

fmeriaux commented 5 years ago

Hello,

Do you plan to deliver this library on scala 2.13?

Thank you for your answer :)

tmccombs commented 5 years ago

Eventually, yes. However, it looks like cats doesn't have a stable release available on 2.13 yet. Once cats 2.0 is stable, I can move to that and release a 2.13 version.

aappddeevv commented 5 years ago

Is there a 2.13 compatible branch? I tried compiling under 2.13 to publish local but it looks like ParseResult has some errors due to explicit dependence on BuildFrom.

I also had to upgrade sbt, cats (2.0.0-RC1) and specs2.

tmccombs commented 5 years ago

I do not yet have a branch for 2.13

aappddeevv commented 5 years ago

I tried to do a local change (tried both C[_] and just plain C):

  def combine[A, C[_]](results: collection.Iterable[ParseResult[A]])(implicit cb: collection.BuildFrom[results.type, A, C[A]]): ParseResult[C[A]] = {
    val builder = cb.newBuilder(results)
    val errorBuilder = new ListBuffer[ParseError]
    for (res <- results) {
      for (v <- res) builder += v
      errorBuilder ++= res.errors
    }
    val seq = builder.result()
    if (errorBuilder.isEmpty) {
      ParseSuccess(seq)
    } else {
      PartialParseSuccess(seq, errorBuilder.result)
    }
  }

but this fails to compile the test:

scala/xtract/unit-tests/src/test/scala/com/lucidchart/open/xtract/SyntaxSpec.scala:24:24: type mismatch;
[error]  found   : com.lucidchart.open.xtract.ParseResult[Seq[A]]
[error]  required: com.lucidchart.open.xtract.ParseResult[List[A]]
[error]     ParseResult.combine(xml.map(r.read))
aappddeevv commented 5 years ago

I changed cb to a Factory[A,C] (no C[A]) and it seemed to pass the tests and works in my app now. I also had to bump to xml 1.2 so that 2.13 would be available.

fmeriaux commented 4 years ago

Eventually, yes. However, it looks like cats doesn't have a stable release available on 2.13 yet. Once cats 2.0 is stable, I can move to that and release a 2.13 version.

https://github.com/typelevel/cats/releases/tag/v2.0.0 🎉

tmccombs commented 4 years ago

I changed cb to a Factory[A,C] (no C[A]) and it seemed to pass the tests and works in my app now

Hmmm, I'd like to keep support for 2.11 and 2.12. I'm not sure what the best solution for that is.

aappddeevv commented 4 years ago

I think you are supposed to make a source path with 2.13 collections code and perhaps another one for older releases on src/main/scala. Conditionally include that based on the scala version in the build. I've not done this but that's what I see others do. You could make that method inside a trait and then inherit from that trait which is declared identically in both source paths.

tmccombs commented 4 years ago

Thanks I figured out how to do something like that. See the associated PR.