-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: encoding/base64: add Encoding.EncodeString to match DecodeString #51295
Comments
The reverse is I am not sure what else you are looking for. |
Is the function |
What is the exact API that you are proposing? We currently have // DecodeString returns the bytes represented by the base64 string s.
func (enc *Encoding) DecodeString(s string) ([]byte, error)
// EncodeToString returns the base64 encoding of src.
func (enc *Encoding) EncodeToString(src []byte) string What are you suggesting that we add? Thanks. |
Do you not understand that the input arguments to those functions are different? If there is an Encode function there should be a Decode function with the same name structure and:
I realize what happened, someone was building it and needing to encode a byte array to a string so they wrote that function, then needed to decode a string to a byte array so they wrote that function. When people read the documentation first, they are confused by the lack of a common naming structure and input/output type structure. That should be cleaned and reorganized so that when people are working and reading the 1000's of Go functions (certainly impossible to remember all) they will not get frustrated by constant flipping between string and []byte and a lack of a naming structure that can be quickly remembered because it adheres to both common function names per functionality and input/output type. |
This comment was marked as off-topic.
This comment was marked as off-topic.
@andrewhodel Help us out a bit if you want us to consider your change. Please tell us exactly what method you want added. You've more than adequately defined the motivation - but we're still not clear on what exactly the changes you're proposing are. Write us a prototype of the function/method you want, and a 1-2 sentence godoc description of what it does. Thanks. |
I didn't read every function, but you should and you should adhere this naming structure and ensure that all relevant sets exist. In other words
Then when someone is working for example with the encoding/base64 library, they can set a number of days or hours aside to work with the library then get used to the functions. In other words, they can type Encode and Decode and EncodeString and DecodeString enough times to not need to deal with remembering arguments to every single function instead only needing to remember the top level function names as they all follow a common naming pattern. |
Ok, but you still haven't answered my question - what exactly are you proposing to add to |
I answered your question in the first sentence of the original reply to your original comment/question. EncodeString() and DecodeString() should both exist because one of them does and the concept of encoding/decoding is fundamentally bound together. These are functions in Go of the encoding/base64 library. One exists, one doesn't. Again, these are functions in Go. |
@andrewhodel That is not an answer to the question that @randall77 asked and it is not an answer to the question I asked. I will repeat: We currently have // DecodeString returns the bytes represented by the base64 string s.
func (enc *Encoding) DecodeString(s string) ([]byte, error)
// EncodeToString returns the base64 encoding of src.
func (enc *Encoding) EncodeToString(src []byte) string What are you suggesting that we add? Please write it in the exact form that I wrote it. This is a programming language. |
You should also read and make sure all the other pairs exist for encoding and decoding of, with respect to both input and output:
If you want me to write it, then provide commit access. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
I don't see why Other than that, seems like a reasonable function to have. Do you have a situation where you have a |
In any case, thanks for defining precisely what you suggest that we add (although you omitted the doc comment). I gather you are thinking of something along the lines of // EncodeString returns the base64 encoding of s.
func (enc *Encoding) EncodeString(s string) ([]byte, error) {
buf := make([]byte, enc.EncodedLen(len(s)))
enc.Encode(buf, []byte(src))
return buf, nil
} I think the argument that you are making is that this is parallel to the existing How often does a need for this method come up in practice? Do you have any examples of code that would use it? Thanks. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Lack of symmetry.
Yes you need an error, what if the memory bound is reached while reading?
Thank You,
Andrew Hodel
… On Feb 22, 2022, at 11:09 AM, Keith Randall ***@***.***> wrote:
I don't see why EncodeString should return an error. The other Encode functions do not. Base64 encoding and decoding are not symmetric in that encoding should always succeed, whereas decoding might not.
Other than that, seems like a reasonable function to have. Do you have a situation where you have a string source and wish to encode it to a []byte output? Just wondering if this is a function you actually ran into a need for, or if you just were confused about the lack of a symmetric API.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
This comment was marked as off-topic.
This comment was marked as off-topic.
If a Go program runs out of memory, it will crash. There is no way for a function to return an error because it has run out of memory. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Again. This is why other libraries have per iteration input functions, for reading streams etc.
That isn’t to say that you could read the input string length, then read the available memory and know what difference the encoding process will require (bytes per chunk for input and output + anything else) and return an error when that is greater than n*available.
That is how things work.
Thank You,
Andrew Hodel
… On Feb 22, 2022, at 11:13 AM, Ian Lance Taylor ***@***.***> wrote:
If you count the number of times someone commits vs the changes approved in that list you will get it.
Again, I'm sorry, but I don't understand what you mean. If you have actual data, can you simply show it?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
This comment was marked as off-topic.
This comment was marked as off-topic.
These truths of memory testing (especially when it is very low resource usage to do so) are similar to network programming in a day and age that testing is sold for entertainment as blocking certain tcp packets against protocol.
In other words it has to be dealt with anytime network traffic is greater than the packet MTU so may as well exist at the memory level.
Thank You,
Andrew Hodel
… On Feb 22, 2022, at 11:23 AM, Andrew Hodel ***@***.***> wrote:
Again. This is why other libraries have per iteration input functions, for reading streams etc.
That isn’t to say that you could read the input string length, then read the available memory and know what difference the encoding process will require (bytes per chunk for input and output + anything else) and return an error when that is greater than n*available.
That is how things work.
Thank You,
Andrew Hodel
>> On Feb 22, 2022, at 11:13 AM, Ian Lance Taylor ***@***.***> wrote:
>>
>
> If you count the number of times someone commits vs the changes approved in that list you will get it.
>
> Again, I'm sorry, but I don't understand what you mean. If you have actual data, can you simply show it?
>
> —
> Reply to this email directly, view it on GitHub, or unsubscribe.
> You are receiving this because you were mentioned.
|
The current helper methods assume that base64 is applied to binary data, which is almost always a []byte. |
Based on the discussion above, this proposal seems like a likely decline. |
I guess every middleman your Go servers work with support perfect utf8. I need to send data to things so old that anything beyond 127 bits is out of the question so they can pass it on and then without base64 what are you going to do? I know, another go type modification is just what the doctor called for! |
@rsc it's simple, you have a string and a validation function that ensures that string is "at protocol" meaning hasn't the distant device doesn't even fully support ASCII, then you need to pass it through that distant device so you base64 encode the validated string and send it. What you are saying isn't real, there's no where on earth where an operating system requires code to not using bits and those work with Go servers that use strings for simplicity. |
@andrewhodel I think it would help a lot if you made an example on https://go.dev/play/ to show your use-case and why this isn't trivially covered by the existing API. |
Code reviewer: "That is messy code, there are too many type modifications". I guess they will never figure it out, oh well poof. |
No change in consensus, so declined. |
The DecodeString() method exists as a single function, an EncodeString() method should be added as it will not break forward compatibility and conform to a standard naming structure and module offering.
https://pkg.go.dev/encoding/base64
This is not a duplicate of a bug, I was asked to make it a proposal regardless of it being a bug or proposal.
This is a proposal in writing to a software repository.
The text was updated successfully, but these errors were encountered: