lsp/templating: gracefully handle unknown root #1171
Conversation
| // if there are no parsed modules in the cache, then there is | ||
| // no need to run the aggregate report. This can happen if the | ||
| // server is very slow to start up. | ||
| if len(l.cache.GetAllModules()) == 0 { |
There was a problem hiding this comment.
This is unrelated, but is something that I noticed when working with empty files, a better check here is to test if there are modules rather than if there are files since empty files have no parse.
| @@ -124,6 +116,73 @@ func TestTemplateContentsForFile(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestTemplateContentsForFileInWorkspaceRoot(t *testing.T) { | |||
There was a problem hiding this comment.
Root is not allowed and is an error which can be silently ignored if the caller understands it.
| } | ||
| } | ||
|
|
||
| func TestTemplateContentsForFileWithUnknownRoot(t *testing.T) { |
There was a problem hiding this comment.
When the root is unknown, but there is a path, we can attempt to use the server's root URI as a root instead.
internal/lsp/server.go
Outdated
| @@ -989,7 +989,9 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) { | |||
| // determine the new contents for the file, if permitted | |||
| newContents, err := l.templateContentsForFile(job.URI) | |||
| if err != nil { | |||
| l.logError(fmt.Errorf("failed to template new file: %w", err)) | |||
| if !errors.Is(err, &templatingInRootError{}) { | |||
| l.logError(fmt.Errorf("failed to template new file: %w", err)) | |||
There was a problem hiding this comment.
Is it really an error unless it's something the user asked for and didn't get? If I saw this in the logs, I would probably think I did something wrong and that I should look into how to fix it.
There was a problem hiding this comment.
I've opted to not use the error for control flow here now: 0936ca7b7827ed2fc91a93a3b00bbccf7556e3be
While it's still an error, the callers of the function are now expected to test if the file is in the root and avoid calling the function accordingly.
internal/lsp/server_template_test.go
Outdated
|
|
||
| err = os.WriteFile(filepath.Join(td, ".regal/config.yaml"), []byte{}, 0o600) | ||
| if err != nil { | ||
| t.Fatalf("failed to create directory %s: %s", filepath.Join(td, ".regal"), err) |
There was a problem hiding this comment.
This is not creating a directory
charlieegan3
force-pushed
the
templating-no-root
branch
from
4d95123
to
f44021f
Compare
This also disables templating for files in the root. Fixes StyraInc#1164 Related to StyraInc#1141, but this needs more work Signed-off-by: Charlie Egan <charlie@styra.com>
Signed-off-by: Charlie Egan <charlie@styra.com>
Signed-off-by: Charlie Egan <charlie@styra.com>
charlieegan3
force-pushed
the
templating-no-root
branch
from
f44021f
to
baed864
Compare
This also disables templating for files in the root.
Fixes #1164
Related to #1141, but this needs more work
Sorry for the low res video...
Screen.Recording.2024-10-03.at.18.16.04.mov