percent-encoding: Allow to remove character from AsciiSet

[DoumanAsh]

Allow to remove character from AsciiSet

I liked trait approach better, but if you're going with something like AsciiSet it would be better to allow to remove bytes, as sometimes it is easier to take NON_ALPHANUMERIC and remove few necessary characters

---

This change is 

[SimonSapin]

remove looks good to me. What’s the motivation for exposing contains? If you’d like this on crates.io, please also bump the version number to 2.1.0.

[DoumanAsh]

@SimonSapin for contains I have use case when I want to determine if I'm going to use percent_encoding or not at all at runtime.
And previous major version had such ability, so I think it would be convenient to leave it out for user to use too.

Yes, I'll bump version too

UPD: To clarify, my use case is determining whether I should use filename or filename* in Content-Disposition

[SimonSapin]

Sorry, I still have a hard time seeing how calling contains is useful. Are you writing functions or methods that take a &AsciiSet parameter as input?

What does filename* refer to?

[DoumanAsh]

filename referes to https://tools.ietf.org/html/rfc6266#section-4.3

If you're planning to encode file name in content-disposition header you should use different attribute instead of regular filename and provide encoding information.
I only want to percent encode if file name doesn't contain valid set of symbols for path and/or outside of ASCII.
So originally I always checked ahead if I'm planning to encode file name https://github.com/DoumanAsh/yukikaze/blob/1.0/src/header/content_disposition.rs#L38 like that for 1.0 version

[SimonSapin]

Simply making contains public and using it in the code above would be incorrect as you’d also need to look for non-ASCII bytes.

It looks like what you’re trying to do is find out whether percent_encode would make any change to a given input string. To do that you could also use std::borrow::Cow::<str>::from(percent_encode(…)). If you get a Cow::Borrowed, the output is the same as the input and none of the bytes are encoded.

[SimonSapin]

Oh sorry that’s not accurate. That conversion can also return Cow::Borrowed if the input is a single byte that needs to be encoded. (In that case it borrows from a static string.)

Would branching on output == input work for you?

[DoumanAsh]

@SimonSapin Yes, I'd need to check both for non-ASCII and for AsciiSet::contains.

I guess if comparing output with input would be the only choice I'd go for it too, but it would require for me to percent ahead instead of doing it later.
Which may be ok in this case, but I'm not sure about other cases.

Nevertheless, if you feel like you don't want to expose this method, I can remove it from PR.

[DoumanAsh]

@SimonSapin ping

[benesch]

This would simplify the AttrCharEncodeSet in seanmonstar/reqwest#583 quite a bit, so I'm greatly in support of this landing as well.

[SimonSapin]

Looks good, thanks!

@bors-servo r+

[bors-servo]

📌 Commit ed7b991 has been approved by SimonSapin

[bors-servo]

⌛ Testing commit ed7b991 with merge 0e874dd...

[bors-servo]

☀️ Test successful - checks-travis
Approved by: SimonSapin
Pushing 0e874dd to master...

[SimonSapin]

This is in https://crates.io/crates/percent-encoding/2.1.0