[WIP] Implement CIP-0025 #27
Conversation
Currently, there is a separation between the image media type and other ones. The reason for that was due to the requirement of an image MIME type for the outer CIP-0025, while the one inside individual `File`s is allowed to be any of the MIME types (a smarter datatype management can probably address this). Also, the implemented `Image` datatype might be too excessive. An alternate solution could be utilizing the `elm/mimetype` package.
src/String/Extra.elm
Outdated
| chunksOf : Int -> String -> List String | ||
| chunksOf chunkSize str = | ||
| let | ||
| go remainingStr acc = |
There was a problem hiding this comment.
Unfortunately I don't think the elm compiler is able to do TCO on let bindings. We'd have to compile an example to be 100% sure. Othewise, extracting as a top level function will do the trick.
There was a problem hiding this comment.
Thank you for pointing this out, I wasn't aware of it. This article has gone into details of which locations are subject to TCO, and let bindings are not among them. I'll define a top level helper.
src/Cardano/Cip25.elm
Outdated
| to [IANA registry](https://iana.org/assignments/media-types/media-types.xhtml#image). | ||
|
|
||
| Since the `image` field of CIP-0025 is required, and also must be one of the | ||
| image types, this datatype leads to a more robust model (too excessive?). |
There was a problem hiding this comment.
I'm wondering if we could do something similar to https://elm.dmy.fr/packages/danyx23/elm-mimetype/latest/MimeType
So it covers the majority of needs but explicitly models common file formats while leaving some room for extension.
I'd avoid directly depending on the package though as it's nice to keep our dependencies to a minimum.
There was a problem hiding this comment.
I did mention that package in one of my commit messages (incorrectly 😅). One thing I don't like about e.g. OtherImage approach is that it leads to allowing two representations for "predefined" variants. However, this excessive list of constructors is also not pleasant, as I think most of them are never going to be needed and can only bloat the final bundles.
There was a problem hiding this comment.
What do you think of making it much simpler by enabling potential multiple representations but providing a canonicalize function for that mime so that in practice we always call it, and the only case where it would not end in a canonical form would be when the user explicitly use the wrong constructor.
And even in that case, what are the potential downsides?
I suspect the encoder with end up with the same representation. So maybe the only downside is if someone tries to compare directly cip metadata on their own? Is that likely?
| The standard simply lays out a set of fields, some of which are optional. | ||
|
|
||
| -} | ||
| type alias Cip25 = |
There was a problem hiding this comment.
Description, mediatype and version seem optional https://github.com/cardano-foundation/CIPs/tree/master/CIP-0025#cddl
Also wondering why version is a tuple instead of a single Int?
There was a problem hiding this comment.
If I recall correctly, the reason I didn't use Maybe for description was similar to files, i.e. defaulting to an empty string/list in case of their absence (and omitting them from encoding if they're empty).
mediaType was an error on my part, I thought it was required. I'll make it a Maybe 👍
The standard points out that "A future version will introduce schema.org." While I frankly don't fully understand what they mean, looking through the website and also borrowing from MeshJS, it seems the idea is to support primary and secondary version numbers.
There was a problem hiding this comment.
Sorry for having taken 7 months to answer ... :(
Regarding the version, let’s keep it to what the CDDL says, so just an Int.
As for the description and files. Empty list for files sounds fine, but empty string for description is sketchy. I’d prefer a Maybe for this one. And if someone put an empty string in there, we’ll have a Just "".
Potentially adding an "other" variant in future.
I've modeled the arbitrary fields using `otherProps` accessors in both `File` and `Cip25`, with types of `Dict String Metadatum`. This could be somewhat restrictive as it only allows for `String` keys.
As mentioned in CIP-0068, the
metadatafield of thereference_NFT"is a low-level representation of the metadata, following closely the structure of CIP-0025." I'm still unsure whether it's a good idea to couple the two CIPs here. However, I think we should support CIP-0025 regardless.