valentiay / phobos

Efficient and expressive XML data-binding library for Scala
Apache License 2.0
20 stars 5 forks source link

#3 ignore namespaces #5

Closed iyfedorov closed 7 months ago

iyfedorov commented 7 months ago

Hello! I didn't fully understand quasiquotes but can you look at it?

iyfedorov commented 7 months ago

Hi! Thanks for your contribution!

I like the idea of modifying Cursor. It's good in its simplicity. The solution I was thinking of was much more complicated.

I've left some minor comments in the code, but there's also one major thing to consider. Look at this code sample:

import phobos.decoding.ElementDecoder
import phobos.decoding.XmlDecoder
import phobos.derivation.semiauto.deriveXmlDecoderConfigured
import phobos.configured.ElementCodecConfig
import phobos.derivation.semiauto.deriveElementDecoder
import phobos.syntax.xmlns

case object com
implicit val comNamespace: Namespace[com.type] = Namespace.mkInstance("example.com")

final case class Bar(@xmlns(com) qux: String)
// Default config with ignoreNamespaces=false is used
implicit val barDecoder: ElementDecoder[Bar] = deriveElementDecoder

val fooConfig = ElementCodecConfig.default.withIgnoreNamespaces()
final case class Foo(baz: String, @xmlns(com) bar: Bar)
implicit val fooDecoder: XmlDecoder[Foo] = deriveXmlDecoderConfigured("foo", fooConfig)

val string =
  """<com:foo xmlns:com="example.com">
    |  <com:baz>test2</com:baz><!-- Namespace is reset here -->
    |  <com:bar><com:qux>test1</com:qux></com:bar> <!-- But namespace is not reset here -->
    |</com:foo>
    |""".stripMargin

println(fooDecoder.decode(string))
// Right(Foo(test2,Bar(test1)))

Here, "ignoreNamespaces" applies to "baz" (because it uses a primitive string decoder), but not to "bar" (because it uses derived decoder with default configuration). This does not cause any bugs, but it is definitely counter-intuitive and can also be inconvenient.

I think that "ignoreNamespaces" should work like "scopeDefaultNamespace", which affects all elements within the current one. If "ignoreNamespaces" is made an optional setting, then it will be possible to implement this behavior with a stack within "Cursor", just as for "scopeDefaultNamespace". This way the behaviour would not only be more intuitive but you would also not have to set "ignoreNamespace" for every derived decoder.

It`s true. Propagation strategy will be more convenient. I will try to add this