Emily Martins: 2 implement sorting on map keys implement sorting on map keys 8 files changed, 58 insertions(+), 16 deletions(-)
Committed and pushed! Thank you for being patient and willing to try out the patch workflow. Note for the future, you can write comments that should be included in the commit message by writing them above the first three-dashes-line, and comments that should not be in the commit message below the three dashes. Your comments seemed like they were for the patch, not necessarily to go into the commit message, so I amended your commit to clean/touch up the message slightly. For example: (email start) (Subject) My commit title here (Body) Extended commit message here --- Comments for the discussion list, but won't make it into the commit message comment.rkt | 12 ++++ (and so on) Thanks for contributing!
Thanks! I'll keep that in mind for when I make a patch in the future :)
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~williewillus/racket-libraries/patches/40188/mbox | git am -3Learn more about email & git
Looks like I accidentally included my Nix files, which I'm not sure belong here. Certainly not in the state there are. I've dropped those now. --- common.rkt | 12 ++++++++++-- encode.rkt | 12 +++++++++--- info.rkt | 2 +- scribblings/manual.scrbl | 12 ++++++++++-- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/common.rkt b/common.rkt index 979fb0a..79f9370 100644 --- a/common.rkt +++ b/common.rkt @@ -31,9 +31,11 @@ (provide cbor-config? cbor-config-tag-deserializers cbor-config-null-value + cbor-config-sorted-map-keys (contract-out [cbor-empty-config cbor-config?] [with-cbor-null (-> cbor-config? any/c cbor-config?)] + [with-sorted-map-keys (-> cbor-config? boolean? cbor-config?)] [with-cbor-tag-deserializer (-> cbor-config? cbor-valid-tag-number? (-> cbor-valid-tag-number? any/c any/c) @@ -41,9 +43,10 @@ (struct cbor-config (tag-deserializers - null-value)) + null-value + sorted-map-keys)) -(define cbor-empty-config (cbor-config #hasheqv() 'null)) +(define cbor-empty-config (cbor-config #hasheqv() 'null #f)) (define (with-cbor-tag-deserializer config id deser) (define old-handlers (cbor-config-tag-deserializers config)) @@ -55,3 +58,8 @@ (struct-copy cbor-config config [null-value v])) + +(define (with-sorted-map-keys config sorted) + (struct-copy + cbor-config config + [sorted-map-keys sorted])) diff --git a/encode.rkt b/encode.rkt index 4439a73..b9a09a6 100644 --- a/encode.rkt +++ b/encode.rkt @@ -3,6 +3,7 @@ (require racket/generic racket/undefined "common.rkt") +(require racket/match) (provide cbor-write gen:cbor-custom-write cbor-custom-write? @@ -93,9 +94,14 @@ (define (cbor-write-map config m out) (define len (hash-count m)) (write-argument out 5 len) - (hash-for-each m (lambda (k v) - (cbor-write config k out) - (cbor-write config v out))) + (if (cbor-config-sorted-map-keys config) + (for-each (match-lambda + [(cons k v) + (cbor-write config k out) + (cbor-write config v out)]) (sort (hash->list m) < #:key car))
Using match-lambda for just one case seems a bit weird, maybe just use a normal lambda and car/cdr?
+ (hash-for-each m (lambda (k v) + (cbor-write config k out) + (cbor-write config v out)))) (when (> len u64-max) (write-break out))) diff --git a/info.rkt b/info.rkt index 3b94a5c..21104fa 100644 --- a/info.rkt +++ b/info.rkt @@ -1,6 +1,6 @@ #lang info (define collection "cbor") -(define version "0.9") +(define version "0.10") (define deps '("base")) (define build-deps '("racket-doc" "rackunit-lib" diff --git a/scribblings/manual.scrbl b/scribblings/manual.scrbl index 21e0047..0009606 100644 --- a/scribblings/manual.scrbl +++ b/scribblings/manual.scrbl @@ -55,12 +55,14 @@ Major types are decoded as follows: } @section{Configuration Options} -@defstruct[cbor-config ((tag-deserializers (hash/c cbor-valid-tag-number? (-> cbor-valid-tag-number? any/c any/c))) (null-value any/c))]{ +@defstruct[cbor-config ((tag-deserializers (hash/c cbor-valid-tag-number? (-> cbor-valid-tag-number? any/c any/c))) (null-value any/c) (sorted-map-keys boolean?))]{ Configuration object for CBOR encoding and decoding. Note that this struct is only exposed opaquely, and must be manipulated with the provided functions and values. @racket[tag-deserializers] is a hash from CBOR tag numbers to a procedure that takes the CBOR tag number and a raw data value and produces the meaningful interpretation of that value. @racket[null-value] is the value that CBOR @tt{null} will be encoded and decoded as. + +@racket[sorted-map-keys] determines whether or not map keys should appear in sorted order. This only affects encoding; unsorted map keys will still be parsed identically. Keep in mind, this functionality should not be depended on by any decoders, but for legacy purposes some decoders don't parse properly otherwise.
I would add a note that this is only for serialization, not for deserialization. Maybe just change it to "determines whether or not map keys should be serialized in sorted order".
} @defthing[cbor-empty-config cbor-config?]{ @@ -80,6 +82,12 @@ Registers a tag deserializer to the given config, returning a new config. Registers a null value to the given config, returning a new config. } +@defproc[(with-sorted-map-keys [config cbor-config?] + [v boolean?]) + cbor-config?]{ +Set whether map keys should be sorted.
"Set whether map keys should be serialized in sorted order"
+} + @defthing[gen:cbor-custom-write any/c]{ A @tech{generic interface} that supplies a method, @racket[cbor-write-proc] that can serialize arbitrary values to CBOR. Implementations of the method should accept a value to be serialized and return it in serialized form as a @racket[cbor-tag?]. } @@ -113,4 +121,4 @@ Patches to improve these issues are welcome! Email patches to @tt{~williewillus/ @item{There should be a config or parameter to specify maximum lengths for indefinite-length objects} @item{More correctness-testing is needed.} @item{Some recommended tag and simple values from the RFC are not supported yet.} -] \ No newline at end of file +] -- 2.36.2
Hi! Thanks for the patch, just a couple comments: Emily Martins <emi@haskell.fyi> writes:
Thanks for the review! That all makes sense to me. I've applied those changes and here's the updated patch. --- common.rkt | 12 ++++++++++-- encode.rkt | 10 +++++++--- info.rkt | 2 +- scribblings/manual.scrbl | 12 ++++++++++-- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/common.rkt b/common.rkt index 979fb0a..79f9370 100644 --- a/common.rkt +++ b/common.rkt @@ -31,9 +31,11 @@ (provide cbor-config? cbor-config-tag-deserializers cbor-config-null-value + cbor-config-sorted-map-keys (contract-out [cbor-empty-config cbor-config?] [with-cbor-null (-> cbor-config? any/c cbor-config?)] + [with-sorted-map-keys (-> cbor-config? boolean? cbor-config?)] [with-cbor-tag-deserializer (-> cbor-config? cbor-valid-tag-number? (-> cbor-valid-tag-number? any/c any/c) @@ -41,9 +43,10 @@ (struct cbor-config (tag-deserializers - null-value)) + null-value + sorted-map-keys)) -(define cbor-empty-config (cbor-config #hasheqv() 'null)) +(define cbor-empty-config (cbor-config #hasheqv() 'null #f)) (define (with-cbor-tag-deserializer config id deser) (define old-handlers (cbor-config-tag-deserializers config)) @@ -55,3 +58,8 @@ (struct-copy cbor-config config [null-value v])) + +(define (with-sorted-map-keys config sorted) + (struct-copy + cbor-config config + [sorted-map-keys sorted])) diff --git a/encode.rkt b/encode.rkt index 4439a73..c0a2a07 100644 --- a/encode.rkt +++ b/encode.rkt @@ -93,9 +93,13 @@ (define (cbor-write-map config m out) (define len (hash-count m)) (write-argument out 5 len) - (hash-for-each m (lambda (k v) - (cbor-write config k out) - (cbor-write config v out))) + (if (cbor-config-sorted-map-keys config) + (for-each (lambda (pair) + (cbor-write config (car pair) out) + (cbor-write config (cdr pair) out)) (sort (hash->list m) < #:key car)) + (hash-for-each m (lambda (k v) + (cbor-write config k out) + (cbor-write config v out)))) (when (> len u64-max) (write-break out))) diff --git a/info.rkt b/info.rkt index 3b94a5c..21104fa 100644 --- a/info.rkt +++ b/info.rkt @@ -1,6 +1,6 @@ #lang info (define collection "cbor") -(define version "0.9") +(define version "0.10") (define deps '("base")) (define build-deps '("racket-doc" "rackunit-lib" diff --git a/scribblings/manual.scrbl b/scribblings/manual.scrbl index 21e0047..66c9a06 100644 --- a/scribblings/manual.scrbl +++ b/scribblings/manual.scrbl @@ -55,12 +55,14 @@ Major types are decoded as follows: } @section{Configuration Options} -@defstruct[cbor-config ((tag-deserializers (hash/c cbor-valid-tag-number? (-> cbor-valid-tag-number? any/c any/c))) (null-value any/c))]{ +@defstruct[cbor-config ((tag-deserializers (hash/c cbor-valid-tag-number? (-> cbor-valid-tag-number? any/c any/c))) (null-value any/c) (sorted-map-keys boolean?))]{ Configuration object for CBOR encoding and decoding. Note that this struct is only exposed opaquely, and must be manipulated with the provided functions and values. @racket[tag-deserializers] is a hash from CBOR tag numbers to a procedure that takes the CBOR tag number and a raw data value and produces the meaningful interpretation of that value. @racket[null-value] is the value that CBOR @tt{null} will be encoded and decoded as. + +@racket[sorted-map-keys] determines whether or not map keys should be serialized in sorted order. This only affects encoding; unsorted map keys will still be decoded identically. } @defthing[cbor-empty-config cbor-config?]{ @@ -80,6 +82,12 @@ Registers a tag deserializer to the given config, returning a new config. Registers a null value to the given config, returning a new config. } +@defproc[(with-sorted-map-keys [config cbor-config?] + [v boolean?]) + cbor-config?]{ +Set whether map keys should be serialized in sorted order. +} + @defthing[gen:cbor-custom-write any/c]{ A @tech{generic interface} that supplies a method, @racket[cbor-write-proc] that can serialize arbitrary values to CBOR. Implementations of the method should accept a value to be serialized and return it in serialized form as a @racket[cbor-tag?]. } @@ -113,4 +121,4 @@ Patches to improve these issues are welcome! Email patches to @tt{~williewillus/ @item{There should be a config or parameter to specify maximum lengths for indefinite-length objects} @item{More correctness-testing is needed.} @item{Some recommended tag and simple values from the RFC are not supported yet.} -] \ No newline at end of file +] -- 2.36.2
Committed and pushed! Thank you for being patient and willing to try out the patch workflow. Note for the future, you can write comments that should be included in the commit message by writing them above the first three-dashes-line, and comments that should not be in the commit message below the three dashes. Your comments seemed like they were for the patch, not necessarily to go into the commit message, so I amended your commit to clean/touch up the message slightly. For example: (email start) (Subject) My commit title here (Body) Extended commit message here --- Comments for the discussion list, but won't make it into the commit message comment.rkt | 12 ++++ (and so on) Thanks for contributing!