Skip to content

Commit

Permalink
Silence a bad Clippy suggestion
Browse files Browse the repository at this point in the history
Clippy suggests

    !('\u{fe00}'..='\u{fe0f}').contains(&ch)

instead of

    (ch < '\u{fe00}' || ch > '\u{fe0f}')

I disagree, because:

 * It adds more line noise (negation, &ch instead of ch).
 * It's hard to read (no space around character literals, which are
   already cluttered by the \ and {}, it becomes a line full of sigils.)
 * It uses the arcane ..= syntax that is relatively recent and very
   uncommon. Readers are not likely to know what it means.
 * It calls a method on a range, which is also somewhat arcane. It makes
   sense, but I think it is less obvious to a reader who may not know
   Rust as well, as plain comparisons that exist in any language.
 * "Inclusive range does not contain" does not feel obviously simpler
   to me than "outside of these bounds".

For integer literals it might make sense, but in this case the
suggestion looks like code golfing to me. It is also longer.
  • Loading branch information
ruuda committed Aug 11, 2022
1 parent 7ba101c commit bf45b8b
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions cli/maintainer/src/maintenance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,8 @@ fn sanitize_validator_name(name: &str) -> String {
// adds no information, strip it here to leave more space for graphs in
// dashboards, and not waste so much space on the redundant part of the name.
match name.strip_prefix("Lido / ") {
// Negated range.contains syntax only makes things more cryptic below.
#[allow(clippy::manual_range_contains)]
// I don't want distracting emojis in my Grafana dashboards.
Some(suffix) => suffix
.chars()
Expand Down

0 comments on commit bf45b8b

Please sign in to comment.