mirror of
https://github.com/lotabout/skim.git
synced 2026-09-10 07:16:23 -04:00
fix: path_name_offset returns bytes while Rank::begin is a char index (#1160)
The PathName tiebreak computes path_name_offset - begin, so mixing the two units inflates the score for any path with a non-ASCII directory component and ranks a filename match below a directory match. Co-authored-by: VXNCXNX <VXNCXNX@users.noreply.github.com>
This commit is contained in:
parent
d699733053
commit
e8f5b3ae65
15
src/item.rs
15
src/item.rs
|
|
@ -82,11 +82,18 @@ impl RankBuilder {
|
|||
key
|
||||
}
|
||||
|
||||
/// Computes the byte offset of the first character after the last path separator
|
||||
/// (`/` or `\`) in `text`. Returns `0` when no separator is present.
|
||||
/// Computes the **character** index of the first character after the last path
|
||||
/// separator (`/` or `\`) in `text`. Returns `0` when no separator is present.
|
||||
///
|
||||
/// This must be a char index, not a byte offset: the `PathName` tiebreak
|
||||
/// subtracts it from [`Rank::begin`], which is a char index, so counting bytes
|
||||
/// here would mix units and mis-rank any path with a non-ASCII component.
|
||||
fn path_name_offset(text: &str) -> i32 {
|
||||
text.rfind(['/', '\\'])
|
||||
.map_or(0, |pos| i32::try_from(pos).unwrap_or(i32::MAX).saturating_add(1))
|
||||
text.rfind(['/', '\\']).map_or(0, |pos| {
|
||||
i32::try_from(text[..pos].chars().count())
|
||||
.unwrap_or(i32::MAX)
|
||||
.saturating_add(1)
|
||||
})
|
||||
}
|
||||
|
||||
/// Builds a `Rank` from raw match measurements.
|
||||
|
|
|
|||
|
|
@ -37,6 +37,33 @@ fn build_rank_records_offsets_and_pathname() {
|
|||
assert_eq!(rank.path_name_offset, i32::try_from("src/lib/".len()).unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_name_offset_counts_chars_not_bytes() {
|
||||
// `path_name_offset` is subtracted from `Rank::begin`, which is a char index,
|
||||
// so a multi-byte directory component must not inflate it.
|
||||
let rb = RankBuilder::default();
|
||||
// "ééé/" is 4 chars but 7 bytes; the filename starts at char index 4.
|
||||
let rank = rb.build_rank(0, 4, 5, "ééé/a");
|
||||
assert_eq!(rank.path_name_offset, 4);
|
||||
|
||||
// With the match sitting on the filename, PathName must score it as 0 (best).
|
||||
// A byte-based offset would give 7 - 4 = 3 and rank it below a plain match.
|
||||
assert_eq!(rank.sort_key(&[RankCriteria::PathName])[0], 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pathname_tiebreak_prefers_filename_match_with_non_ascii_dir() {
|
||||
// "ééééé/a" matches in the filename (best), "a/xxxxx" matches in the dir part.
|
||||
let rb = RankBuilder::new(vec![RankCriteria::PathName, RankCriteria::Index]);
|
||||
let in_filename = MatchedItem::new(item("ééééé/a"), rb.build_rank(0, 6, 7, "ééééé/a"), None, &rb);
|
||||
let in_dir = MatchedItem::new(item("a/xxxxx"), rb.build_rank(0, 0, 1, "a/xxxxx"), None, &rb);
|
||||
|
||||
assert!(
|
||||
in_filename < in_dir,
|
||||
"a filename match must outrank a directory match even when the directory is non-ASCII"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sort_key_flips_score_sign() {
|
||||
let rank = Rank {
|
||||
|
|
|
|||
18
tests/cli.rs
18
tests/cli.rs
|
|
@ -183,6 +183,24 @@ fn nth_index_past_i32_does_not_fall_back_to_field_1() {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pathname_tiebreak_is_not_broken_by_a_non_ascii_directory() {
|
||||
// `path_name_offset` used to be a byte offset while `Rank::begin` is a char
|
||||
// index, so a multi-byte directory component inflated the PathName score and
|
||||
// pushed the filename match below the directory match.
|
||||
let (code, stdout, stderr) = run_sk_argv("ééééé/a\na/xxxxx\n", &["-f", "a", "--scheme", "path"], &[]);
|
||||
assert_eq!(code, Some(0), "stderr: {stderr}");
|
||||
assert_eq!(
|
||||
stdout.lines().next(),
|
||||
Some("ééééé/a"),
|
||||
"the filename match must rank first, got: {stdout:?}"
|
||||
);
|
||||
|
||||
// The all-ASCII shape of the same input already ranked correctly; both must agree.
|
||||
let (_, ascii_stdout, _) = run_sk_argv("eeeee/a\na/xxxxx\n", &["-f", "a", "--scheme", "path"], &[]);
|
||||
assert_eq!(ascii_stdout.lines().next(), Some("eeeee/a"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_1_with_output_format() {
|
||||
// --output-format renders the selected item through the printf branch.
|
||||
|
|
|
|||
Loading…
Reference in a new issue