Blanket impl breaks things

[Manishearth]

https://github.com/reem/rust-modifier/blob/master/src/lib.rs#L37

I really liked the blanked impl at first, however when using it in hyper it clashes with the set() method of Headers. set() is a pretty common method name, perhaps we should rename it? Or remove the blanket impl and have users explicitly impl it on structs.

[reem]

The blanket impl lets you use modifiers even on types you haven't defined, which I think is really important.

I would be open to renaming the method, what do you think would be a better name?

[Manishearth]

Perhaps it could be hidden behind a cfg flag?

The names would have to be unique so that they don't clash with anything. Not sure of any names that would work that way. apply_modifier() seems pretty unique.

Alternatively, T.builder().set(...).set(....).set(....).inner() where Builder<T> == Builder(T) can be deref'd into the inner type (and is just a wrapper) would work.

[reem]

I don't think a cfg flag would work here, since other libraries might depend on the blanket impl, unless cargo can deal with cases like that.

I'm also not a huge fan of Builder because it makes the non-builder case (.builder().set().inner()) very noisy.

[Manishearth]

Firstly, is there a major need to be able to add a modifier to an externally-defined type? I think the general recommendation for doing cross-trait stuff still applies here -- use a newtype. Blanket impls effectively add more reserved identifiers, this isn't really good :/

[reem]

I see your point and filed an rfc issue asking for conventions here.

I'm trying to think of a good solution that doesn't hamper most use cases, but I'm having trouble coming up with anything that meets all of the needed criteria. The best I have is to have two traits, Set and SetExt, where SetExt has a blanket impl and Set doesn't (and has a longer name for the method) but that breaks using Set as a generic bound, since types might impl SetExt but not Set. It's also pretty confusing.

[Manishearth]

that doesn't hamper most use cases

I don't see many use cases where you'd want to make a builder out of an existing type.

[Manishearth]

Also, +1 rfc :)

[reem]

I don't see many use cases where you'd want to make a builder out of an existing type.

Modifier is actually useful outside the builder case too - one of the primary motivators for its existence is that it allows you to write chaining methods that work on &mut self -> &mut Self and self -> Self once and use them in both cases.

For instance in Iron modifier is used to construct Response through set, but AfterMiddleware can happily use the same infrastructure to modify responses with set_mut.

[Manishearth]

Right, that makes sense.

[reem]

I've thought about it more, and I am thinking that you may be right and it might be best to just remove the blanket impl and require a impl Set for Type {} for types you want to use modifiers with.

[reem]

The blanket impl is gone.