[scala-bts] #3573: XML mode Scala escapes should consider implicit conversion to NodeSeq

----------------------------+-----------------------------------------------
Reporter: willisblackburn | Owner: scala-xml_team
Type: enhancement | Status: new
Priority: normal | Component: XML support
Keywords: |
----------------------------+-----------------------------------------------
Changes (by harrah):

* cc: harrah@… (added)

Comment:

Not in the official specification, but according to the unofficial one:

The first translates to
{{{
new UnprefixedAttribute("attr", x, xml.Null)
}}}
while the second translates to
{{{
val buffer = new xml.NodeBuffer
buffer &+ x
new xml.Elem(null, "sometag", xml.Null, scope, buffer)
}}}
!UnprefixedAttribute has overloaded constructors:
{{{
new UnprefixedAttribute(key: String, value: String, next: MetaData)
new UnprefixedAttribute(key: String, value: Seq[Node], next1:
MetaData)
new UnprefixedAttribute(key: String, value: Option[Seq[Node]], next:
MetaData)
}}}
The signature of `&+` is:
{{{
&+(o: Any): NodeBuffer
}}}

So, an attribute value must conform to `String`, `Seq[Node]`, or
`Option[Seq[Node]]`. Element content is allowed to be anything and so
implicits are never considered.

This is also why
{{{

}}}
doesn't work, but this does:
{{{
{1}
}}}

In any case, this is essentially the same issue as #1787 and should
probably be closed as a duplicate.

----------------------------+-----------------------------------------------
Reporter: willisblackburn | Owner: scala-xml_team
Type: enhancement | Status: new
Priority: normal | Component: XML support
Keywords: |
----------------------------+-----------------------------------------------

Comment(by harrah):

Well, perhaps it is different enough to be a separate ticket. With the
attached patch:

{{{
scala> {3}
res1: scala.xml.Elem = 3

scala> {new Xmlable(3)}
res2: scala.xml.Elem = i = 3
}}}

In theory it should be source compatible (test suite passes), but I'm sure
it breaks someone's code somewhere.

#1787 requires a more invasive approach since type parameters aren't
allowed on auxiliary constructors.

----------------------------+-----------------------------------------------
Reporter: willisblackburn | Owner: scala-xml_team
Type: enhancement | Status: new
Priority: normal | Component: XML support
Keywords: |
----------------------------+-----------------------------------------------

Comment(by willisblackburn):

The patch is very elegant! I've learned a couple of things about Scala
just by looking at it. For some reason it never occurred to me that I
could default an implicit parameter.

I'd be surprised if there are too much existing code where (a) some class
X is defined, (b) there's an implicit conversion from X to NodeSeq in
scope, and (c) an expression evaluating to X is used within {}, but (d)
the developer expects the X to be converted to NodeSeq via toString.

One suggestion: If the method is called with an Iterable[T] and there is
an implicit conversion from T to NodeSeq in scope, then IMHO the method
should map the Iterable with the conversion function before passing it to
addAny.

----------------------------+-----------------------------------------------
Reporter: willisblackburn | Owner: scala-xml_team
Type: enhancement | Status: new
Priority: normal | Component: XML support
Keywords: |
----------------------------+-----------------------------------------------

Comment(by harrah):

The signature is the right one, in my opinion. You could add an overload
on Seq[T], but you can't have overloaded methods with defaults and even if
you could, you are restricting yourself to one level of nesting.

Rather, the caller could define an implicit like:
{{{
implicit def views[T](x: Seq[T])(implicit view: T => Seq[Node]): Seq[Node]
=
x flatMap view
}}}

With that you can nest arbitrarily:
{{{
scala> val xs = Seq(3,4) map (new Xmlable(_))
xs: Seq[Xmlable] = List(Xmlable@6d172f8f, Xmlable@d338d3d)

scala {Seq(xs,xs)}
res4: scala.xml.Elem = i = 3i = 4i = 3i = 4
}}}