Remove unused base64.encode overload

[AmjadHD]

No description provided.

[Varriount]

Do we know why this procedure was added in the first place?

[AmjadHD]

There was the string overload, then the openarray overload was added in 6901a8c (#646), Ig the author wasn't aware that string is compatible with openArray[char].

[AmjadHD]

Is it ok to have an overload for integer types of arbitrary sizes. Shouldn't the input be byte strings.

[Varriount]

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?

[AmjadHD]

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 ?

[Varriount]

Can you give an example of what happens with 1-byte integers, vs multi-byte integers?

[AmjadHD]

import std/base64

var x = @[72, 101, 108, 108, 111] # Hello
let e = encode(x)
x[0] += 256
assert encode(x) == e

[Varriount]

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.

[AmjadHD]

I tried T: range[0..255]|uint8|char but this doesn't work even for encode [10].

[arnetheduck]

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)

[AmjadHD]

Should I change T: byte|char then and deprecate the T: SomeInteger overload ?

[AmjadHD]

I tried T: range[0..255]|uint8|char but this doesn't work even for encode [10].

Why doesn't it work ?

[AmjadHD]

cc @Varriount ^.

[Araq]

Instead of T: range[0..255]|uint8|char use T: byte|char

[AmjadHD]

@Araq, What about code like encode @[111, 113, 32] ? should it be deprecated ?

[AmjadHD]

@Araq is this fine ?

[Araq]

@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.

[AmjadHD]

Can this be merged now ?

[Araq]

So ... you left this one as SomeInteger? Bad idea.

[AmjadHD]

@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.

So code like that just breaks ?

[Varriount]

Wait, so can you explain why this procedure was left as-is?

[AmjadHD]

Wait, so can you explain why this procedure was left as-is?

@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.

" leave it as it is"

[AmjadHD]

@Araq please clarify what you want:

1. T: byte|char
2. T: byte|char and deprecate T: SomeInteger and not byte
3. T: SomeInteger|char

[Araq]

(2)

[AmjadHD]

@Araq merge this please.

[github-actions]

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 944e4cf

Hint: mm: orc; opt: speed; options: -d:release
162851 lines; 9.323s; 614.402MiB peakmem