ksvg icon indicating copy to clipboard operation
ksvg copied to clipboard

Request for multiplatform

Open NikkyAI opened this issue 5 years ago • 12 comments

Is it possible to package this for multiplatform?

for my own project i just copied your code into my commonMain

i use this with kotlin/js and react, but having it in common code leaves the option open to render stuff serverside too

seems like Chat.isUpperCase() does not exist on js so i solved it expect/actual, iirc this is the only problem i ran into

//common
expect val Char.isUpperCase: Boolean

// js
actual val Char.isUpperCase: Boolean
    get() = (this == this.toUpperCase() && this != this.toLowerCase())

// jvm
actual val Char.isUpperCase: Boolean
    get() = isUpperCase()

NikkyAI avatar Jan 12 '20 09:01 NikkyAI

I've thought this would be a good thing to do. I've not done any multi platform yet. Will look at it.

On Sun, Jan 12, 2020 at 4:01 AM NikkyAI [email protected] wrote:

Is it possible to package this for multiplatform?

for my own project i just copied your code into my commonMain

i use this with kotlin/js and react, but having it in common code leaves the option open to render stuff serverside too

seems like Chat.isUpperCase() does not exist on js so i solved it expect/actual, iirc this is the only problem i ran into

//common expect val Char.isUpperCase: Boolean

// js actual val Char.isUpperCase: Boolean get() = (this == this.toUpperCase() && this != this.toLowerCase())

// jvm actual val Char.isUpperCase: Boolean get() = isUpperCase()

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nwillc/ksvg/issues/3?email_source=notifications&email_token=AA3VMXM7WEPCA6Q52JNPIHLQ5LL5VA5CNFSM4KFWQ7Q2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IFSCURA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3VMXJNNMPTSXVNUF3EYMLQ5LL5VANCNFSM4KFWQ7QQ .

nwillc avatar Jan 12 '20 17:01 nwillc

I assume the logging support will need fixing too.

nwillc avatar Jan 13 '20 02:01 nwillc

i would suggest kotlin-logging, not that there is much to replace.. have a look at my port: https://github.com/NikkyAI/pentagame/tree/a0311c68ade0dc768a539c6e8da6a8901682aa92/shared/src/commonMain/kotlin/com/github/nwillc/ksvg and the Character.isUpperCase code here https://github.com/NikkyAI/pentagame/blob/ca4215cdade3e9f8b3be5519365620952dd4c9bd/shared/src/jsMain/kotlin/isUpperCase.kt

NikkyAI avatar Jan 13 '20 06:01 NikkyAI

I've undertaken making the project multi-platform. Won't happen instantly, and happy for any help, but I am working on it in my free time.

On Mon, Jan 13, 2020 at 1:30 AM NikkyAI [email protected] wrote:

i would suggest kotlin-logging, not that there is much to replace.. have a look at my port: https://github.com/NikkyAI/pentagame/tree/a0311c68ade0dc768a539c6e8da6a8901682aa92/shared/src/commonMain/kotlin/com/github/nwillc/ksvg and the Character.isUpperCase code here https://github.com/NikkyAI/pentagame/blob/ca4215cdade3e9f8b3be5519365620952dd4c9bd/shared/src/jsMain/kotlin/isUpperCase.kt

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nwillc/ksvg/issues/3?email_source=notifications&email_token=AA3VMXID7NS5DGFOQEE5GDDQ5QDAPA5CNFSM4KFWQ7Q2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIXU67Y#issuecomment-573525887, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3VMXKHANONDBAOMLRG72DQ5QDAPANCNFSM4KFWQ7QQ .

nwillc avatar Jan 14 '20 02:01 nwillc

I may have a working version. Would you be able to look at the feature/multi-platform branch to review the JS work? I don't have expertise to evaluate the JS bits.

On Mon, Jan 13, 2020 at 9:42 PM [email protected] wrote:

I've undertaken making the project multi-platform. Won't happen instantly, and happy for any help, but I am working on it in my free time.

On Mon, Jan 13, 2020 at 1:30 AM NikkyAI [email protected] wrote:

