Skip to content

feat(wasm): export an autocomplete function #3148

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

Merged
merged 6 commits into from
Jun 6, 2023

Conversation

Codelax
Copy link
Member

@Codelax Codelax commented May 26, 2023

Closes #3111
Closes #3114
Currently not using directly autocomplete as it requires a filled Context, will be done in the future.

The completion tree is built upon each call.

@codecov-commenter
Copy link

codecov-commenter commented May 26, 2023

Codecov Report

Merging #3148 (20ebab1) into master (14b7712) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3148      +/-   ##
==========================================
- Coverage   75.82%   75.78%   -0.05%     
==========================================
  Files         171      173       +2     
  Lines       37374    37719     +345     
==========================================
+ Hits        28340    28585     +245     
- Misses       8029     8103      +74     
- Partials     1005     1031      +26     

see 13 files with indirect coverage changes

words := append(cfg.LeftWords, cfg.SelectedWord)
words = append(words, cfg.RightWords...)

completeCommand := []string{"autocomplete", "complete", "zsh", strconv.FormatInt(indexToComplete, 10), "scw"}
Copy link
Member

@remyleone remyleone May 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could ZSH be in the AutoCompleteConfig? Maybe depending on the different feature of a shell we want to configure them differently? I know that we won't have a different shell in the console but maybe we will have different autocomplete feature depending on the shell we target.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the exported command use the zsh complete command but the best method would be to call directly core.Autocomplete.
As you can see, the current arguments for this javascript function are copied from the Go function prototype.

func AutoComplete(ctx context.Context, leftWords []string, wordToComplete string, rightWords []string) *AutocompleteResponse

But the autocomplete function currently need a working context that is created in core.Bootstrap. To make it work quickly I directly just called the cli command to complete.
Autocomplete need to be refactored to avoid needing a context.

Also all the completion commands (zsh, bash, fish) are called with a different order of argument, that would require more support

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment about this directly in the code to give directions about how to refactor this code if you think it is going to be complex and time-consuming?

@remyleone remyleone added this pull request to the merge queue Jun 6, 2023
Merged via the queue into scaleway:master with commit 563e39b Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wasm: correct typings wasm: autocomplete function
3 participants