marshallpierce / rust-base64

base64, in rust
Apache License 2.0
617 stars 115 forks source link

Restore base64::{encode, decode} functions #213

Closed rillian closed 1 year ago

rillian commented 1 year ago

This is a request to restore the base64::encode and base64::decode convenience functions deprecated in the 0.21.0 release.

Normally I agree with explicit being better than implicit, but I don't see the harm here. If there's no specific interoperability requirements, explicitly choosing a configuration isn't particularly important. For quick scripts, adding all of

// Import the base64 crate Engine trait anonymously so we can
// call its methods without adding to the namespace.
use base64::engine::Engine as _;
use base64::engine::general_purpose::STANDARD as BASE64;

at the top of the file and then calling BASE64.encode() is more cognitive load. With the new API I have to think about the trait, the current namespace, and why I'm calling a method on a constant. That's being explicit about the crate's implementation choices, not just the base64 variant.

In contrast, when I see a bare base64::encode() its clear some base64 implementation is being called to encode something in the default config, which is good enough a lot of the time.

marshallpierce commented 1 year ago

Sorry, I have no interest in making that style of coding easier. I want users to consciously choose what config they're using. I view blindly picking a default as a mistake, and I don't want to enable that, much in the same way that Java's concept of a "default text encoding" was also a mistake.

rillian commented 1 year ago

Thank you for explaining your motivation.

MathieuDuponchelle commented 1 year ago

Sorry, I have no interest in making that style of coding easier. I want users to consciously choose what config they're using. I view blindly picking a default as a mistake, and I don't want to enable that, much in the same way that Java's concept of a "default text encoding" was also a mistake.

Could we reach a middle ground here and have a more explicit convenience API, eg base64::standard_decode ?

I had never used this crate and had to use it yesterday, and without experience of the previous API I still found the current API quite cumbersome, especially the use base64::engine::Engine as _; part leaves a pretty bad taste :)

bradfier commented 1 year ago

Rust has a 'default text encoding' and that doesn't appear to be a justification to put every string method behind a StringLike trait, because that would be masses of cognitive overhead for very little benefit.

A convenience API as @MathieuDuponchelle suggests feels like the best compromise here if you're determined to deprecate encode and decode - at the moment 0.21 simply makes the library more difficult to use which I can't imagine was the intent.

marshallpierce commented 1 year ago

The encode() and decode() methods are not coming back. They were a mistake that predated my involvement in the project.

Rust's "default text encoding" is not a valid comparison because it is the only encoding used, and is provided as a guarantee throughout that text is UTF-8. The concept of default encoding in Java is a better comparison, where it has not worked out well.

If things like "using traits" are seen as too much of a burden, I can't help you.

sdroege commented 1 year ago

You're absolutely right that it should be an explicit choice which variant of base64 should be used (just like with e.g. CRC32 and percent encoding), but the way the current API is defined is really not great. It requires a lot of boilerplate to do such a simple task. There must be a better way for the boring cases where one wants just one of the standard alphabets.

Even having a set of crate-level functions in the style of fn decode<T: Engine>(input: impl AsRef<[u8]>) -> Result<...> would make usage less cumbersome for the simple case as it wouldn't require bringing the trait into scope.

That's also close to how the percent_encoding (alphabet as parameter) and crc (crate-level constants for the standard algorithms with directly accessible methods) crates work. Having a look at how other, similar crates handle this would seem like a good idea instead of only providing this usage-unfriendly API.

marshallpierce commented 1 year ago

This has already been discussed extensively elsewhere. I am not sympathetic to concerns over useing a trait. Rust is not the language for you if you don't like traits. Feel free to collect a full refund on your way out.

sdroege commented 1 year ago

Thanks for making clear that you care more about feeling superior than caring about API usability. I'll make sure to not use any of your crates if I can avoid it then. Too bad that one of your crates is occupying such a prominent name on crates.io. I hope you're feeling good about all the beginners that are just learning Rust and need base64 handling that are turned away by "base64 handling in Rust is just too complicated". Should they just go back to JavaScript, right?

And to be clear, this is not about not being able to understand your API. Just that usage of it simply sucks more than it has to. You might want to look at some of my crates if you feel like I'm just too stupid to deal with traits.

marshallpierce commented 1 year ago

@sdroege has earned himself a ban. Anyone else who has never contributed an ounce of effort to this library feel like criticizing the work I do for free?

sdroege commented 1 year ago

Thanks, likewise!

marshallpierce commented 1 year ago

shakes head some people just can't take a hint I guess. Locking this issue.