Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Mar 31 16:51
  • Mar 31 16:47
    scala-steward opened #163
  • Mar 15 20:38
    khagaydv01 commented #161
  • Mar 15 20:38
    khagaydv01 commented #161
  • Mar 14 10:42
    satabin commented #161
  • Mar 13 16:42
    khagaydv01 commented #161
  • Mar 06 21:59
  • Mar 06 21:58
    satabin closed #154
  • Mar 06 21:57
    scala-steward opened #162
  • Mar 05 15:38
    satabin commented #161
  • Mar 05 15:37
    satabin labeled #161
  • Mar 04 17:36
    khagaydv01 edited #161
  • Mar 04 17:35
    khagaydv01 opened #161
  • Feb 27 15:25
    Travis gnieh/diffson (master) passed (355)
  • Feb 27 15:24
  • Feb 27 15:22
    satabin closed #160
  • Feb 27 15:22

    satabin on master

    Update cats-core to 2.1.1 Merge pull request #160 from sc… (compare)

  • Feb 25 13:41
  • Feb 25 13:38
    scala-steward opened #160
  • Feb 23 16:25
    Travis gnieh/diffson (master) passed (353)
Lucas Satabin
@satabin
when looking at this list of import I ask myself "what have i done ?!"
I have the impression I created a monster
Arnaud Gourlay
@agourlay
:laughing:
Lucas Satabin
@satabin
hmm my bad there is no default implicit Lcs
you must have added it yourself
can you find it in your code?
Arnaud Gourlay
@agourlay
I am pulling
object lcsdiff {
    object remembering {
      implicit def JsonDiffDiff[Json: Jsony: Lcs]: Diff[Json, JsonPatch[Json]] =
        new JsonDiff[Json](true, true)
    }
}
Lucas Satabin
@satabin
yes but the diff function takes an implicit Lcs
in the README I use Patience
but diffson doesn't provide any implicit default LCS implementation
I thought about it, but wanted to make it a conscious choice for users as it can make a big performance difference
Arnaud Gourlay
@agourlay
found it!
  implicit private val lcs: Patience[Json] = new Patience[Json]
Lucas Satabin
@satabin
ok
so fallback algorithm is DynProg
and between 3 and 4, only differences I see are also Eq and the caching part
Arnaud Gourlay
@agourlay
Ok, thanks for the info.
I'll try to extract a test case showing the difference so that we can have something more concrete to discuss next time :)
Lucas Satabin
@satabin
yeah sure
LCS is such a pain :)
Arnaud Gourlay
@agourlay
I am just a simple library user :p
Lucas Satabin
@satabin
if you can share the jsons on which you noticed the difference I can try to find out
Arnaud Gourlay
@agourlay
Will do :thumbsup:
Arnaud Gourlay
@agourlay
@satabin I have created a repo that shows my issue.
https://github.com/agourlay/diffson-issue
master is 4.x
and branch 3.x is for 3.x
It contains a single test implemented for both version
https://github.com/agourlay/diffson-issue/blob/master/src/test/scala/IssueSpec.scala
https://github.com/agourlay/diffson-issue/blob/3.x/src/test/scala/IssueSpec.scala
The test checks that the diff of two strings is containing the expected Remove operation.
The test is green on master and red on 3.x.
The Strings are straight from my employer's CI system, sorry for the size.
Lucas Satabin
@satabin
ok thanks I will investigate
Arnaud Gourlay
@agourlay
Awesome!
Arnaud Gourlay
@agourlay
Looks like I did some weird copy pasta in there, I'll do a second pass to make everything is fine.
Arnaud Gourlay
@agourlay
The test case is invalid, please no not lose your time with this before I fix it :)
Lucas Satabin
@satabin
ok :)
Arnaud Gourlay
@agourlay
Been hunting the issue all day, now I suspect it might not be in diffson after all :laughing:
Lucas Satabin
@satabin
I like this kind of bugs ^^
Arnaud Gourlay
@agourlay
And I just found it, it was a tricky one!
The culprit is actually the code feeding data to the diff logic, this means diffson is perfectly fine.
Garbage in, garbage out!
Thanks a lot for your support and Patience :laughing:
Lucas Satabin
@satabin
ahah sure :)
Arnaud Gourlay
@agourlay
Hi :wave: this is me again with a question :)
In version 3.x I used to call toString on the JsonPatch instance to get a nice printed JSON representation of the operations.
It worked by turning the JsonPatch into a JSON object and then calling pretty print on it.
https://github.com/gnieh/diffson/blob/v3.1.x/core/src/main/scala/gnieh/diffson/JsonPatchSupport.scala#L117
https://github.com/gnieh/diffson/blob/v3.1.x/circe/src/main/scala/gnieh/diffson/CirceInstance.scala#L184
However in 4.x this behaviour is gone as the toString is not overridden, so instead I print the object structure which is unexpected.
JsonPatch(List(Replace(Chain(Left(assets), Right(0), Left(key)),"new-val",Some("old-val"))))
I looked for a Show instance but could not find any.
Is it possible to reinstate the previous behaviour?
Or should I do it myself? How then?
Thank you :)
Lucas Satabin
@satabin
Indeed it's not there, because I decided to not put the (de)serialization things in diffson
however, an instance of Show provided by Jsony could be a nice idea
I will work on something
Lucas Satabin
@satabin
Currently if you want it, then you just need to go through the Json representation of your patch with patch.asJson.spaces2 in circe for instance
I am looking into how it would make sense to have it in diffson without lifting json (de)serealization into the core
Arnaud Gourlay
@agourlay
Thank you very much for the pointer, I have been able to recreate the desired behaviour :thumbsup:
Lucas Satabin
@satabin
Happy I could help :)
Zach Cox
@zcox
Hi - the implementation of JsonMergePatch.Value.apply just returns the toJson value, should it instead use toJson to merge patch the original argument? https://github.com/gnieh/diffson/blob/master/core/src/main/scala/diffson/jsonmergepatch/JsonMergePatch.scala#L35-L38
With diffson 3.1.1 we were using JsonMergePatch(p)(j), upgrading to 4.0.2 we're trying to use JsonMergePatch.Value[Json](p).apply[F](j) but that does not seem to use p to merge patch j, and instead just returns p.
Is there a different way in 4.0.2 to create a merge patch from a json value, and then apply it to another json value?
Zach Cox
@zcox
I think I answered my own question: use json.as[JsonMergePatch[Json]] to create the merge patch?
Lucas Satabin
@satabin
if I understand correctly you want to get a JsonMergePatch from as Json object?
I moved stuffs out from the core of diffson in version 4, namely everything that is related to (de)serialization
but you seem to have found it. I tried to reflect that in the README, if it's not clear enough I can try to explain a bit more
Lucas Satabin
@satabin
Just to make it clear: the Value constructor should only be constructed with non-object Json values. If you build it with an object then it won't work, since it is intended to be used for leaves
a patch for an object should use JsonMergePatch.Object
khagaydv01
@khagaydv01
Hi! After calling val diffResults = diff(json1, json2), is there a way to traverse through the diffResults.ops(0).path?