Complete module source and version
#1024
Conversation
dbanck
force-pushed
the
f-complete-source-version
branch
from
a2ea679
to
83a1835
Compare
dbanck
force-pushed
the
f-complete-source-version
branch
from
83a1835
to
67d0f41
Compare
dbanck
force-pushed
the
f-complete-source-version
branch
from
8897ba4
to
ff56bd0
Compare
internal/context/context.go
Outdated
| func WithAlgoliaCredentials(ctx context.Context, algoliaAppID, algoliaAPIKey string) context.Context { | ||
| return context.WithValue(ctx, ctxAlgoliaCredentials, algolia.AlgoliaCredentials{ | ||
| AlgoliaAppID: algoliaAppID, | ||
| AlgoliaAPIKey: algoliaAPIKey, | ||
| }) | ||
| } | ||
|
|
||
| func AlgoliaCredentials(ctx context.Context) (algolia.AlgoliaCredentials, bool) { | ||
| credentials, ok := ctx.Value(ctxAlgoliaCredentials).(algolia.AlgoliaCredentials) | ||
| return credentials, ok | ||
| } |
There was a problem hiding this comment.
I know the context package was/is used quite a lot for various things, but it actually represents some technical debt in that a better pattern is keeping the context helpers With* and FromContext within the package which produces the data we end up adding to the context - i.e. within algolia package, so that it's more self-contained.
| } | ||
|
|
||
| for _, mod := range modules { | ||
| if strings.Contains(mod.Path, datadir.DataDirName) { |
There was a problem hiding this comment.
Wouldn't this also match anything.terraformanything? i.e. do we not need to match the filepath.Separator on both sides here?
| continue | ||
| } | ||
| if mod.Path == path.Path { | ||
| // Exclude the current module |
There was a problem hiding this comment.
| // Exclude the current module | |
| // Exclude the module we're providing completion in | |
| // to avoid cyclic references |
| if !strings.HasPrefix(relPath, ".") { | ||
| relPath = fmt.Sprintf("./%s", relPath) | ||
| } |
There was a problem hiding this comment.
Why is this necessary here? 🤔
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Would be nice to also test early cancellation of the request via context.
|
|
||
| for _, mod := range modules { | ||
| if strings.Contains(mod.Path, datadir.DataDirName) { | ||
| // Skip anything from the data directory |
There was a problem hiding this comment.
| // Skip anything from the data directory | |
| // Skip installed module copies in cache directories |
| var algoliaAppID = "" | ||
|
|
||
| // Algolia API key which should be used for searching Terraform registry modules | ||
| var algoliaAPIKey = "" |
There was a problem hiding this comment.
Do you mind updating the GoReleaser config as well to pass these?
I have just added the secrets to the repo as ALGOLIA_API_KEY and ALGOLIA_APP_ID.
|
|
||
| modules, err := h.fetchModulesFromAlgolia(ctx, prefix) | ||
| if err != nil { | ||
| return candidates, err |
There was a problem hiding this comment.
Given that we ignore these errors from hooks upstream https://github.com/hashicorp/hcl-lang/blob/b66451713db4e6e3bcf6cde7d70bd8cb51cc2729/decoder/expression_candidates.go#L278
is it worth logging it here? e.g. by plumbing logger to Hooks and using it here.
| ctx.CompletionHooks["CompleteLocalModuleSources"] = h.LocalModuleSources | ||
| credentials, ok := algolia.CredentialsFromContext(s.srvCtx) | ||
| if ok { | ||
| h.AlgoliaClient = search.NewClient(credentials.AppID, credentials.APIKey) |
There was a problem hiding this comment.
Nitpick: I suppose we could also avoid registering the completion hook if the credentials aren't available?
There was a problem hiding this comment.
Interesting idea. I think the code looks a bit cleaner if we always register the hook and have it exiting early. So it's up to the hook to decide if all requirements are met for a run.
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Closes hashicorp/vscode-terraform#672
This PR adds three hooks for completing module
sourceandversion.CompleteLocalModuleSourcesUses the local indexed modules and offers relative path suggestions, when completing module sources.
CompleteRegistryModuleSourcesSearches the Terraform registry modules via Algolia and fetches the full source address and module description.
For this to work, we now need to pass
main.algoliaAppIDandmain.algoliaAPIKeyvialdflagson build.CompleteRegistryModuleVersionsFetches the available module versions from the Terraform registry.
Deferred work
Completion of required inputs was moved to #1029