Allow tuples to be converted to JsonNode by % in json.nim as discussed in #24082 .
#24089
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allow tuples as input for `%`, but disallow them in the generic type of the `%` proc that accepts openArrays. Additionally addedthe isNamedTuple(T) proc. Can instead also be imported from std/typetraits, but ig dependencies should be kept to a minimum.
added additional conversion info to the proc, copied it straight from an earlier issue
Forgot to change parts of the copied code from jsonutils.
|
I think this is what causes the fails... but I don't really know why. Don't know what a Cluster or Pix is tbh. |
|
There was an issue that would cause the generic overload with |
Something like proc myproc[T: object|tuple](o:T): void = ... might not accept the tuple properly even though it's supposed to, such that: myproc(mytup) doesn't work. Idk if that was the actual problem here, but just to be sure, I changed the respective functions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in issue #24082.
%, to be able to convert named and unnamed tuples. Also changed part of the function itself to correctly discern between named and anonymous/unnamed tuples. The code was copied directly from jsonutils.nim (slightly changed).%proc to not allow tuples as array elements, such thatcannot be compiled.
Added / Copied the
isNamedTuple()proc from the jsonutils.nim file instead of adding a dependency by usingimport std/typetraits.Added the respective tests in tjson.nim, both for the macro
%*and the overloaded proc%for both anonymous/unnamed and named tuples.As in jsonutils.nim, the named tuples are converted to JObject while the anonymous tuples are converted to JArray.
Quick explanation as to why we had to disallow tuples for the openArray (so people don't have to scroll through the other convo):
Curly brackets together with a key value pair, kind of like this:
{"error": "No message"}will be compiled to[("error", "no message")]by Nim. Which is an array of tuples.That means, if we do
%{"error": "No message"}, the compiler will treat it as an array with a tuple inside.It will look like this under the hood:
%[("error", "No message")].If we would pass this to the openarray function (with the new tuple
%proc implemented), we should get:[["error", "No message"]].And that is somehow a problem, idk why though tbh.
Related issues or issues that I copied code/comments from:
#16851, #12290, #16851, #10019, #10823