Nim
Nim copied to clipboard
Remove unused `base64.encode` overload
Do we know why this procedure was added in the first place?
There was the string overload, then the openarray overload was added in 6901a8cb13d19a3d251911af240c5f2a54809eba (https://github.com/nim-lang/Nim/pull/646), Ig the author wasn't aware that string
is compatible with openArray[char]
.
Is it ok to have an overload for integer types of arbitrary sizes. Shouldn't the input be byte strings.
Please add a changelog entry, then this can be merged.
Is it ok to have an overload for integer types of arbitrary sizes. Shouldn't the input be byte strings.
Is this a question? And if so, what code are you referencing?
Please add a changelog entry, then this can be merged.
Is this needed, from the user's perspective nothing changed.
Is it ok to have an overload for integer types of arbitrary sizes. Shouldn't the input be byte strings.
Is this a question? And if so, what code are you referencing?
Yes, https://github.com/nim-lang/Nim/pull/20369/files#diff-cc4f63a53ac195237326427c11b84c89967c417d474b4cd55ad54b48a09c9a1cL144. This overload takes arbitrary integer types and wraps the values that do not fit in 1 byte, Is this behaviour desired or should it error instead ?
Can you give an example of what happens with 1-byte integers, vs multi-byte integers?
import std/base64
var x = @[72, 101, 108, 108, 111] # Hello
let e = encode(x)
x[0] += 256
assert encode(x) == e
Hm. I don't know. I would expect is one of the following:
- When given a multi-byte integer,
encode
would output multiple characters. -
encode
would not accept multi-byte integers.
Both are breaking changes, though the current behavior is arguably a bug. I would implement #2 first, and see how many, if any, packages that breaks.
I tried T: range[0..255]|uint8|char
but this doesn't work even for encode [10]
.
for reference, the API we use is: https://github.com/status-im/nim-stew/blob/master/stew/base64.nim (notably, there are many "base64" alphabet standards)
Should I change T: byte|char
then and deprecate the T: SomeInteger
overload ?
I tried
T: range[0..255]|uint8|char
but this doesn't work even forencode [10]
.
Why doesn't it work ?
cc @Varriount ^.
Instead of T: range[0..255]|uint8|char
use T: byte|char
@Araq, What about code like encode @[111, 113, 32]
? should it be deprecated ?
@Araq is this fine ?
@Araq, What about code like encode @[111, 113, 32] ? should it be deprecated ?
I think so but there is no RFC for it so better leave it as it is.
Can this be merged now ?
@Araq merge this please.
Thanks for your hard work on this PR! The lines below are statistics of the Nim compiler built from 944e4cf5853ad12f0738900e224109bb321715d0
Hint: mm: orc; opt: speed; options: -d:release 162851 lines; 9.323s; 614.402MiB peakmem