encoding feature breaks API

[stbuehler]

Hi,

the way features work in cargo is that any crate in your dependency tree can enable additional features for its dependencies, and each dependency ("crate") is only compiled once with all features merged; thus they must only add additional "APIs" without breaking what was before.

I.e. enabling the encoding feature should provide new structs and functions (or just improve existing ones without changing their signature).

Also the reader::Decoder struct is not public and not documented, but Reader::decoder() is... I guess either both or none should be public.

[tafia]

You are right, this is an oversight on my side.

[pcleavelin]

I believe this issue should be re-opened due to this commit as it still breaks the API when enabling the encoding feature.

This is causing issues on a project I work on that relies on Attribute::unescape_value existing when including a dependency that enables encoding internally.

[Mingun]

I think, that we shouldn't. The current situation removes some functions if encoding feature is enabled. Yes, that is not a good thing, because another crate in your dependency tree can enable that feature and your code that uses this function will stop to compile. In that case just do not use those functions if you unsure if something can enable them. Definitely it is bad idea use it in the library code, but you could find them useful in application code. I'll accept a PR that will add an explicit attention to docstrings of that functions about that nuance.

[pcleavelin]

Maybe I'm missing context on why the function is removed with encoding in the first place. Is there some state that isn't updated if you were to use it? (Forgive me I'm not familiar with this codebase).

[Mingun]

Because those set of functions uses UTF-8 encoding to convert internal &[u8] to strings. That is possible, because when encoding feature is not enabled we always assume that documents in UTF-8. When encoding feature is not enabled, we cannot assume that, because &[u8] can contain data in other encoding. Having functions which will try to interpret them as UTF-8 would be source of subtle bugs, so we does not provide those functions in that case. You should use alternatives with explicit Decoder argument.