Clojurians Log v2

Permalink to relevant source please

Probably this one? https://github.com/juji-io/editscript/blob/a259a8c8e1346a5b8e400abd2200422fcd6997de/src/editscript/util/pairing.cljc#L74

This guy - sorry I thought I added it https://github.com/juji-io/editscript/blob/a259a8c8e1346a5b8e400abd2200422fcd6997de/src/editscript/util/pairing.cljc#L71

bb has a priority map impl built-in so it could be replaced with that one

Thank you - I'll ask the maintainer

probably it's best to experiment yourself in a local branch first before bothering the maintainers to see if it's going to work out

yeah well on the other end if there is no will to do that I have to drop my hopes (or maintain my own fork) we'll see

thanks you though, I am going to play a bit with it

nextjournal clerk also uses editscript. if it works in bb, then it would also be useful for that

since clerk now also works in bb

I have a branch here which makes at least the code load in bb, but it isn't working yet. https://github.com/borkdude/editscript/tree/bb-support • hashCode on Object: this can be supported in SCI, so something I'm going to add in SCI • Comparable: not sure about this one yet, but we can work around this by making a custom compare function for stuff

$ bb -cp src -e "(require '[editscript.core :as e]) (e/diff [1 2 3] [1 2 4])"
[[[2] :r 4]]

(this is without alterations to bb, surprised that worked)

cc @huahaiy

@richiardiandrea perhaps you can tell more about your use case of editscript in bb

@borkdude I have a piece of code in my service that diffs with editscript and I have to bulk-modify (cause there was a bug) some data ...currently, we do these kinds of data migrations with bb here - so JVM cannot be used.

with the above branch I'm getting this with the core tests:

26 failures, 0 errors.
{:test 11, :pass 43, :fail 26, :error 0, :type :summary}

wow that's cool - let's see what @huahaiy thinks about it and thanks for trying it out

So what's in editscript that bb doesn't support right now? We can probably work around.

@huahaiy With these changes I get the code to load: https://github.com/borkdude/editscript/commit/05c85b3e5b8c91fa7c50d05d5671e826a2b4b1ee The hashCode thing I can fix in bb / SCI itself. The Comparable thing: probably not, but we can make a custom compare function perhaps to work around this... The priority map: we can fall back on clojure.data.priority-map in bb for this.

I can take a better look at this tomorrow

Ok thx, I am open to a PR

I added hashCode on deftype to bb. Guess what. This fixes the remaining issues.

$ bb -cp "$(lein with-profiles +dev classpath)" "(require '[editscript.core :as e]) (e/diff [1 2 3 6] [1 2 4]) (require '[editscript.core-test]) (clojure.test/run-tests 'editscript.core-test)"

Testing editscript.core-test
{:result true, :num-tests 2000, :seed 1768321398287, :time-elapsed-ms 1608, :test-var "a-star-end-2-end-generative-test"}
{:result true, :num-tests 2000, :seed 1768321399896, :time-elapsed-ms 1850, :test-var "combine-edits-generative-test"}
{:result true, :num-tests 2000, :seed 1768321401746, :time-elapsed-ms 1167, :test-var "quick-end-2-end-generative-test"}

Ran 11 tests containing 69 assertions.
0 failures, 0 errors.
{:test 11, :pass 69, :fail 0, :error 0, :type :summary}

at least for the core tests. Apparently Comparable wasn't needed to make those tests work.

I'll try the other tests as well

Really? wow that's cool

I'll definitely play around with this

this is the location https://github.com/juji-io/editscript/blob/a259a8c8e1346a5b8e400abd2200422fcd6997de/src/editscript/diff/a_star.cljc#L45 it is actually working against hashCode but java.lang.Comparable is a java class that we extend...wondering if creating a editscript.diff.a-star/Coord works and compares fine

all tests working without further changes.

$ bb -cp "$(lein with-profiles +dev classpath)" "(require '[editscript.core :as e]) (e/diff [1 2 3 6] [1 2 4]) (require '[editscript.core-test] '[editscript.edit-test] '[editscript.util.pairing-test] '[editscript.diff.a-star-test] '[editscript.diff.quick-test]) (clojure.test/run-tests 'editscript.core-test 'editscript.edit-test 'editscript.util.pairing-test 'editscript.diff.a-star-test 'editscript.diff.quick-test)"

Testing editscript.core-test
{:result true, :num-tests 2000, :seed 1768321703402, :time-elapsed-ms 1411, :test-var "a-star-end-2-end-generative-test"}
{:result true, :num-tests 2000, :seed 1768321704814, :time-elapsed-ms 1705, :test-var "combine-edits-generative-test"}
{:result true, :num-tests 2000, :seed 1768321706519, :time-elapsed-ms 1008, :test-var "quick-end-2-end-generative-test"}

Testing editscript.edit-test

Testing editscript.util.pairing-test

Testing editscript.diff.a-star-test

Testing editscript.diff.quick-test

Ran 23 tests containing 158 assertions.
0 failures, 0 errors.
{:test 23, :pass 158, :fail 0, :error 0, :type :summary}

