Skip to content

Rollback xml parsing work-around#1765

Merged
eed3si9n merged 1 commit intosbt:0.13from
ajozwik:0.13
Dec 12, 2014
Merged

Rollback xml parsing work-around#1765
eed3si9n merged 1 commit intosbt:0.13from
ajozwik:0.13

Conversation

@ajozwik
Copy link
Contributor

@ajozwik ajozwik commented Dec 5, 2014

Fixes #1738
scala/scala#4186 solves bug in scala compiler.

@eed3si9n eed3si9n added the ready label Dec 5, 2014
@typesafe-tools
Copy link

Can one of the admins verify this patch?

@eed3si9n
Copy link
Member

eed3si9n commented Dec 6, 2014

Given SI-9027, I think it's better to roll back #1671 and ask the user to use semicolon or xml:group rather than rolling out our own lexers.

scala> val xs: xml.NodeSeq = <xml:group><x/><y/></xml:group>
xs: scala.xml.NodeSeq = <x/><y/>

@ajozwik
Copy link
Contributor Author

ajozwik commented Dec 6, 2014

scala> :pa
// Entering paste mode (ctrl-D to finish)

val g = <xml:group><a/><b/></xml:group>
val b = <a/><b/>;
val c = (<a/><b/>)
val a = <a/><b/>


// Exiting paste mode, now interpreting.

g: scala.xml.Group = <a/><b/>
b: scala.xml.NodeBuffer = ArrayBuffer(<a/>, <b/>)
c: scala.xml.NodeBuffer = ArrayBuffer(<a/>, <b/>)
a: scala.xml.NodeBuffer = ArrayBuffer(<a/>, <b/>)


scala> a == b
res4: Boolean = true

scala> a == c
res5: Boolean = true

scala> a == g
res6: Boolean = false

Group is not the same as NodeBuffer.

Second problem is that how to detect xml compilation problem?

See

scala> :pa
// Entering paste mode (ctrl-D to finish)

val b = <a/><b/>
val s = 3

// Exiting paste mode, now interpreting.

<console>:2: error: ';' expected but 'val' found.
val s = 3
^

So the problem will be how to detect, that compilation problem is xml group compilation error?

@ajozwik
Copy link
Contributor Author

ajozwik commented Dec 8, 2014

@jsuereth @eed3si9n - we need use 2.10.4 in current sbt by at least 4 months. See scala/scala#4186 (comment)

@eed3si9n
Copy link
Member

eed3si9n commented Dec 8, 2014

My preference is to tell user to put semicolon or parens when they see error: ';' expected but 'val' found. rather than trying to implement our own lexer. As #1738 has shown, workarounds can cause more issues. Even though it is several months away, a definitive fix is coming up anyway.

As per NodeBuffer vs Group, if they behave the same semantically as NodeSeq, I am ok with them not being equal, but just to be safe we can recommend semicolon or parens instead of xml:group.

@ajozwik
Copy link
Contributor Author

ajozwik commented Dec 8, 2014

OK, so I have to change some tests as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test will pass even when scala parser will be corrected. The second statement val s = ' is incorrect

@ajozwik ajozwik changed the title Do not modify xml in string literals. Rollback xml parsing work-around Dec 10, 2014
eed3si9n added a commit that referenced this pull request Dec 12, 2014
Rollback xml parsing work-around
@eed3si9n eed3si9n merged commit 9fc174f into sbt:0.13 Dec 12, 2014
@eed3si9n eed3si9n removed the ready label Dec 12, 2014
@eed3si9n eed3si9n added this to the 0.13.8 milestone Dec 12, 2014
adpi2 pushed a commit to adpi2/sbt that referenced this pull request Oct 9, 2024
Rollback xml parsing work-around
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.

3 participants