MONGOSH-268: Fix autocompletion in compass-shell #251
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lrlna
suggested changes
Jul 15, 2020
|
|
||
| const connectInfo = getConnectInfo( | ||
| this.uri, | ||
| this.uri ? this.uri : '', |
There was a problem hiding this comment.
We should perhaps add a test for an empty uri in service-provider-core/src/connect-info.spec.ts, just to make sure the rest of the information from this function comes out ok.
There was a problem hiding this comment.
Good call - added a test. I also noticed we didn't have a test script in the service-provider-core package. Is that intended? I added one, commented below.
Anemy
commented
Jul 15, 2020
| "compile-ts": "tsc -p tsconfig.json", | ||
| "prepublish": "npm run compile-ts", | ||
| "test": "mocha -r \"../../scripts/import-expansions.js\" --timeout 60000 --colors -r ts-node/register \"./src/**/*.spec.ts\"", | ||
| "test-ci": "mocha -r \"../../scripts/import-expansions.js\" --timeout 60000 -r ts-node/register \"./src/**/*.spec.ts\"", |
There was a problem hiding this comment.
Didn't see these tests being run with the lerna tests, so I added the test script.
There was a problem hiding this comment.
nice! i don't think this was intentionally left out.
lrlna
approved these changes
Jul 15, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://jira.mongodb.org/browse/MONGOSH-268
Small fix: the uri passed to the service provider is optional, however when we were fetching connection info to have the mongo version for autocomplete we were running string operations on the uri, which were erroring out. Added a small test to catch in future. Was cool to learn more of the inner workings of mongosh.
I think in future we could make this a bit more streamlined by passing the connection information in from compass into the browser repl/shell api internal state, so we can avoid the extra fetching of connection info inside of mongosh.
Auto completion working in compass-shell:
Was thinking about pulling telemetry into this ticket, but since it involves changes in compass metrics also I opened a new ticket and can work on that: https://jira.mongodb.org/browse/COMPASS-4351
Once we merge this we can have a release of mongosh and pull in the new compass-shell version in compass to pull it into the next compass beta release for 1.22.0