clap icon indicating copy to clipboard operation
clap copied to clipboard

WIP: fix(complete): escaping in zsh completion

Open pseyfert opened this issue 2 years ago • 1 comments

I basically went through the reproducer from https://github.com/clap-rs/clap/issues/1596 and tried to narrow down which characters cause issues in:

fn main() {                                                                                                                 
    let values = (0u8..128u8)                             
        .map(|i| i as char)                                                                                           
        .filter(|c| !c.is_ascii_control())                                                                            
        .map(|c| c.to_string())                                                                                       
        .collect::<Vec<_>>();                                                                                        
                                                                                                                      
    let mut app = Command::new("clap-zsh-completions-repro");                                                         
                       
    app = app.arg(Arg::new("kuku").required(true).possible_values(values.iter().map(|v| PossibleValue::new(v).help("crazy char")).collect::<Vec<PossibleValue>>())); // without tooltip
    // app = app.arg(Arg::new("kuku").required(true).possible_values(values.iter().map(|v| PossibleValue::new(v)).collect::<Vec<PossibleValue>>())); // with tooltip
    
    app.set_bin_name("my_app");                                                                                       
    generate(Zsh, &mut app, "my_app", &mut io::stdout());                                                             
}   

turns out, there are a few edge cases (due to the tool tip handling) that need different escaping in the two versions. In a few cases (see comment) escaping is only needed in without-tooltip, but doesn't lead to wrong suggestions with-tooltip, so i didn't add a distinction. Note: i only went through what comes in with .possible_values. (at time of writing). I didn't check what happens to characters in PossibleValue::help or other strings that get written to completion functions

from the top of my head, that could be

  • Arg(...).value_name[s]
  • Arg(...).help
  • Arg(...).long

I'm a tad surprised that the escaping for single quote that was already in the code base didn't work for me …

Marking as WIP because I'm wondering: should we pursue this further? is fancy characters just a won't-fix? do we want to cover the other user-defined strings exhaustively before merging?

pseyfert avatar May 09 '22 22:05 pseyfert

Marking as WIP because I'm wondering: should we pursue this further? is fancy characters just a won't-fix? do we want to cover the other user-defined strings exhaustively before merging?

I'd be more comfortable with this if we could say why certain characters are allowed in some positions but not others and made the design centered on that, rather than on trial and error. It'd help make the code clearer if nothing else.

epage avatar May 10 '22 20:05 epage

Without updates from the author, I'm going ahead and closing this out.

epage avatar Jan 03 '23 16:01 epage