Use futures::future::select instead of select! macro

[gferon]

I always found the usage of select! macro makes it harder to understand what's happening. OTOH it's not really a necessary change, so I'd understand if you don't want it!

On top of this, rust-analyzer manages to understand this code, but fails with select!.

(this change will please @boxdot)

[rubdos]

I always found the usage of select! macro makes it harder to understand what's happening. OTOH it's not really a necessary change, so I'd understand if you don't want it!

IMO, I find the use of Either quite confusing. As long as there are only two possible paths, I suppose this is alright. If this makes people happier, I'll be glad to have it :-)

On top of this, rust-analyzer manages to understand this code, but fails with select!.

Not ever with the latest versions? I feel like they became smarter with macros.

this change will please @boxdot

Care to elaborate why? Hacking on that part?

[boxdot]

Care to elaborate why? Hacking on that part?

Nevermind. This is just my opinion of avoiding using custom DSL instead of plain Rust whenever possible. Feel free to close.

[rubdos]

Care to elaborate why? Hacking on that part?

Nevermind. This is just my opinion of avoiding using custom DSL instead of plain Rust whenever possible. Feel free to close.

The problem IMO is that there's a runtime penalty to Either (which is probably compiled away) and a legibility penalty (wtf is "Left" and "Right" in this context). The select! macro isn't that much more legible indeed, but at least it doesn't introduce packing everything up in an enum and then unpacking it again (maybe it does this behind the screens, like what I did before select! was a thing).

I'm just trying to understand the reasoning here. If this indeed makes RA happy, then it makes quite a lot of sense to merge it.

[gferon]

Note: the problem with rust-analyzer is that futures 0.3 uses proc_macro_hack.

See:

• "Unresolved macro html!" rust-lang/rust-analyzer#7221
• Removing proc_macro_hack rust-lang/futures-rs#2406

[rubdos]

Note: the problem with rust-analyzer is that futures 0.3 uses proc_macro_hack.

According to rust-lang/futures-rs#2406, this should be fixed in an upcoming Futures version.

Given that, and that the macro seems more legible to me (argument being Left and Right), I propose we close this.

[gferon]

I propose we close this.

I was going to propose exactly the same thing! Closing this PR.