-
Notifications
You must be signed in to change notification settings - Fork 79
feat(autocomplete): add show/use completions MONGOSH-563 #829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add support for autocompletion for shell commands, in particular, `show` and `use`. Autocompletion for `use` works similar to autocompletion for `db.<collection>`. The implementation details for use/show autocompletion here have been left to the shell-api package, so that changes to them do not require any autocompletion changes.
const command = splitLineWhitespace[0]; | ||
if (SHELL_COMPLETIONS[command]?.isDirectShellCommand) { | ||
// If we encounter a direct shell commmand, we know that we want completions | ||
// specific to that command, so we set the 'exclusive' flag on the result. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary? Will this also work in the browser?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcasimir Are you referring to the 'exclusive'
flag here?
The mongosh-repl.ts changes have a somewhat more specific context here, but the gist of it is that we run both the Node.js default autocompleter and our own there and combine the results; but the Node.js REPL thinks that it should complete e.g. use a
to use assert
, because it just looks at the a
and finds global variables with the same name.
Will this also work in the browser?
I don’t think the extra array entry should hurt, we’re only using the first entry anyway:
mongosh/packages/browser-runtime-core/src/autocompleter/shell-api-autocompleter.ts
Line 22 in 831220a
const entries = completions[0].map((completion) => { |
(async() => await mongoshCompleter(text))() | ||
]); | ||
this.bus.emit('mongosh:autocompletion-complete'); // For testing. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think I've got a bit better the context. Makes sense.
topologies: ALL_TOPOLOGIES, | ||
serverVersions: ALL_SERVER_VERSIONS | ||
serverVersions: ALL_SERVER_VERSIONS, | ||
isDirectShellCommand: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a custom "autocompleter" for arguments? And this will work with a worker thread as well cause the function will be called inside the runtime right?
Slowly getting it :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a custom "autocompleter" for arguments?
The entry below is, yes.
And this will work with a worker thread as well cause the function will be called inside the runtime right?
I think so, yes. The autocompleter in the browser also lives inside the same environment as e.g. the ShellInternalState
object, and only the caller-facing getCompletions()
method of the runtime is forwarded to the parent/rendering process, so this should work.
deprecated?: boolean; | ||
returnType?: string | TypeSignature; | ||
attributes?: { [key: string]: TypeSignature }; | ||
isDirectShellCommand?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used only for autocompletion or also to set up the command in the various shells? Would be great if we have to have this for it to be the source of truth about that. Hope i've understood what this means.
I think a comment here could help, the concept of "direct command" may not be immediate without context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used only for autocompletion or also to set up the command in the various shells? Would be great if we have to have this for it to be the source of truth about that. Hope i've understood what this means.
The property on the function instance itself is what the shell evaluator uses to determine whether to allow bash-style calls to it, the property on the signature is what the autocompleter uses (like all other properties).
In either case, the source of truth is the @directShellCommand
decorator.
I think a comment here could help, the concept of "direct command" may not be immediate without context.
Good idea, done! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually now that you called it that way .. is "bash-style" command a name that's even more clear? Of course is fine as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, it’s not really bash-specific, it’s like that in most OS shells (even Windows cmd.exe) :)
Add support for autocompletion for shell commands, in particular,
show
anduse
. Autocompletion foruse
works similar toautocompletion for
db.<collection>
.The implementation details for use/show autocompletion here have
been left to the shell-api package, so that changes to them do not
require any autocompletion changes.