nushell: str downcase deprecation warning in completion.nu

[tsssni]

Checklist

• I have read through the manual page (man fzf)
• I have searched through the existing issues
• For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.73.1

OS

• Linux
• macOS
• Windows
• Etc.

Shell

• bash
• zsh
• fish

Problem / Steps to reproduce

Warning: nu::parser::deprecated

  ⚠ Command deprecated.
     ╭─[/nix/store/vxr7g2i5vcaasw1hr8baxjig55l83kgg-nushell-fzf-integration.nu:288:41]
 287 │       $ssh_configs | append $ssh_configs_d | append $ssh_config_global
 288 │                    | where {|it| ($it | str downcase | str starts-with 'host') or ($it | str downcase | str starts-with 'hostname') }
     ·                                         ──────┬─────
     ·                                               ╰── str downcase was deprecated in 0.114.0 and will be removed in a future release.
 289 │                    | parse --regex '^\s*host(?:name)?\s+(?<hosts>.+)' # Extract hosts after keyword
     ╰────
  help: Use `str lowercase` instead.

[junegunn]

/ping @sim590

[sim590]

I see that this happened in 0.114.0. However, there is also some annoying bug right now with 0.114.0. I was able to test the fix to lowercase instead and it works. However, do we want to force the change to lowercase or do we want to use the appropriate command depending on the version? The latter approach would make the switch to 0.114.0 easier for anybody using the nushell FZF integration.

The only implementation I managed to make work is this one:

let __fzf_str_lowercase_impl = if ((version | get major) == 0 and (version | get minor) < 114) {
  "str downcase"
} else {
  "str lowercase"
}

def __fzf_str_lowercase []: string -> string {
  $in | nu --commands $"\"($in)\" | ($__fzf_str_lowercase_impl)"
}

However, this calls a subshell each time we call this function, so it slows it down a bit. Note that this is only used in the ssh completion code.

Writing a closure with variable definition doesn't work since the closure would need to compile in both versions, but it can't.

I don't like either of the solutions. But I'm guessing that the simplest would be best. Just simply substitute downcase with lowercase and may be tell the user to update to Nushell 0.114.0 to make it work.

@junegunn: do you agree?

[sim590]

Substituting for lowercase with user message could look something like this for a Nushell<0.114.0 user:

Error: nu::parser::extra_positional

  × Extra positional argument.
     ╭─[/home/simon/.config/nushell/autoload/_fzf_integration.nu:287:45]
 286 │                      # NOTE: str lowercase is new from Nushell 0.114.0. Please upgrade to Nushell >=0.114.0 if you are seeing `extra positional argument` error regarding `str lowercase` in this block.
 287 │                    | where {|it| ($it | str lowercase | str starts-with 'host') or ($it | str lowercase | str starts-with 'hostname') }
     ·                                             ────┬────
     ·                                                 ╰── extra positional argument
 288 │                    | parse --regex '^\s*host(?:name)?\s+(?<hosts>.+)' # Extract hosts after keyword
     ╰────
  help: Usage: str

Notice the # NOTE comment that is taken straight from the file by the compiler. I don't think there's another way to show an error message since the compiler makes a first pass before executing anything in the file and that is when it displays the error. Any attempt to use an instruction to display a message will not work. So, the comment is just a little hack I found to do it...

I implemented it in 4f7b79c, but I can't open a PR since I'm not a contributorcollaborator.

[junegunn]

Note that this is only used in the ssh completion code.

Given this, I'd prefer the simpler approach, let me cherry-pick your commit. Thanks!

[junegunn]

Oh, the error message always appears during load if you have an old version of nushell, not only when you use ssh completion.

Then, I think we should take the __fzf_str_lowercase_impl approach you showed above. The extra delay only applies to ssh completion. If you push the fix to your fork, I'll cherry-pick it.

[sim590]

OK. I implemented it in 4ea65a3.

NOTE: On my machine, executing the function yields this run duration for 38 ssh addresses:

>: timeit { __fzf_list_hosts }
1sec 290ms 502µs 637ns

compared to the original:

>: timeit { __fzf_list_hosts }
7ms 186µs 953ns

[junegunn]

1sec 290ms

Thanks, but whoa, I didn't expect it to be that slow. I thought it would only add a few tens of milliseconds. A full second is a deal-breaker, so I'll stick with the previous one.

[junegunn]

Merged to master, thanks!