feat: add autocomplete #333
Conversation
kakhaUrigashvili
force-pushed
the
autocomplete
branch
from
6f46a9d
to
1105c06
Compare
kakhaUrigashvili
force-pushed
the
autocomplete
branch
4 times, most recently
from
02cbcdf
to
06b4833
Compare
kakhaUrigashvili
force-pushed
the
autocomplete
branch
from
06b4833
to
a643589
Compare
| this.autoCompleteHintsFile = path.join(__dirname, 'autocomplete-hints.json'); | ||
| } | ||
|
|
||
| _getAutoCompleteOptions() { |
There was a problem hiding this comment.
minor, should this func be renamed to _getAutoCompleteCmd?
There was a problem hiding this comment.
why cmd? it just loads auto complete options tree.
{
smapi: [commandOne, commandTwo...]
...
}
| */ | ||
| setUpAutoComplete() { | ||
| this.reloadAutoCompleteHints(); | ||
| this._withProcessExitDisabled(() => this.completion.setupShellInitFile()); |
There was a problem hiding this comment.
II think we need a try catch to cover this setupShellInitFile function, as suggested by the user https://github.com/f/omelette#automated-install. This step has too many potential problems depending on user's machine.
We need to do the cleanup if it fails, and pop up the message for the failure
There was a problem hiding this comment.
What type of clean up? How can we know what needs to be cleaned up? If this steps fails it will show user the error message from omelette. What extra can we do in try and catch except of rethrowing the error?
There was a problem hiding this comment.
When i run in windows here is the result:
PS D:\Github\ask-cli> ask autocomplete setup
D:\Github\ask-cli\node_modules\omelette\src\omelette.js:189
throw new Error('Shell could not be detected');
^
Error: Shell could not be detected
at Omelette.getActiveShell (D:\Github\ask-cli\node_modules\omelette\src\omelette.js:189:17)
at Omelette.getDefaultShellInitFile (D:\Github\ask-cli\node_modules\omelette\src\omelette.js:210:35)
at Omelette.setupShellInitFile (D:\Github\ask-cli\node_modules\omelette\src\omelette.js:238:42)
at D:\Github\ask-cli\lib\commands\autocomplete\helper.js:52:61
at Helper._withProcessExitDisabled (D:\Github\ask-cli\lib\commands\autocomplete\helper.js:35:9)
at Helper.setUpAutoComplete (D:\Github\ask-cli\lib\commands\autocomplete\helper.js:52:14)
at Command.<anonymous> (D:\Github\ask-cli\lib\commands\autocomplete\index.js:29:20)
at Command.listener (D:\Github\ask-cli\node_modules\commander\index.js:370:29)
at Command.emit (events.js:314:20)
at Command.EventEmitter.emit (domain.js:486:12)
Should we pre-check the OS? And give a good message if it falls through?
| @@ -0,0 +1,29 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
my biggest question would be can we save this registry step each time a new command is created? I know this is a subcommand, but is it still possible to load from the main commander object? Based on the current cmd structure, we only need it for ask ,ask smapi, ask utils, ask skill. Is it possible to iterate the command list in bin folder, and dynamically load them?
There was a problem hiding this comment.
I cannot think of a way. Do you know a way?
|
will need to import these changes into the new ask-cli code base. closing this PR for now issue 17 is still open |
to Add
And the then add the following line to ~/.bash_profile or ~/.bashrc:
Enable
to Remove