Decide on extension trait convention

[aturon]

Tide uses many "extension traits" (which extend an existing type with new methods, and have no other impls). We should standardize on a naming convention.

[aturon]

One possibility would be FooExt where Foo is the name of the type being extended -- including the type in the name helps the reader quickly see what is being extended.

A downside is that we want to provide many extensions to Context. We'd end up with cookies::ContextExt and forms::ContextExt etc. But given that you only use these trait names on import, where they will be qualified by module, perhaps this is fine? The paths do contain the full set of relevant info: what is the extension, and what is being extended?

[bIgBV]

I think using ContextExt would be a good idea. This would be the common type name across the entire ecosystem which would signal that this type extends something on tide's Context. I think the recognizability this achieves will be really useful to convey the idea of how it's supposed to be used. Overall it provides a consistent name for the core extension trait in a given crate across all such crates.

@aturon I agree with you, given that the type itself will only be present at the top of the file for the most part and the type will not be used anywhere else, the import path will give enough context as to where this is coming from.

[secretfader]

Edit: see updated comment below.

[fairingrey]

I'm in support of using the *Ext naming scheme -- it's what I see a lot in the ecosystem already and feels the most familiar (namely in the futures crate).

EDIT: updated comment

[secretfader]

@fairingrey Ignore my comment above. I completely misunderstood the convention. A more appropriate naming scheme for the extension provided by Tide's querystring module is tide::querystring::ContextExt.

[fairingrey]

Oh, sorry! Yeah, that makes sense.

[Nemo157]

A downside of the chosen convention I recently ran into is not being able to actually bring multiple ContextExt into scope. A workaround is to use the relatively recent use foo::Foo as _; feature to import the traits methods into scope without having the trait nameable:

use tide::{cookies::ContextExt as _, forms::ContextExt as _};

This can also be mitigated in Tide for prelude users, if Tide has

pub mod prelude {
    use crate::{cookies::ContextExt as _, forms::ContextExt as _};
}

then a user adding use tide::prelude::*; will still get the effect of importing the methods without the traits.

[bIgBV]

Having a prelude is certainly useful, and I think we eventually should have one for the most common extension traits, but not all of them would be a part of the prelude right.

So we might have to rethink the naming scheme itself.

[Nemo157]

Personally I don't mind the as _, and am anti-preludes, so I'm happy to keep using that. (Although I've also had to use it in other places to force importing crates when I don't use any of their items before, so I may be pre-biased).

[bIgBV]

But don't you think as the ecosystem grows, the number of such imports would be an annoyance to type always.

Though to be fair, most would only be typed once and then forgotten.

[yoshuawuyts]

A few data points:

• RFC-0445 defines the general schema of how to name Ext traits.
• Add tests for ext traits + rename traits async-rs/futures-timer#18 recently went through a similar question, and we settled on FooExt where FooExt as _ can be used to prevent conflicts
• In Runtime's time PR we are defining trait impls that may conflict with futures::prelude, but using as _ in the prelude these problems can be evaded.

Given the prior discussion on this thread also, I feel we should go with the schema laid out in RFC-0445, and keep the as _ use in mind if we ever decide to define a prelude.

[Nemo157]

One thing to keep in mind is that examples should use as _ as well if that's how users are expected to use them (relevant to async-rs/futures-timer#18 as that is currently shadowing futures::prelude::TryFutureExt in the example, which could really confuse users as to why their method calls suddenly stop working when introducing futures-timer (EDIT: I'll make sure futures::prelude is fixed so that its extension traits are unshadowable, but I still think it's best to have as _ in all examples)).

[yoshuawuyts]

The discussion here has more or less quieted down. The futures crate is now also shadowing extension traits. It feels like we've reached consensus, and it's okay to close this now (: