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.

Sign up for GitHub

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

Add scala-xml module #664

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add scala-xml module #664

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d3d21c
Add scalaxml module
bplommer Jun 10, 2022
525f6b6
update github workflow
bplommer Jun 13, 2022
84f56a0
make xml codec test js-only
bplommer Jun 13, 2022
6c593e6
Make scala-xml jvm only
bplommer Jun 14, 2022
89426bb
Fix mima
bplommer Jun 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ jobs:

- name: Make target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: mkdir -p modules/circe/.jvm/target target .js/target modules/docs/target modules/core/js/target modules/circe/.js/target modules/core/jvm/target modules/tests/js/target .jvm/target .native/target modules/refined/.js/target modules/refined/.jvm/target modules/tests/jvm/target modules/example/target project/target
run: mkdir -p modules/scalaxml/.jvm/target modules/circe/.jvm/target target .js/target modules/docs/target modules/scalaxml/.js/target modules/core/js/target modules/circe/.js/target modules/core/jvm/target modules/tests/js/target .jvm/target .native/target modules/refined/.js/target modules/refined/.jvm/target modules/tests/jvm/target modules/example/target project/target

- name: Compress target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: tar cf targets.tar modules/circe/.jvm/target target .js/target modules/docs/target modules/core/js/target modules/circe/.js/target modules/core/jvm/target modules/tests/js/target .jvm/target .native/target modules/refined/.js/target modules/refined/.jvm/target modules/tests/jvm/target modules/example/target project/target
run: tar cf targets.tar modules/scalaxml/.jvm/target modules/circe/.jvm/target target .js/target modules/docs/target modules/scalaxml/.js/target modules/core/js/target modules/circe/.js/target modules/core/jvm/target modules/tests/js/target .jvm/target .native/target modules/refined/.js/target modules/refined/.jvm/target modules/tests/jvm/target modules/example/target project/target

- name: Upload target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
Expand Down
17 changes: 15 additions & 2 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ lazy val commonSettings = Seq(

lazy val skunk = tlCrossRootProject
.settings(name := "skunk")
.aggregate(core, tests, circe, refined, example)
.aggregate(core, tests, circe, refined, example, `scala-xml`)
.settings(commonSettings)

lazy val core = crossProject(JVMPlatform, JSPlatform)
Expand Down Expand Up @@ -198,10 +198,23 @@ lazy val circe = crossProject(JVMPlatform, JSPlatform)
)
)

lazy val `scala-xml` = crossProject(JVMPlatform, JSPlatform)
Copy link
Member

Choose a reason for hiding this comment

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

Probably easiest to make it JVM only for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That requires rearranging the tests too, since they're cross-built for js and jvm. I've tried moving the test into jvm-only sources, let's see what happens.

Copy link
Member

Choose a reason for hiding this comment

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

Right, nothing wrong with that. Still, better not to publish for JS unless you have a cross-platform parser. scala-xml.js is kind of a lie at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we'd then need to also exclude the js version from mima? Might be better to reorganize the modules instead to do this properly

Copy link
Member

@armanbilge armanbilge Jun 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I mean, it's pretty simple. Instead of using crossProject, just make an ordinary project :)

The tricky part is for the tests, you need to do a .jvmConfigure(_.dependsOn(`scala-xml`))

.crossType(CrossType.Pure)
.in(file("modules/scalaxml"))
.dependsOn(core)
.enablePlugins(AutomateHeaderPlugin)
.settings(commonSettings)
.settings(
name := "skunk-scala-xml",
libraryDependencies ++= Seq(
"org.scala-lang.modules" %%% "scala-xml" % "2.1.0"
)
)

lazy val tests = crossProject(JVMPlatform, JSPlatform)
.crossType(CrossType.Full)
.in(file("modules/tests"))
.dependsOn(core, circe)
.dependsOn(core, circe, `scala-xml`)
.enablePlugins(AutomateHeaderPlugin, NoPublishPlugin)
.settings(commonSettings)
.settings(
Expand Down
20 changes: 20 additions & 0 deletions modules/scalaxml/src/main/scala/XmlCodecs.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) 2018-2021 by Rob Norris
// This software is licensed under the MIT License (MIT).
// For more information see LICENSE or https://opensource.org/licenses/MIT

package skunk
package scalaxml.codec

import cats.syntax.all._

import scala.xml._
import skunk.data.{Arr, Type}

trait XmlCodecs {
val xml: Codec[Elem] = Codec.simple(_.toString, s => Either.catchNonFatal(XML.loadString(s)).leftMap(_.getMessage), Type.xml)
val _xml: Codec[Arr[Elem]] = Codec.array(_.toString, s => Either.catchNonFatal(XML.loadString(s)).leftMap(_.getMessage), Type._xml)
Comment on lines +14 to +15
Copy link
Member

Choose a reason for hiding this comment

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

If we use fs2-data-xml for parsing then it can cross-compile for JS/Native.

}

object xml extends XmlCodecs

object all extends XmlCodecs
21 changes: 21 additions & 0 deletions modules/tests/shared/src/test/scala/codec/XmlCodecTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) 2018-2021 by Rob Norris
// This software is licensed under the MIT License (MIT).
// For more information see LICENSE or https://opensource.org/licenses/MIT

package tests
package codec

import cats.Eq
import skunk.scalaxml.codec.all._

import scala.xml.{Elem, XML}

class XmlCodecTest extends CodecTest {
implicit val eq: Eq[Elem] = Eq.fromUniversalEquals

val x: Elem = XML.loadString("<foo>bar</foo>")

roundtripTest(xml)(x)
}