Update Replacer to return a Result<_, T>

[pixel27]

This is more an enhancement requests. I'm wondering if it makes sense to update the method:

Regex::replace<R:Replacer>(&self, text: &str, rep: R) -> String 

To be

Regex::replace<E, R:Replacer<E>>(&self, text &str, rep: R) -> Result<String, E>

Then update regex Replacer to be:

pub trait Replacer<E> {
    fn reg_replace(&mut self, caps: &Captures) -> Result<Cow<str>, E>;

    fn no_expand(&mut self) -> Result<Option<Cow<str>>, E>;
}

The issue I ran into when implementing a Replacer was that I wanted to report an error with the replacement. The only way I could do that was have my Replacer implementation have a mutable reference to an Option<_> that I would set if I encountered an error, then in reg_replace check that and return "" because I was going to throw out the result anyway.

It would be nice if I could return an error from reg_replace() that would abort the whole replacement immediately and return the error to the caller. I know this would be a API change and could break existing code so that needs to be taken into consideration.

I'm not adverse to taking a crack at doing this change, if you are not adverse to changing the API, but I'd want to know that this is something you would consider before accepting before attempting it.

[BurntSushi]

This honestly seems a bit niche to me, and therefore may not be worth the added API complexity (which is somewhat significant IMO). If you look at the implementation of the replacen, you'll see that it is not that complex. I think I'd recommend copying it and modifying it for your needs at the moment.

Also, note that the Replacer trait is changing in 1.0 to something like this: #151 (comment)

I wonder if it would be plausible to change replace_append to return a bool, which would cause the replacement to abort if false. It still seems a little kludgy to me.

[pixel27]

I do agree it would complicate the API. I guess my feeling would be then that the Regex:replace operation is only useful in situations where the replace cannot fail or that panic would be a valid response to failure. Otherwise people would always need to implement their own replace operation like replacen does.

[BurntSushi]

@pixel27 Well, yes. I suspect the vast majority of use cases of replacen are not expected to fail. I don't really like the idea of making the API more complex to support an uncommon use case, especially when the cost of not doing so is small. Perhaps I am wrong about it being an uncommon use, but I don't recall this type of thing being prominently supported by other regex libraries either.

[BurntSushi]

I think the current API of replace matches what most would expect from using other regex libraries. Moreover, the actual implementation of replace is really simple with no special internal APIs used and no unsafe required, which I think means it's pretty easy to just roll this special type of API yourself.

[xixixao]

I ran into this as well. I guess nothing stops a small separate crate from implementing this.

I don't think it's a good candidate for "implement yourself", because the ask is very generic - Take a replacer that returns a result, and return a result, but hand rolling replacen is some amount of work.

It could be added here as try_replacen().

Edit: I made a crate for this https://docs.rs/regex_try/0.1.0/regex_try/