[Bash completion]: Respects .file() extensions, uses _filedir #590
Conversation
…ionKind.file()` and uses `_filedir` when available for `.file` and `.directory`.
|
@swift-ci please test |
rauhul
left a comment
There was a problem hiding this comment.
I know I don't have enough experience with bash to say if these changes are the most correct implementation. the general shape looks good to me. If possible could you link to clap's or another language's arg parser script generation that does similar to compare?
|
@rauhul I have to be honest; this is my first time tinkering with Bash completion 😅. Here are the references I used in coming up with this:
|
|
|
||
| return """ | ||
| if declare -F _filedir >/dev/null; then | ||
| \(safeExts.map { "_filedir '\($0)'" }.joined(separator:"\n ")) |
There was a problem hiding this comment.
I think we need an additional call to _filedir -d here – when I run completions on an extension-constrained .file item, it should show me the matching files plus the available directories, since I might need to navigate before finding the correct file.
There was a problem hiding this comment.
bash-3.2$ math stats quantiles --file
CHANGELOG.md CMakeLists.txt CODE_OF_CONDUCT.md CONTRIBUTING.md LICENSE.txt README.md
bash-3.2$ math stats quantiles --file Tes
<no output>
bash-3.2$ math stats quantiles --file Tests/
<auto-completes to:>
bash-3.2$ math stats quantiles --file Tests/CMakeLists.txt
Here, Tests and the other directories should be visible and autocomplete.
There was a problem hiding this comment.
Good catch, done and done! 👍
natecook1000
left a comment
There was a problem hiding this comment.
Thanks so much for this fix, @gwynne! One note about the files with extensions case, otherwise this looks great. 👏🏻👏🏻👏🏻👏🏻
|
@swift-ci please test |
|
@gwynne it looks like some tests need to be updated: if declare -F _filedir >/dev/null; then
_filedir 'txt'
_filedir 'md'
_filedir 'TXT'
_filedir 'MD'
+ _filedir -d
else |
Whoops, sorry! Incoming momentarily 😅 |
|
@swift-ci please test |
natecook1000
left a comment
There was a problem hiding this comment.
Thanks again, @gwynne! 👏🏻👏🏻
…sons learned from apple/swift-argument-parser#590, pretty much just for the hell of it. Deprecate `ConsoleErrror`.
…sons learned from apple/swift-argument-parser#590, pretty much just for the hell of it. Deprecate `ConsoleErrror`.
* Split ConsoleKit into two functional targets - one for terminal interaction, one for command handling, plus an umbrella for compatibility. Also update README, and docs images. Clean up several bits of code. * Apply some of the lessons learned from apple/swift-argument-parser#590, pretty much just for the hell of it. Deprecate `ConsoleErrror`. * Readme cleanup * Make the levenshteinDistance method noticeably more efficient. --------- Co-authored-by: Mahdi Bahrami <github@mahdibm.com> Co-authored-by: Tim Condon <0xTim@users.noreply.github.com>
This PR implements two TODOs left over in the Bash completion script generator code:
CompletionKind.file(extensions:)did not actually honor theextensionsarray. It now does so.CompletionKind.file(extensions:)and.directorynow both preferentially invoke the_filedirshell function frombash-completion(v1 for Bash 3.2 & v2 for Bash 4.2+), if it is available in the shell environment at the time, falling back oncompgenif it is not.Note: I could not find any open issues referring to these concerns.
Checklist