Improvements to EitherOrBoth
#629
Conversation
JoJoJet
commented
Jul 4, 2022
•
JoJoJet
commented
- Added more methods for accessing the left and right variants.
- Added methods for inserting left and right values (using unsafe code).
- Improved some grammar.
|
Hi there, thanks for addressing the compilation issues. Could you rebase the changes so we do not need to review back-and-forth changes? |
Done, I think. |
| pub fn both(self) -> Option<(A, B)> { | ||
| match self { | ||
| Both(a, b) => Some((a, b)), | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| /// If `Left` or `Both`, return the left value. Otherwise, convert the right value and return it. | ||
| pub fn into_left(self) -> A |
There was a problem hiding this comment.
I prefer left_biased, which implies the fallback behavior on Right. So does into_right -> right_biased.
This *_biased wording is also used by rowan, the parser library of rust-analyzer, in an enum like our EitherOrBoth but allows empty.
https://docs.rs/rowan/0.15.11/rowan/enum.TokenAtOffset.html#method.left_biased
There was a problem hiding this comment.
I don't think that the *_biased naming convention really captures what's going on here. The TokenAtOffset struct does not have to perform type conversions which makes that name work -- however EitherOrBoth does have to perform type conversions, so calling it biased doesn't really make sense to me. The into_* convention communicates that this operation performs a type conversion when necessary. Also, the TokenAtOffset struct seems too different to be comparable to EitherOrBoth. That struct does not have Left or Right variants, just a Single variant that does not specify a side. Plus, as you mentioned, it has an None variant which drastically changes how this operation works.
There was a problem hiding this comment.
I don't think that the
*_biasednaming convention really captures what's going on here.
To my intuition, left_biased implies that "prefer Left, but fallback to return Right if there's no Left". into_left sounds like "we know there's a Left inside, just unwrap it" (like into_inner?).
But you're right, there's a conversion when Right(_) is encountered, and into_left captures this behavior.
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
