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

Fix 2.13 collections methods for NodeSeq #396

Merged
merged 3 commits into from
Jan 31, 2020
Merged

Conversation

ashawley
Copy link
Member

@fanf
Copy link

fanf commented Jan 22, 2020

With that patch, I get at some places that overloading error:

Error:(730, 30) overloaded method value flatMap with alternatives:
  (f: scala.xml.Node => scala.collection.IterableOnce[scala.xml.Node])scala.xml.NodeSeq <and>
  [B](f: scala.xml.Node => scala.collection.IterableOnce[B])Seq[B]
 cannot be applied to (scala.xml.Node => net.liftweb.common.Box[com.normation.rudder.services.eventlog.RollbackedEvent])
      entry     <- eventlogs \ "rollbackedEvent"

Where eventlogs has type Node.
It happens only in the following kind of code:

  for{
      event     <- xml
      eventlogs <- event.child
      entry     <- eventlogs \ "rollbackedEvent"
      id        <- (entry \ "id").headOption.map(_.text.toInt) ?~! ("error: id can't be null")
    } yield {
      id
    }

Where ?~! is a method on liftweb Box type, and certainly there is implicit implied of the problem, but I don't understand what/how... But actually the code is not homogenous, and I'm not sure it should have compiled before - at least, it doesn't do what it's documented to do.
And it's easily solved by changing the line like:

      entry     <- (eventlogs \ "rollbackedEvent").iterator // or .headOption

In all case, the patch solve the MUCH bigger problem of NodeSeq vs Seq[Node], and it's a relief for us.

Summary: LGTM

Thanks!

@ashawley
Copy link
Member Author

Thanks for trying it out and for this feedback.

In truth, there may not be a lot of tests around for-comprehensions or flatMap in the test suite. Even if your issue was unique to liftweb, these changes probably warrant further study.

@fanf
Copy link

fanf commented Jan 27, 2020

Could we have at least a SNAPSHOT release (sorry if I missed them if they exist)? We realy depend on that fix to have a "non totally broken" app.
If it's impossible to get a nightly, I will need to localy publish a scala-xml version with the patch.

@ashawley ashawley added this to the 1.3.0 milestone Jan 31, 2020
@ashawley ashawley merged commit 306fe58 into scala:1.x Jan 31, 2020
@ashawley ashawley deleted the issue-392 branch January 31, 2020 13:06
@SethTisue
Copy link
Member

@fanf a release attempt is underway at #401

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