Dyalog / nuget

Tools to help use NuGet packages from Dyalog APL
MIT License
3 stars 3 forks source link

On Dyalog 19.0 the function NuGet.Using does not accept a serialized namespace with curly braces as left parameter anymore #14

Open Alexander-Block opened 4 months ago

Alexander-Block commented 4 months ago

The function NuGet.Using is meant to accept a JSON5 string as left option argument which is internally converted to a Dyalog APL namespace. This conversion uses a call to ⎕SE.Dyalog.Array.Deserialise without left parameter. The semantics of ⎕SE.Dyalog.Array.Deserialise seems to have changed with the release of Dyalog APL 19.0, which leads to this conversion not working as expected anymore:

      ⎕SE.Dyalog.Array.Deserialise '{includePrimary : 0}'
     ∇{includePrimary:0}
     ∇

This in turn leads to a call to NuGet.Using with a JSON5 string left argument failing with a DOMAIN ERROR. I have described an example of this error in this Cider issue.

Alexander-Block commented 4 months ago

I have filed a corresponding issue for the NuGet api here.

mkromberg commented 4 months ago

I think the old version of Deserialise accepted the incorrect use of curly braces to denote a namespace. The correct notation is to use parentheses:

  NuGet.Using project_dir

,/tmp/nuget-test/published/Clock.dll ,/tmp/nuget-test/published/nuget-test.dll '(includePrimary:0)' NuGet.Using project_dir ,/tmp/nuget-test/published/Clock.dll

P.S. Apologies if I was the one who gave you the impression that {} braces would be OK, I have made this mistake many times in the past ;)

Alexander-Block commented 4 months ago

Thank you for the very fast response, @mkromberg! That makes a lot of sense considering that JSON5 notation can clash with defn notation.

Unfortunately it was not only my impression that curly braces are okay in this context, but it is currently part of the documentation here.

I think that the documentation should be changed accordingly and this API change may break the setup of projects that used NuGet under Dyalog APL 18.2 before (which admittedly may only be Cider whose problem should be solved with this PR). The other option would be to change from a use of ⎕SE.Dyalog.Array.Deserialise to ⎕JSON with fitting option parameters, but if I understand you correctly that may clash with the general API template you envsion for passing options to APL functions.

mkromberg commented 4 months ago

Yes, so it is I who has led you astray, I will correct the documentation today. I suspect your PR is all that is required at this point. Apologies and thanks for catching this and reporting it!

mkromberg commented 4 months ago

I did not read your report carefully enough: To be precise, the left argument is not JSON5, it is APL Array Notation. A bug in the old implementation allowed curly braces.

Alexander-Block commented 4 months ago

Thank you for the explanation. That clears up my main misunderstanding. I hadn't worked with APL array notation for namespaces before and - based on the example in the README - thought that JSON5 was the way to serialize namespaces in Dyalog APL. It is good to know that this is not the case.