I can add a test in editscript.diff.a-star-test

can I be a contributor to your branch 😉

or wait maybe I can do it quickly here and paste it

no, scratch that

OK, paste it. All tests are now passing on my end without any further changes.

$ bb -cp "$(lein with-profiles +dev classpath)" "(require '[editscript.core :as e]) (e/diff [1 2 3 6] [1 2 4]) (require '[editscript.core-test] '[editscript.edit-test] '[editscript.util.pairing-test] '[editscript.diff.a-star-test] '[editscript.diff.quick-test]) (clojure.test/run-tests 'editscript.core-test 'editscript.edit-test 'editscript.util.pairing-test 'editscript.diff.a-star-test 'editscript.diff.quick-test)"

Testing editscript.core-test
{:result true, :num-tests 2000, :seed 1768321703402, :time-elapsed-ms 1411, :test-var "a-star-end-2-end-generative-test"}
{:result true, :num-tests 2000, :seed 1768321704814, :time-elapsed-ms 1705, :test-var "combine-edits-generative-test"}
{:result true, :num-tests 2000, :seed 1768321706519, :time-elapsed-ms 1008, :test-var "quick-end-2-end-generative-test"}

Testing editscript.edit-test

Testing editscript.util.pairing-test

Testing editscript.diff.a-star-test

Testing editscript.diff.quick-test

Ran 23 tests containing 158 assertions.
0 failures, 0 errors.
{:test 23, :pass 158, :fail 0, :error 0, :type :summary}

the Coord type is part of the a-start algorithm so I don't need to test it explicitly

pushed my hashCode change to my branch

you can test the dev version of bb with this

(check if the main branch has built the version for your OS, you might be getting the old one)

meanwhile I can make a PR for editscript. Exciting!

yeah that's great to have this thank you for working on it

sorry @borkdude how to get the dev version ... I guess it is not in the Releases ...did you mean building bb here locally?

Search history or check changelog, afk now

I've explained it before :-)

bash <(curl )  --dev-build --dir .

(and now my wife is getting mad at me so back to dinner)

duh apologies to wife 😄

Ok here is what I did to test my code • Downloaded bb from above • Cloned your repo - checked out your branch • Added an {} deps.edn • Added :local/rootreference to your branch in my :deps • Required my test namespace and ran tests

Testing unit.utils.history-test

Ran 2 tests containing 4 assertions.
0 failures, 0 errors.
{:test 2, :pass 4, :fail 0, :error 0, :type :summary}

@huahaiy Do you mind if I add a deps.edn to the project? I can do without it, but it's a bit easier for bb to run tests that way

actually I take that back, this is all I need to get bb tests running:

{:tasks
 {test:bb {:extra-paths ["src" "test"]
           :extra-deps {io.github.cognitect-labs/test-runner
                        {:git/tag "v0.5.0" :git/sha "b3fd0d2"}
                        org.clojure/test.check {:mvn/version "1.1.1"}}
           :task cognitect.test-runner/-main}}}

seems ok enough

deps.edn is nice to have cause you don't need to release for us to try things out 🙂

that's true!

https://github.com/juji-io/editscript/pull/42

ok let me add one deps.edn so we can use editscript as a git dep...

go ahead

I notice the CLJS tests are failing (due to some advanced error). Want me to fix it? Separate PR?

sure, thanks

ok, deps.edn added for this PR. ready for review

oh lol, already merged :)

I'll add the deps.edn in another PR

here's the deps.edn PR: https://github.com/juji-io/editscript/pull/43

now going to look at solving the CLJS issue

in another PR

deps.edn: https://github.com/juji-io/editscript/pull/43 CLJS tests: https://github.com/juji-io/editscript/pull/44

After merging this, I can make a new bb release but a new editscript release would also be nice since we're also using editscript in clerk and clerk has bb support, but I worked around editscript not working in bb :)

this is awesome to see - and so quickly 😄

thank you both - really

will do

hmm, weird, the tests seem to not work in CI and seem to download an older bb.

Message:  Unable to resolve symbol: sci.impl.deftype/hashCode

ah found the culprit. I had an expired token in bb CI but the build didn't fail on that, so for linux the dev build wasn't updated

let me update this so we can re-run editscript's CI

of course it's because of bash...

I'll get back to this in 15 minutes or so

it's no hurry, take your time

ok, if you can rerun editscript github actions, it should work now

All work now. Thanks!

nice!

I'll release bb now. will make another PR to remove dev build from your build and then you can make a release if you want

cool

https://github.com/juji-io/editscript/pull/45

I am not sure if we want to require java 21.

CI broke because you upgraded CLJS which requires it as a mininum JVM now due to Google Closure.

I can revert that

I know. I can downgrade CLJS for now. I think there are still people not ready for 21 yet

sure

revert pushed

you know what

I can make a separate job for CLJS

in this PR

if you want

that would be good

pushed

I can also split out bb if that makes sense

sure, if in the future it needs its own JVM etc.

ok all passing now

Awesome, I will cut a release

Thanks!