Details
These all now seem to do the right thing:
arc we<tab> -> arc weld (+space) arc weld fo<tab> -> arc weld foo\ bar (+space) arc weld 'fo<tab> -> arc weld 'foo bar' (+space) arc weld src<tab> -> arc weld src/ (no space) arc weld src/w<tab> -> arc weld src/work (no space) arc weld src/work<tab><tab> -> suggests "workflow/", "workingcopy/" arc shell-complete --gen<tab> -> arc shell-complete --generate
These also work, which I think is nice-to-have:
arc WEL<tab> -> arc weld arc shell-complete --GEN<tab> -> arc shell-complete --generate
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 20897 Build 28412: Run Core Tests
Event Timeline
| src/toolset/workflow/ArcanistShellCompleteWorkflow.php | ||
|---|---|---|
| 509–514 | We now do need to do this ourselves (see below) so I threw this comment away. | |
| 599 | I've replaced "FILE\n" with slightly more explicit magic, but it's still magic. | |
| 603–605 | Instead of returning magic, we can just return nothing, so this is a slight simplification. | |
| 613 | We're no longer using compgen -W ... (my goal in removing it is to reduce our reliance on Bash magic) so we now need to do our own matching in all cases. We were doing it in most cases anyway already so this isn't a large change. | |
| support/shell/templates/bash-template.sh | ||
| 5–9 | We need to quote ${COMP_WORDS[@]} or foo\ bar gets passed to shell-complete as two separate arguments. Otherwise, I just wrapped this so it looks a little nicer. | |
| 15–17 | See T9116 for more details on this. The new code is basically the same as the old code, except I've replaced -f with the more explicit -A file (both have the same meaning, -A file is just more explicit and easier to find in the documentation) and am replacing the result instead of turning it into an array here, so we can do the "turn it into an array" part later on in one place. At least on my machine, I can't get the correct behavior from this command unless we run compgen from the script instead of from PHP. I'd prefer to just run compgen ... from PHP over returning "<compgen:file>" as a magic string, but couldn't get it to work. | |
| 19–20 | This is probably the most important part in getting the behavior correct, and means: $COMPREPLY = explode("\n", $RESULT);The previous behavior was to explode on any whitespace, without regard for quotes, so a\ b would become two tokens. IFS is "internal field separator", a piece of bash dark magic. | |
I definitely exhaustively tested this code and 100% understand it completely.
But seriously, from reading the comments on T9116, this looks like brain poison and I'm not prepared to forget all of college calculus just to understand the depths of how bash shell completion works. Your test plan looks pretty exhaustive, and I will bookmark this revision for a long plane flight.
Yeah, I basically spent six hours prodding at Bash to ultimately add one line of local IFS=$'\n' and a pair of quotation marks. I feel no wiser for the experience.