Improve matches on command palette

[Imgkl]

Release Notes:

Fixes #8184

Optionally, include screenshots / media showcasing your addition that can be included in the release notes.

https://github.com/zed-industries/zed/assets/25414681/a4682247-f52c-4ab9-a32a-51ab5cf3dbcc

[mrnugget]

Hey! Thanks for opening the PR. I'm a bit concerned that this might change the command palette maybe too much, but maybe we can fix it in a smaller way.

Right now (on main) whitespace is significant in the palette, meaning: <space><space><space> will match entries that contain 3 spaces.

https://github.com/zed-industries/zed/assets/1185253/e29a33f5-6fd5-4da2-96a1-44be0d79ddc2

So maybe we want to keep the behaviour that foo<space>bar<space> matches entries with 2 spaces and foo and bar in it, by changing the code to only trim the start of the query?

[hferreiro]

Right now (on main) whitespace is significant in the palette, meaning: will match entries that contain 3 spaces.

Is that a common use case? I'm more concerned about accidentally typing 2 spaces between words and not finding some entries.

[mrnugget]

Is that a common use case? I'm more concerned about accidentally typing 2 spaces between words and not finding some entries.

Not sure, to be honest, but it does allow you to type words partially and use the space.

Say I want to toggle left dock. Compare:

vs

[Imgkl]

Hey @mrnugget,

Thanks for explaining why in current implementation using whitespace is significant. I checked VS code, Looks like they do something similar to what you explained with "Toggle left" example. I'll tweak my code.

@hferreiro, even I wasn't sure that this was a legit use case, but VS code do this. Since more people are coming into Zed from VSC, it'll be good to keep this interaction similar.

[hferreiro]

I think I explained myself badly. I agree with you. My point was that if I type "tog le" (2 spaces in between tog and le), those 2 spaces should be interpreted as 1; that is, any number of spaces is only meaningful in that they separate words.

[hferreiro]

In summary, I think we should trim spaces at the beginning and at the end of the search text, and also interpret any number of spaces between words as a single separator.

[mrnugget]

Agree with that. .trim() on input and then use

    let re = Regex::new(r"\s+").unwrap();
    let result = re.replace_all(query, " ");

but put the regex into lazy_static! like this:

https://github.com/zed-industries/zed/blob/d48fe0a1320d86bab15a0167418a0d3ae933ccc9/crates/languages/src/go.rs#L31-L33

[Imgkl]

we should trim spaces at the beginning and at the end of the search text, and also interpret any number of spaces between words as a single separator.

Now the changes are inline.

[mrnugget]

Looks like you need to run the formatter:

-            let query = CONSECUTIVE_WHITESPACES.replace_all(&query.trim(), " ").to_string();
+            let query = CONSECUTIVE_WHITESPACES
+                .replace_all(&query.trim(), " ")
+                .to_string();

[TerminalFi]

@mrnugget @Imgkl

From a performance perspective and reducing imports. This manual method I believe is faster

fn trim_consecutive_whitespaces(input: &str) -> String {
    let mut result = String::with_capacity(input.len());
    let mut last_char_was_whitespace = false;

    for char in input.trim().chars() {
        if char.is_whitespace() {
            if !last_char_was_whitespace {
                result.push(' ');
            }
            last_char_was_whitespace = true;
        } else {
            result.push(char);
            last_char_was_whitespace = false;
        }
    }
    result
}

[TerminalFi]

Here is the code used in my testing

100_000 iterations

Regex method took: 979.481709ms Manual method took: 162.163583ms Manual method two took: 459.505292ms

1 iteration Regex method took: 2.933667ms Manual method took: 3.125µs Manual method two took: 8.5µs

use regex::Regex;
use std::time::Instant;

#[macro_use(lazy_static)]
extern crate lazy_static;

lazy_static! {
    static ref CONSECUTIVE_WHITESPACES: Regex = Regex::new(r"\s+").unwrap();
}
// Function using Regex
fn trim_consecutive_whitespaces_regex(input: &str) -> String {
    CONSECUTIVE_WHITESPACES
        .replace_all(input.trim(), " ")
        .to_string()
}

fn trim_consecutive_whitespaces_manual_two(input: &str) -> String {
    input
        .split_whitespace()
        .collect::<Vec<&str>>()
        .join(" ")
}

fn trim_consecutive_whitespaces_manual(input: &str) -> String {
    let mut result = String::with_capacity(input.len());
    let mut last_char_was_whitespace = false;

    for char in input.trim().chars() {
        if char.is_whitespace() {
            if !last_char_was_whitespace {
                result.push(' ');
            }
            last_char_was_whitespace = true;
        } else {
            result.push(char);
            last_char_was_whitespace = false;
        }
    }
    result
}

fn main() {
    let test_str = "This   is a    test string    with  various  whitespaces   ";
    let iterations = 100_000;

    // Benchmark Regex method
    let start = Instant::now();
    for _ in 0..iterations {
        let _ = trim_consecutive_whitespaces_regex(test_str);
    }
    let duration_regex = start.elapsed();

    // Benchmark Manual method
    let start = Instant::now();
    for _ in 0..iterations {
        let _ = trim_consecutive_whitespaces_manual(test_str);
    }
    let duration_manual = start.elapsed();

    // Benchmark Manual method two
    let start = Instant::now();
    for _ in 0..iterations {
        let _ = trim_consecutive_whitespaces_manual_two(test_str);
    }
    let duration_manual_two = start.elapsed();

    println!("Regex method took: {:?}", duration_regex);
    println!("Manual method took: {:?}", duration_manual);
    println!("Manual method two took: {:?}", duration_manual_two);
}

[mrnugget]

Thanks @TerminalFi! That makes sense. Felt a bit unsure about the regex myself. The only "problem" with your manual method I see is that .is_whitespace returns true for more than just , so let's push the char instead of ' ' instead.

[TerminalFi]

@Imgkl Can you update the code to use ?

fn trim_consecutive_whitespaces_manual(input: &str) -> String {
    let mut result = String::with_capacity(input.len());
    let mut last_char_was_whitespace = false;

    for char in input.trim().chars() {
        if char.is_whitespace() {
            if !last_char_was_whitespace {
                result.push(char);
            }
            last_char_was_whitespace = true;
        } else {
            result.push(char);
            last_char_was_whitespace = false;
        }
    }
    result
}

[Imgkl]

@TerminalFi Yeah, pushing the changes in a bit.

Thanks for pointing it out.