Skip to content

Commit

Permalink
Improve case insensitivity consistency (nushell#10884)
Browse files Browse the repository at this point in the history
# Description

Add an extension trait `IgnoreCaseExt` to nu_utils which adds some case
insensitivity helpers, and use them throughout nu to improve the
handling of case insensitivity. Proper case folding is done via unicase,
which is already a dependency via mime_guess from nu-command.

In actuality a lot of code still does `to_lowercase`, because unicase
only provides immediate comparison and doesn't expose a `to_folded_case`
yet. And since we do a lot of `contains`/`starts_with`/`ends_with`, it's
not sufficient to just have `eq_ignore_case`. But if we get access in
the future, this makes us ready to use it with a change in one place.

Plus, it's clearer what the purpose is at the call site to call
`to_folded_case` instead of `to_lowercase` if it's exclusively for the
purpose of case insensitive comparison, even if it just does
`to_lowercase` still.

# User-Facing Changes

- Some commands that were supposed to be case insensitive remained only
insensitive to ASCII case (a-z), and now are case insensitive w.r.t.
non-ASCII characters as well.

# Tests + Formatting

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
  • Loading branch information
2 people authored and dmatos2012 committed Feb 20, 2024
1 parent 31c77eb commit 4143be4
Show file tree
Hide file tree
Showing 35 changed files with 176 additions and 122 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions crates/nu-cli/src/completions/custom_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use nu_protocol::{
engine::{EngineState, Stack, StateWorkingSet},
PipelineData, Span, Type, Value,
};
use nu_utils::IgnoreCaseExt;
use reedline::Suggestion;
use std::collections::HashMap;
use std::sync::Arc;
Expand Down Expand Up @@ -153,8 +154,8 @@ fn filter(prefix: &[u8], items: Vec<Suggestion>, options: &CompletionOptions) ->
(true, true) => it.value.as_bytes().starts_with(prefix),
(true, false) => it.value.contains(std::str::from_utf8(prefix).unwrap_or("")),
(false, positional) => {
let value = it.value.to_lowercase();
let prefix = std::str::from_utf8(prefix).unwrap_or("").to_lowercase();
let value = it.value.to_folded_case();
let prefix = std::str::from_utf8(prefix).unwrap_or("").to_folded_case();
if positional {
value.starts_with(&prefix)
} else {
Expand Down
3 changes: 2 additions & 1 deletion crates/nu-cli/src/completions/file_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use nu_protocol::{
engine::{EngineState, StateWorkingSet},
levenshtein_distance, Span,
};
use nu_utils::IgnoreCaseExt;
use reedline::Suggestion;
use std::path::{Path, MAIN_SEPARATOR as SEP};
use std::sync::Arc;
Expand Down Expand Up @@ -125,7 +126,7 @@ pub fn matches(partial: &str, from: &str, options: &CompletionOptions) -> bool {
if !options.case_sensitive {
return options
.match_algorithm
.matches_str(&from.to_ascii_lowercase(), &partial.to_ascii_lowercase());
.matches_str(&from.to_folded_case(), &partial.to_folded_case());
}

options.match_algorithm.matches_str(from, partial)
Expand Down
8 changes: 3 additions & 5 deletions crates/nu-cli/src/completions/variable_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ impl Completer for VariableCompletion {
) -> Vec<Suggestion> {
let mut output = vec![];
let builtins = ["$nu", "$in", "$env"];
let var_str = std::str::from_utf8(&self.var_context.0)
.unwrap_or("")
.to_lowercase();
let var_str = std::str::from_utf8(&self.var_context.0).unwrap_or("");
let var_id = working_set.find_variable(&self.var_context.0);
let current_span = reedline::Span {
start: span.start - offset,
Expand All @@ -57,7 +55,7 @@ impl Completer for VariableCompletion {
// Completions for the given variable
if !var_str.is_empty() {
// Completion for $env.<tab>
if var_str.as_str() == "$env" {
if var_str == "$env" {
let env_vars = self.stack.get_env_vars(&self.engine_state);

// Return nested values
Expand Down Expand Up @@ -109,7 +107,7 @@ impl Completer for VariableCompletion {
}

// Completions for $nu.<tab>
if var_str.as_str() == "$nu" {
if var_str == "$nu" {
// Eval nu var
if let Ok(nuval) = eval_variable(
&self.engine_state,
Expand Down
13 changes: 6 additions & 7 deletions crates/nu-cli/src/menus/help_completions.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use nu_engine::documentation::get_flags_section;
use nu_protocol::{engine::EngineState, levenshtein_distance};
use nu_utils::IgnoreCaseExt;
use reedline::{Completer, Suggestion};
use std::fmt::Write;
use std::sync::Arc;
Expand All @@ -13,21 +14,19 @@ impl NuHelpCompleter {

fn completion_helper(&self, line: &str, pos: usize) -> Vec<Suggestion> {
let full_commands = self.0.get_signatures_with_examples(false);
let folded_line = line.to_folded_case();

//Vec<(Signature, Vec<Example>, bool, bool)> {
let mut commands = full_commands
.iter()
.filter(|(sig, _, _, _, _)| {
sig.name.to_lowercase().contains(&line.to_lowercase())
|| sig.usage.to_lowercase().contains(&line.to_lowercase())
sig.name.to_folded_case().contains(&folded_line)
|| sig.usage.to_folded_case().contains(&folded_line)
|| sig
.search_terms
.iter()
.any(|term| term.to_lowercase().contains(&line.to_lowercase()))
|| sig
.extra_usage
.to_lowercase()
.contains(&line.to_lowercase())
.any(|term| term.to_folded_case().contains(&folded_line))
|| sig.extra_usage.to_folded_case().contains(&folded_line)
})
.collect::<Vec<_>>();

Expand Down
8 changes: 4 additions & 4 deletions crates/nu-cli/src/reedline_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ fn add_parsed_keybinding(
let modifier = match keybinding
.modifier
.into_string("", config)
.to_lowercase()
.to_ascii_lowercase()
.as_str()
{
"control" => KeyModifiers::CONTROL,
Expand All @@ -641,7 +641,7 @@ fn add_parsed_keybinding(
let keycode = match keybinding
.keycode
.into_string("", config)
.to_lowercase()
.to_ascii_lowercase()
.as_str()
{
"backspace" => KeyCode::Backspace,
Expand Down Expand Up @@ -728,15 +728,15 @@ fn parse_event(value: &Value, config: &Config) -> Result<Option<ReedlineEvent>,
match value {
Value::Record { val: record, .. } => match EventType::try_from_record(record, span)? {
EventType::Send(value) => event_from_record(
value.into_string("", config).to_lowercase().as_str(),
value.into_string("", config).to_ascii_lowercase().as_str(),
record,
config,
span,
)
.map(Some),
EventType::Edit(value) => {
let edit = edit_from_record(
value.into_string("", config).to_lowercase().as_str(),
value.into_string("", config).to_ascii_lowercase().as_str(),
record,
config,
span,
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cmd-dataframe/src/dataframe/eager/sql_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn apply_window_spec(expr: Expr, window_type: Option<&WindowType>) -> Result<Exp
fn parse_sql_function(sql_function: &SQLFunction) -> Result<Expr> {
use sqlparser::ast::{FunctionArg, FunctionArgExpr};
// Function name mostly do not have name space, so it mostly take the first args
let function_name = sql_function.name.0[0].value.to_lowercase();
let function_name = sql_function.name.0[0].value.to_ascii_lowercase();
let args = sql_function
.args
.iter()
Expand Down
3 changes: 2 additions & 1 deletion crates/nu-cmd-extra/src/extra/formats/to/html.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use nu_protocol::{
record, Category, Config, DataSource, Example, IntoPipelineData, PipelineData,
PipelineMetadata, ShellError, Signature, Spanned, SyntaxShape, Type, Value,
};
use nu_utils::IgnoreCaseExt;
use rust_embed::RustEmbed;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
Expand Down Expand Up @@ -180,7 +181,7 @@ fn get_theme_from_asset_file(
let th = asset
.themes
.into_iter()
.find(|n| n.name.to_lowercase() == theme_name.to_lowercase()) // case insensitive search
.find(|n| n.name.eq_ignore_case(theme_name)) // case insensitive search
.unwrap_or_default();

Ok(convert_html_theme_to_hash_map(is_dark, &th))
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-color-config/src/nu_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ fn fill_modifiers(attrs: &str, style: &mut Style) {
//
// since we can combine styles like bold-italic, iterate through the chars
// and set the bools for later use in the nu_ansi_term::Style application
for ch in attrs.to_lowercase().chars() {
for ch in attrs.chars().map(|c| c.to_ascii_lowercase()) {
match ch {
'l' => style.is_blink = true,
'b' => style.is_bold = true,
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/conversions/fill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn fill(
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);

let alignment = if let Some(arg) = alignment_arg {
match arg.to_lowercase().as_str() {
match arg.to_ascii_lowercase().as_str() {
"l" | "left" => FillAlignment::Left,
"r" | "right" => FillAlignment::Right,
"c" | "center" | "m" | "middle" => FillAlignment::Middle,
Expand Down
4 changes: 2 additions & 2 deletions crates/nu-command/src/conversions/into/bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ fn into_bool(
}

fn string_to_boolean(s: &str, span: Span) -> Result<bool, ShellError> {
match s.trim().to_lowercase().as_str() {
match s.trim().to_ascii_lowercase().as_str() {
"true" => Ok(true),
"false" => Ok(false),
o => {
let val = o.parse::<f64>();
match val {
Ok(f) => Ok(f.abs() >= f64::EPSILON),
Ok(f) => Ok(f != 0.0),
Err(_) => Err(ShellError::CantConvert {
to_type: "boolean".to_string(),
from_type: "string".to_string(),
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/conversions/into/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl Zone {
}
}
fn from_string(s: String) -> Self {
match s.to_lowercase().as_str() {
match s.to_ascii_lowercase().as_str() {
"utc" | "u" => Self::Utc,
"local" | "l" => Self::Local,
_ => Self::Error,
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/date/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub fn datetime_in_timezone(
None => Err(ParseErrorKind::OutOfRange),
},
Err(ParseErrorKind::Invalid) => {
if s.to_lowercase() == "local" {
if s.eq_ignore_ascii_case("local") {
Ok(dt.with_timezone(Local::now().offset()))
} else {
let tz: Tz = parse_timezone_internal(s)?;
Expand Down
5 changes: 4 additions & 1 deletion crates/nu-command/src/filters/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use nu_protocol::{
record, Category, Config, Example, IntoInterruptiblePipelineData, IntoPipelineData, ListStream,
PipelineData, Record, ShellError, Signature, Span, SyntaxShape, Type, Value,
};
use nu_utils::IgnoreCaseExt;

#[derive(Clone)]
pub struct Find;
Expand Down Expand Up @@ -318,7 +319,9 @@ fn highlight_terms_in_record_with_search_columns(
}

fn contains_ignore_case(string: &str, substring: &str) -> bool {
string.to_lowercase().contains(&substring.to_lowercase())
string
.to_folded_case()
.contains(&substring.to_folded_case())
}

fn find_with_rest_and_highlight(
Expand Down
41 changes: 19 additions & 22 deletions crates/nu-command/src/filters/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use nu_protocol::{
record, Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData,
Record, ShellError, Signature, Span, Type, Value,
};
use nu_utils::IgnoreCaseExt;
use std::cmp::Ordering;

#[derive(Clone)]
Expand Down Expand Up @@ -220,22 +221,22 @@ fn sort_record(
b.0.clone()
};

// Convert to lowercase if case-insensitive
// Fold case if case-insensitive
let left = if insensitive {
left_res.to_ascii_lowercase()
left_res.to_folded_case()
} else {
left_res
};
let right = if insensitive {
right_res.to_ascii_lowercase()
right_res.to_folded_case()
} else {
right_res
};

if natural {
compare_str(left, right)
} else {
left.partial_cmp(&right).unwrap_or(Ordering::Equal)
left.cmp(&right)
}
});

Expand All @@ -262,28 +263,24 @@ pub fn sort(
let span_a = a.span();
let span_b = b.span();
if insensitive {
let lowercase_left = match a {
Value::String { val, .. } => {
Value::string(val.to_ascii_lowercase(), span_a)
}
let folded_left = match a {
Value::String { val, .. } => Value::string(val.to_folded_case(), span_a),
_ => a.clone(),
};

let lowercase_right = match b {
Value::String { val, .. } => {
Value::string(val.to_ascii_lowercase(), span_b)
}
let folded_right = match b {
Value::String { val, .. } => Value::string(val.to_folded_case(), span_b),
_ => b.clone(),
};

if natural {
match (lowercase_left.as_string(), lowercase_right.as_string()) {
match (folded_left.as_string(), folded_right.as_string()) {
(Ok(left), Ok(right)) => compare_str(left, right),
_ => Ordering::Equal,
}
} else {
lowercase_left
.partial_cmp(&lowercase_right)
folded_left
.partial_cmp(&folded_right)
.unwrap_or(Ordering::Equal)
}
} else if natural {
Expand Down Expand Up @@ -326,23 +323,23 @@ pub fn process(
let result = if insensitive {
let span_left = left_res.span();
let span_right = right_res.span();
let lowercase_left = match left_res {
Value::String { val, .. } => Value::string(val.to_ascii_lowercase(), span_left),
let folded_left = match left_res {
Value::String { val, .. } => Value::string(val.to_folded_case(), span_left),
_ => left_res,
};

let lowercase_right = match right_res {
Value::String { val, .. } => Value::string(val.to_ascii_lowercase(), span_right),
let folded_right = match right_res {
Value::String { val, .. } => Value::string(val.to_folded_case(), span_right),
_ => right_res,
};
if natural {
match (lowercase_left.as_string(), lowercase_right.as_string()) {
match (folded_left.as_string(), folded_right.as_string()) {
(Ok(left), Ok(right)) => compare_str(left, right),
_ => Ordering::Equal,
}
} else {
lowercase_left
.partial_cmp(&lowercase_right)
folded_left
.partial_cmp(&folded_right)
.unwrap_or(Ordering::Equal)
}
} else {
Expand Down
11 changes: 6 additions & 5 deletions crates/nu-command/src/filters/uniq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use nu_protocol::{
record, Category, Example, IntoPipelineData, PipelineData, PipelineMetadata, ShellError,
Signature, Span, Type, Value,
};
use nu_utils::IgnoreCaseExt;
use std::collections::hash_map::IntoIter;
use std::collections::HashMap;

Expand Down Expand Up @@ -172,7 +173,7 @@ impl ValueCounter {
ValueCounter {
val,
val_to_compare: if flag_ignore_case {
clone_to_lowercase(&vals_to_compare.with_span(Span::unknown()))
clone_to_folded_case(&vals_to_compare.with_span(Span::unknown()))
} else {
vals_to_compare.with_span(Span::unknown())
},
Expand All @@ -182,17 +183,17 @@ impl ValueCounter {
}
}

fn clone_to_lowercase(value: &Value) -> Value {
fn clone_to_folded_case(value: &Value) -> Value {
let span = value.span();
match value {
Value::String { val: s, .. } => Value::string(s.clone().to_lowercase(), span),
Value::String { val: s, .. } => Value::string(s.clone().to_folded_case(), span),
Value::List { vals: vec, .. } => {
Value::list(vec.iter().map(clone_to_lowercase).collect(), span)
Value::list(vec.iter().map(clone_to_folded_case).collect(), span)
}
Value::Record { val: record, .. } => Value::record(
record
.iter()
.map(|(k, v)| (k.to_owned(), clone_to_lowercase(v)))
.map(|(k, v)| (k.to_owned(), clone_to_folded_case(v)))
.collect(),
span,
),
Expand Down
4 changes: 2 additions & 2 deletions crates/nu-command/src/generators/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ used as the next argument to the closure, otherwise generation stops.
let mut err = None;

for (k, v) in iter {
if k.to_lowercase() == "out" {
if k.eq_ignore_ascii_case("out") {
out = Some(v);
} else if k.to_lowercase() == "next" {
} else if k.eq_ignore_ascii_case("next") {
next = Some(v);
} else {
let error = ShellError::GenericError(
Expand Down

0 comments on commit 4143be4

Please sign in to comment.