CP-1794 Add completions.dart support #151
Conversation
| @@ -101,7 +101,7 @@ Future _run(List<String> args) async { | |||
|
|
|||
| ArgResults env; | |||
| try { | |||
| env = _parser.parse(args); | |||
| env = tryArgsCompletion(args, _parser); | |||
There was a problem hiding this comment.
So does this just try add support for parsing incomplete args? Like if someone were to accidentally run ddev form - it would run ddev format?
There was a problem hiding this comment.
I didn't read through the full source, but it seems like it basically just adds a new command that provides completions. The completion script is then almost entirely generic in that it just runs a special version of ddev itself to get the completions.
|
The one drawback here is that the completions are pretty slow because dart_dev essentially has to run every time it needs to do a completion. Which isn't great since there is quite a bit of "stuff" done at startup. |
|
@georgelesica-wf just pulled it down to test it out, it is pretty slow.. is there anyway we could provide an alternate code path for completion to speed it up? |
|
@evanweible-wf I'll look into it, I agree the (lack of) speed is annoying. |
|
@evanweible-wf upon investigation, it is already short-circuiting for us (https://github.com/kevmoo/completion.dart/blob/master/lib/src/try_completion.dart#L69) so I was wrong about why it is slow. The only setup we're doing before it takes control is registering all our commands with the arg parser, which is necessary. I'm not entirely sure, now, why completion is so slow. |
|
@georgelesica-wf we can revisit performance at a later time - this is definitely the more maintainable approach so I'm good with it. +1 |
|
+1 |
1 similar comment
|
+1 |
|
@jayudey-wf ready for merge |
jayudey-wf
changed the title
|
@jayudey-wf I updated the PR and rebuilt the completion script so it should be unambiguous now as to when the completions will work. Spoiler: only in a project, not globally. |
|
+5 i've verified that it's working as expected with bash but I am currently un-able to verify it's working as expected within zsh |
|
@jayudey-wf I'll test with zsh |
|
+5
@jayudey-wf ready for merge |
|
QA Resource Approval: +1
Merging into master. |
|
+1 |
|
Manual CI on dart 1.15.0
|
This addresses #103. I've added basic support for generating the completion file using completions.dart.
See the README for information on how to consume the new completions.
The script even works with custom project-specific tasks because it operates by adding a pseudo command to ddev that effectively introspects its current configuration. This means the script won't actually need updating all that often. In the future we may not even need to store the script, we may just generate it on the fly from completions.dart.
Code Review / +10
Follow the instructions in the README to get it working. Then make sure it works.
Install globally using
pub global activate -s /path/to/cloneinstead of the command in the README since that would just install the release version. Of particular use would be testing in zsh since I don't use it.The completions only work within a project, see the readme for how to set up the
ddevalias, which is required. Link this branch into a random Dart project, pub upgrade, and then you should have completions when you doddev <tab> <tab> <tab>....Review:
@evanweible-wf
@maxwellpeterson-wf
@jayudey-wf