i would suggest kotlin-logging, not that there is much to replace.. have a look at my port: https://github.com/NikkyAI/pentagame/tree/a0311c68ade0dc768a539c6e8da6a8901682aa92/shared/src/commonMain/kotlin/com/github/nwillc/ksvg and the Character.isUpperCase code here https://github.com/NikkyAI/pentagame/blob/ca4215cdade3e9f8b3be5519365620952dd4c9bd/shared/src/jsMain/kotlin/isUpperCase.kt

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nwillc/ksvg/issues/3?email_source=notifications&email_token=AA3VMXID7NS5DGFOQEE5GDDQ5QDAPA5CNFSM4KFWQ7Q2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIXU67Y#issuecomment-573525887, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3VMXKHANONDBAOMLRG72DQ5QDAPANCNFSM4KFWQ7QQ .

nwillc avatar Jan 30 '20 03:01 nwillc

i assume with js specific code you mean the logging?

it looks like it will work but i think it will not show the original file location / line numbers when using the browser dev tools for that iirc the logger calls need to all be inline or so, never looked into it in detail korlibs/klogger works for me, but it has no decent formatting

also 1.3.70-eap had some mindblowing improvements to the js side of things although it is fine for this, since you are depending on barely anything

through experimentation i found out that this or something similar is required in the buildscript

js() {
        useCommonJs()
        browser {
            runTask {
                sourceMaps = true
            }
            webpackTask {
                sourceMaps = true
            }
        }
        compilations.all {
            kotlinOptions {
                sourceMap = true
                metaInfo = true
                main = "call"
            }
        }
        mavenPublication { // Setup the publication for the target
//            artifactId = "ksvg-js"
            // Add a docs JAR artifact (it should be a custom task):
//            artifact(javadocJar)
        }
    }

without it the project using it will complain that ksvg is not configured for js this error only nees to pop up when it is included as subproject, but after fixing it using it as a library from mavenLocal seems to work too

NikkyAI avatar Jan 31 '20 18:01 NikkyAI

I've accepted your PR. It's not the logging I'm worried about. I don't have a JS test harness to try things with and am struggling getting JS unit testing going.

nwillc avatar Feb 01 '20 01:02 nwillc

i can look into that

also have similar problems of not knowing how to run tests in the browser

NikkyAI avatar Feb 01 '20 06:02 NikkyAI

seems like this is breaking things.. https://stackoverflow.com/questions/51475050/spaces-in-test-method-names-when-targeting-multiplatform

TL;DR: cannot have spaces in testnames in commonTest because it cannot compile to js

NikkyAI avatar Feb 01 '20 06:02 NikkyAI

after fixing all the testnames with @JsName

these errors are left over

here jvm and js seem to sort x1, x2, y1, y2 differently https://github.com/nwillc/ksvg/blob/72402ec00c1f4c33ec4f2d5f01335ef12000d207/src/commonTest/kotlin/com/github/nwillc/ksvg/elements/LINETest.kt#L48

IllegalArgumentException: Value (10) is not a valid length or percentage https://github.com/nwillc/ksvg/blob/72402ec00c1f4c33ec4f2d5f01335ef12000d207/src/commonTest/kotlin/com/github/nwillc/ksvg/elements/RECTTest.kt#L43

IllegalArgumentException: Value (M 10,30 A 20,20 0,0,1 50,30 Z) is not a valid path https://github.com/nwillc/ksvg/blob/72402ec00c1f4c33ec4f2d5f01335ef12000d207/src/commonTest/kotlin/com/github/nwillc/ksvg/elements/PATHTest.kt#L28

in addition to that there seem to be a handful failures that look like similar regex or sorting failures https://github.com/nwillc/ksvg/blob/c511c26534111ea9882eefea4256194ef12b7f0e/src/commonTest/kotlin/com/github/nwillc/ksvg/attributes/AttributeTypeTest.kt

for the last few errors it seems like the regex might be acting up, i will open a PR again for the current progress

NikkyAI avatar Feb 01 '20 07:02 NikkyAI

The issues in the tests I believe are resolved. The sort order is now enforced and the other bits and pieces seem resolved. The JVM build artifacts now work with existing clients so I've merged the branch to master. I think the javascript is working but I still don't understand the packaging and how it would be used in an example. More to do there.

nwillc avatar Feb 08 '20 06:02 nwillc

Please see #10 for instructions on how to use this in a multiplatform project

harryjph avatar Jan 12 '22 18:01 harryjph