Add short-circuit check to form-decode-str and pre-lookup charset

ring-clojure / ring-codec

Utility library for encoding and decoding data

MIT License

63 stars 30 forks source link

Add short-circuit check to form-decode-str and pre-lookup charset #38

Closed bsless closed 1 year ago

bsless commented 1 year ago

Closes #37

bsless commented 1 year ago

@borkdude you might care to check if this is sci/babashka compatible or direct me on how to check it myself

borkdude commented 1 year ago

I don't see any problems but I left one comment which will make it more performant in bb :)

bsless commented 1 year ago

Narrowed the changes down only to the short circuit check

@borkdude can't use include? as it was added in 1.8 and ring codec supports 1.7

bsless commented 1 year ago

@weavejester I believe the first MR is ready for review

bsless commented 1 year ago

@weavejester Suggested changes done besides:

str/includes?: requires clojure 1.8
let binding the encoding: compiler couldn't figure out the type hint, but the next MR with charset changes fixes that

weavejester commented 1 year ago

Thanks! Could you change the commit message to:

Add short-circuit check to form-decode-str

Improves performance when form-encoded string has no encoded characters.
Fixes #37.

It's a small change, but just helps indicate that the commit closes #37, rather than being tangentially related. Otherwise it all looks good to merge!

bsless commented 1 year ago

Done :)