From ce223e298cbec0df46921820257e2766bb1d8117 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Wed, 1 Nov 2023 16:21:37 +0100 Subject: [PATCH] Cleanups and TOOOs --- cmds/version.go | 4 ++++ internal/adapter/yamlls/configuration.go | 9 +++++++-- internal/adapter/yamlls/diagnostics.go | 23 +++++++++++++++++++---- internal/adapter/yamlls/hover.go | 3 +++ internal/adapter/yamlls/trimTemplate.go | 1 - internal/adapter/yamlls/yamlls.go | 9 +++++---- internal/handler/completion.go | 8 ++++---- internal/log/log.go | 9 +++++++++ internal/lsp/diagnostics.go | 16 +++++++++++----- internal/lsp/lint.go | 2 +- internal/tree-sitter/gotemplate/README.md | 2 +- 11 files changed, 64 insertions(+), 22 deletions(-) diff --git a/cmds/version.go b/cmds/version.go index 946d8f87..8b145c26 100644 --- a/cmds/version.go +++ b/cmds/version.go @@ -3,9 +3,12 @@ package cmds import ( "fmt" + "github.com/mrjosh/helm-ls/internal/log" "github.com/spf13/cobra" ) +var logger = log.GetLogger() + func newVersionCmd() *cobra.Command { return &cobra.Command{ Use: "version", @@ -20,6 +23,7 @@ func newVersionCmd() *cobra.Command { fmt.Sprintf("Golang: %s", versionInfo.GoVersion), fmt.Sprintf("Compiled by: %s", versionInfo.CompiledBy), ) + logger.Debug("Additional debug info") }, } } diff --git a/internal/adapter/yamlls/configuration.go b/internal/adapter/yamlls/configuration.go index f9ee0985..8d19a08d 100644 --- a/internal/adapter/yamlls/configuration.go +++ b/internal/adapter/yamlls/configuration.go @@ -10,8 +10,13 @@ import ( func handleConfiguration(req jsonrpc2.Request) [5]interface{} { var params lsp.ConfigurationParams if err := json.Unmarshal(req.Params(), ¶ms); err != nil { - logger.Println("Error ") + logger.Error("Error parsing configuration request from yamlls", err) + } + settings := [5]interface{}{YamllsSettings{ + Schemas: map[string]string{"kubernetes": "**"}, + Completion: true, + Hover: true, + }, } - settings := [5]interface{}{YamllsSettings{Schemas: map[string]string{"kubernetes": "**"}, Completion: true, Hover: true}} return settings } diff --git a/internal/adapter/yamlls/diagnostics.go b/internal/adapter/yamlls/diagnostics.go index 142cb7c2..c667c0df 100644 --- a/internal/adapter/yamlls/diagnostics.go +++ b/internal/adapter/yamlls/diagnostics.go @@ -20,10 +20,22 @@ func handleDiagnostics(req jsonrpc2.Request, clientConn jsonrpc2.Conn, documents if !ok { logger.Println("Error handling diagnostic. Could not get document: " + params.URI.Filename()) } - doc.DiagnosticsCache.Yamldiagnostics = filterDiagnostics(params.Diagnostics, doc.Ast, doc.Content) - params.Diagnostics = doc.DiagnosticsCache.GetMergedDiagnostics() + doc.DiagnosticsCache.YamlDiagnostics = filterDiagnostics(params.Diagnostics, doc.Ast, doc.Content) - clientConn.Notify(context.Background(), lsp.MethodTextDocumentPublishDiagnostics, ¶ms) + //TODO: if set to true diagnostics from yamlls will be shown directly when they appear + // this can cause a lot of distraction, as you will get a lot of diagnostics when in the + // middle of typing template blocks + // if set to false diagnostics will only be sent to the client at the same time the + // helm lint diagnostics are sent, which only happens after saving a file + // + // potential problem: the first time we get diagnostics they should always be sent to + // the client + var showYamllsDiagnosticsAlways = true + + if showYamllsDiagnosticsAlways { + params.Diagnostics = doc.DiagnosticsCache.GetMergedDiagnostics() + clientConn.Notify(context.Background(), lsp.MethodTextDocumentPublishDiagnostics, ¶ms) + } } func filterDiagnostics(diagnostics []lsp.Diagnostic, ast *sitter.Tree, content string) (filtered []lsp.Diagnostic) { @@ -48,7 +60,10 @@ func diagnisticIsRelevant(diagnostic lsp.Diagnostic, node *sitter.Node) bool { switch diagnostic.Message { case "Map keys must be unique": return !lsplocal.IsInElseBranch(node) - case "All mapping items must start at the same column", "Implicit map keys need to be followed by map values", "Implicit keys need to be on a single line", "A block sequence may not be used as an implicit map key": + case "All mapping items must start at the same column", + "Implicit map keys need to be followed by map values", + "Implicit keys need to be on a single line", + "A block sequence may not be used as an implicit map key": // TODO: could add a check if is is caused by includes return false diff --git a/internal/adapter/yamlls/hover.go b/internal/adapter/yamlls/hover.go index 319d03c9..dc1e7025 100644 --- a/internal/adapter/yamlls/hover.go +++ b/internal/adapter/yamlls/hover.go @@ -8,6 +8,8 @@ import ( lsp "go.lsp.dev/protocol" ) +// Calls the Completion method of yamlls to get a fitting hover response +// TODO: clarify why the hover method of yamlls can't be used func (yamllsConnector YamllsConnector) CallHover(params lsp.HoverParams, word string) lsp.Hover { if yamllsConnector.Conn == nil { return lsp.Hover{} @@ -20,6 +22,7 @@ func (yamllsConnector YamllsConnector) CallHover(params lsp.HoverParams, word st TextDocumentPositionParams: params.TextDocumentPositionParams, } ) + _, err := (*yamllsConnector.Conn).Call(context.Background(), lsp.MethodTextDocumentCompletion, completionParams, response) if err != nil { return util.BuildHoverResponse(documentation, lsp.Range{}) diff --git a/internal/adapter/yamlls/trimTemplate.go b/internal/adapter/yamlls/trimTemplate.go index ebf0f1df..bb6fd94d 100644 --- a/internal/adapter/yamlls/trimTemplate.go +++ b/internal/adapter/yamlls/trimTemplate.go @@ -6,7 +6,6 @@ import ( func trimTemplateForYamllsFromAst(ast *sitter.Tree, text string) string { var result = []byte(text) - // logger.Println(ast.RootNode()) prettyPrintNode(ast.RootNode(), []byte(text), result) return string(result) } diff --git a/internal/adapter/yamlls/yamlls.go b/internal/adapter/yamlls/yamlls.go index 67406244..00c41670 100644 --- a/internal/adapter/yamlls/yamlls.go +++ b/internal/adapter/yamlls/yamlls.go @@ -17,16 +17,17 @@ type YamllsConnector struct { } func NewYamllsConnector(workingDir string, clientConn jsonrpc2.Conn, documents *lsplocal.DocumentStore) *YamllsConnector { + // TODO: make the path to the executable configureable yamllsCmd := exec.Command("yaml-language-server", "--stdio") stdin, err := yamllsCmd.StdinPipe() if err != nil { - logger.Println("Could not start yaml-language-server, some features may be missing.") + logger.Println("Could not connect to stdin of yaml-language-server, some features may be missing.") return &YamllsConnector{} } stout, err := yamllsCmd.StdoutPipe() if err != nil { - logger.Println("Could not start yaml-language-server, some features may be missing.") + logger.Println("Could not connect to stdout of yaml-language-server, some features may be missing.") return &YamllsConnector{} } @@ -39,13 +40,13 @@ func NewYamllsConnector(workingDir string, clientConn jsonrpc2.Conn, documents * if err != nil { switch e := err.(type) { case *exec.Error: - logger.Println("Could not start yaml-language-server, some features may be missing. Spawning subprocess failed.") + logger.Println("Could not start yaml-language-server, some features may be missing. Spawning subprocess failed.", err) return &YamllsConnector{} case *exec.ExitError: logger.Println("Could not start yaml-language-server, some features may be missing. Command exit rc =", e.ExitCode()) return &YamllsConnector{} default: - logger.Println("Could not start yaml-language-server, some features may be missing. Spawning subprocess failed.") + logger.Println("Could not start yaml-language-server, some features may be missing. Spawning subprocess failed.", err) return &YamllsConnector{} } } diff --git a/internal/handler/completion.go b/internal/handler/completion.go index f9ed82f7..52953d4a 100644 --- a/internal/handler/completion.go +++ b/internal/handler/completion.go @@ -122,17 +122,17 @@ func completionAstParsing(doc *lsplocal.Document, position lsp.Position) (string word string ) - logger.Println("currentNode", currentNode) - logger.Println("relevantChildNode", relevantChildNode.Type()) + logger.Debug("currentNode", currentNode) + logger.Debug("relevantChildNode", relevantChildNode.Type()) switch relevantChildNode.Type() { case gotemplate.NodeTypeIdentifier: word = relevantChildNode.Content([]byte(doc.Content)) case gotemplate.NodeTypeDot: - logger.Println("TraverseIdentifierPathUp") + logger.Debug("TraverseIdentifierPathUp") word = lsplocal.TraverseIdentifierPathUp(relevantChildNode, doc) case gotemplate.NodeTypeDotSymbol: - logger.Println("GetFieldIdentifierPath") + logger.Debug("GetFieldIdentifierPath") word = lsplocal.GetFieldIdentifierPath(relevantChildNode, doc) } return word, nil diff --git a/internal/log/log.go b/internal/log/log.go index 417899b2..870486cb 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -1,6 +1,7 @@ package log import ( + "os" "sync" "github.com/sirupsen/logrus" @@ -8,6 +9,7 @@ import ( type logger interface { Println(args ...interface{}) + Error(args ...interface{}) Debug(args ...interface{}) Printf(format string, args ...interface{}) } @@ -26,5 +28,12 @@ func GetLogger() logger { func createLogger() logger { logger := logrus.New() logger.SetFormatter(&logrus.JSONFormatter{}) + //TODO: make this also configurable with lsp configs + // Check the value of the environment variable + if os.Getenv("LOG_LEVEL") == "debug" { + logger.SetLevel(logrus.DebugLevel) + } else { + logger.SetLevel(logrus.InfoLevel) + } return logger } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 902cf230..a4fd276a 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -3,10 +3,14 @@ package lsp import lsp "go.lsp.dev/protocol" type diagnosticsCache struct { - Yamldiagnostics []lsp.Diagnostic - Helmdiagnostics []lsp.Diagnostic + YamlDiagnostics []lsp.Diagnostic + HelmDiagnostics []lsp.Diagnostic } +// TODO: this should be configurable +// max diagnostics that are shown for a single file +const yamlDiagnosticsLimit = 50 + func NewDiagnosticsCache() diagnosticsCache { return diagnosticsCache{ []lsp.Diagnostic{}, @@ -16,11 +20,13 @@ func NewDiagnosticsCache() diagnosticsCache { func (d diagnosticsCache) GetMergedDiagnostics() (merged []lsp.Diagnostic) { merged = []lsp.Diagnostic{} - for _, diagnostic := range d.Yamldiagnostics { + for _, diagnostic := range d.HelmDiagnostics { merged = append(merged, diagnostic) } - for _, diagnostic := range d.Helmdiagnostics { - merged = append(merged, diagnostic) + for i, diagnostic := range d.YamlDiagnostics { + if i < yamlDiagnosticsLimit { + merged = append(merged, diagnostic) + } } return merged } diff --git a/internal/lsp/lint.go b/internal/lsp/lint.go index b3c29e43..293b4d97 100644 --- a/internal/lsp/lint.go +++ b/internal/lsp/lint.go @@ -30,7 +30,7 @@ func NotifcationFromLint(ctx context.Context, conn jsonrpc2.Conn, doc *Document) if err != nil { return nil, err } - doc.DiagnosticsCache.Helmdiagnostics = diagnostics + doc.DiagnosticsCache.HelmDiagnostics = diagnostics return nil, conn.Notify( ctx, diff --git a/internal/tree-sitter/gotemplate/README.md b/internal/tree-sitter/gotemplate/README.md index a8ad5eda..45f71268 100644 --- a/internal/tree-sitter/gotemplate/README.md +++ b/internal/tree-sitter/gotemplate/README.md @@ -1,6 +1,6 @@ ## Grammar -Taken from https://github.com/ngalaiko/tree-sitter-go-template, or currently this updated branch: https://github.com/msvechla/tree-sitter-go-template/tree/fix_brackets. +Taken from https://github.com/ngalaiko/tree-sitter-go-template, or currently this updated branch: https://github.com/qvalentin/tree-sitter-go-template ## Binding