Migrate lib/autocomplete.js to src #276
Open
Conversation
This file contains 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
|
I was away this weekend. I'll get around to reviewing this tonight. |
milesj
suggested changes
Aug 8, 2017
| | Array<string> | ||
| | void { | ||
| let result; | ||
| if (_.isArray(str)) { |
| const len = ctx.length; | ||
| const trimmed = ctx.replace(/^\s+/g, ''); | ||
| const matchResult = match(trimmed, data.slice(), options); | ||
| if (_.isArray(matchResult)) { |
| */ | ||
|
|
||
| function assembleInput(input: Object) { | ||
| if (_.isArray(input.context)) { |
| */ | ||
|
|
||
| function getCommandNames(cmds: Array<mixed>) { | ||
| let commands = _.map(cmds, '_name'); |
|
|
||
| function getCommandNames(cmds: Array<mixed>) { | ||
| let commands = _.map(cmds, '_name'); | ||
| commands = [...commands, _.map(cmds, '_aliases')]; |
| * @api private | ||
| */ | ||
|
|
||
| function getSuffix(suffix: string) { |
| * @api private | ||
| */ | ||
|
|
||
| function parseInput(str: string, idx: number): Object { |
There was a problem hiding this comment.
Type the return instead of passing an ambiguous Object.
| * @api private | ||
| */ | ||
|
|
||
| function parseMatchSection(obj: Object) { |
There was a problem hiding this comment.
Missing return. Use a more explicit type than Object.
| * @api private | ||
| */ | ||
|
|
||
| function getCommandNames(cmds: Array<mixed>) { |
There was a problem hiding this comment.
Don't use mixed. Missing return type.
| * @api private | ||
| */ | ||
|
|
||
| function getMatchObject(obj: Object, commands: Array<string>) { |
There was a problem hiding this comment.
Use string[]. Missing return type.
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.
This PR migrates
lib/autocomplete.jsto thesrcfolder, fixing most eslint and flow errors. This file really exposes two functions,execandmatch. I thought it was better to not expose these functions as a class, so I kept the filenameautocomplete.jsinstead ofAutocomplete.js.@milesj
There were a a handful of eslint errors and flow errors I purposely did not fix, because I felt by doing so, it would required changing too much of the logic outside of the scope of this PR.
The previous autocomplete tests tested the
_autocompletefunction exposed onsessionrather than unit testing the individual functionality inautocomplete.js. If it's alright with you, I like to try to get a better understanding ofautocomplete.jsand start writing some tests for it. I could whip up a initial implementation of setting up jest testing next if you don't mind.