Skip to content

Commit

Permalink
internal/lsp: use the view options, not the session options
Browse files Browse the repository at this point in the history
Previous changes to the config mechanism made the config options
per-view, not per-session. We should now make sure to obey config
changes per-view. This does not fix the configuration handling for
"watchChangedFile" however. This should be done in a future CL.

Change-Id: I73f6236386c36d2587fdb9c0601670833a4366c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194818
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
  • Loading branch information
stamblerre committed Sep 11, 2019
1 parent 63a3583 commit c7d52e4
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 25 deletions.
4 changes: 4 additions & 0 deletions internal/lsp/cache/view.go
Expand Up @@ -117,6 +117,10 @@ func (v *view) Options() source.Options {
return v.options
}

func (v *view) SetOptions(options source.Options) {
v.options = options
}

// Config returns the configuration used for the view's interaction with the
// go/packages API. It is shared across all views.
func (v *view) Config(ctx context.Context) *packages.Config {
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/code_action.go
Expand Up @@ -47,7 +47,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara

// Determine the supported actions for this file kind.
fileKind := f.Handle(ctx).Kind()
supportedCodeActions, ok := s.session.Options().SupportedCodeActions[fileKind]
supportedCodeActions, ok := view.Options().SupportedCodeActions[fileKind]
if !ok {
return nil, fmt.Errorf("no supported code actions for %v file kind", fileKind)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/completion.go
Expand Up @@ -19,7 +19,7 @@ import (
func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) {
uri := span.NewURI(params.TextDocument.URI)
view := s.session.ViewOf(uri)
options := s.session.Options()
options := view.Options()
f, err := getGoFile(ctx, view, uri)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/debug/info.go
Expand Up @@ -20,7 +20,7 @@ const (
)

// Version is a manually-updated mechanism for tracking versions.
var Version = "v0.1.4"
var Version = "v0.1.5"

// This writes the version and environment information to a writer.
func PrintVersionInfo(w io.Writer, verbose bool, mode PrintMode) {
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/diagnostics.go
Expand Up @@ -27,7 +27,7 @@ func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI
if !ok {
return
}
reports, err := source.Diagnostics(ctx, view, gof, s.session.Options().DisabledAnalyses)
reports, err := source.Diagnostics(ctx, view, gof, view.Options().DisabledAnalyses)
if err != nil {
log.Error(ctx, "failed to compute diagnostics", err, telemetry.File)
return
Expand Down
2 changes: 1 addition & 1 deletion internal/lsp/folding_range.go
Expand Up @@ -15,7 +15,7 @@ func (s *Server) foldingRange(ctx context.Context, params *protocol.FoldingRange
if err != nil {
return nil, err
}
ranges, err := source.FoldingRange(ctx, view, f, s.session.Options().LineFoldingOnly)
ranges, err := source.FoldingRange(ctx, view, f, view.Options().LineFoldingOnly)
if err != nil {
return nil, err
}
Expand Down
5 changes: 2 additions & 3 deletions internal/lsp/hover.go
Expand Up @@ -34,15 +34,14 @@ func (s *Server) hover(ctx context.Context, params *protocol.HoverParams) (*prot
if err != nil {
return nil, err
}
contents := s.toProtocolHoverContents(ctx, hover)
contents := s.toProtocolHoverContents(ctx, hover, view.Options())
return &protocol.Hover{
Contents: contents,
Range: &rng,
}, nil
}

func (s *Server) toProtocolHoverContents(ctx context.Context, h *source.HoverInformation) protocol.MarkupContent {
options := s.session.Options()
func (s *Server) toProtocolHoverContents(ctx context.Context, h *source.HoverInformation, options source.Options) protocol.MarkupContent {
content := protocol.MarkupContent{
Kind: options.PreferredContentFormat,
}
Expand Down
39 changes: 23 additions & 16 deletions internal/lsp/lsp_test.go
Expand Up @@ -110,14 +110,14 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) {
}

func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests.CompletionSnippets, items tests.CompletionItems) {
original := r.server.session.Options()
modified := original
defer func() { r.server.session.SetOptions(original) }()
for src, test := range data {
view := r.server.session.ViewOf(src.URI())
original := view.Options()
modified := original

// Set this as a default.
modified.Completion.Documentation = true
// Set this as a default.
modified.Completion.Documentation = true

for src, test := range data {
var want []source.CompletionItem
for _, pos := range test.CompletionItems {
want = append(want, *items[pos])
Expand All @@ -126,7 +126,7 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
modified.Completion.Deep = strings.Contains(string(src.URI()), "deepcomplete")
modified.Completion.FuzzyMatching = strings.Contains(string(src.URI()), "fuzzymatch")
modified.Completion.Unimported = strings.Contains(string(src.URI()), "unimported")
r.server.session.SetOptions(modified)
view.SetOptions(modified)

list := r.runCompletion(t, src)

Expand All @@ -149,15 +149,22 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
t.Errorf("%s: %s", src, msg)
}
}
view.SetOptions(original)
}
modified.InsertTextFormat = protocol.SnippetTextFormat

for _, usePlaceholders := range []bool{true, false} {

for src, want := range snippets {
view := r.server.session.ViewOf(src.URI())
original := view.Options()
modified := original

modified.InsertTextFormat = protocol.SnippetTextFormat
modified.Completion.Deep = strings.Contains(string(src.URI()), "deepcomplete")
modified.Completion.FuzzyMatching = strings.Contains(string(src.URI()), "fuzzymatch")
modified.Completion.Unimported = strings.Contains(string(src.URI()), "unimported")
modified.Completion.Placeholders = usePlaceholders
r.server.session.SetOptions(modified)
view.SetOptions(modified)

list := r.runCompletion(t, src)

Expand All @@ -181,6 +188,7 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
if expected != got.TextEdit.NewText {
t.Errorf("%s: expected snippet %q, got %q", src, expected, got.TextEdit.NewText)
}
view.SetOptions(original)
}
}
}
Expand Down Expand Up @@ -306,16 +314,15 @@ func summarizeCompletionItems(i int, want []source.CompletionItem, got []protoco
}

func (r *runner) FoldingRange(t *testing.T, data tests.FoldingRanges) {
original := r.server.session.Options()
modified := original
defer func() { r.server.session.SetOptions(original) }()

for _, spn := range data {
uri := spn.URI()
view := r.server.session.ViewOf(uri)
original := view.Options()
modified := original

// Test all folding ranges.
modified.LineFoldingOnly = false
r.server.session.SetOptions(modified)
view.SetOptions(modified)
ranges, err := r.server.FoldingRange(r.ctx, &protocol.FoldingRangeParams{
TextDocument: protocol.TextDocumentIdentifier{
URI: protocol.NewURI(uri),
Expand All @@ -329,7 +336,7 @@ func (r *runner) FoldingRange(t *testing.T, data tests.FoldingRanges) {

// Test folding ranges with lineFoldingOnly = true.
modified.LineFoldingOnly = true
r.server.session.SetOptions(modified)
view.SetOptions(modified)
ranges, err = r.server.FoldingRange(r.ctx, &protocol.FoldingRangeParams{
TextDocument: protocol.TextDocumentIdentifier{
URI: protocol.NewURI(uri),
Expand All @@ -340,7 +347,7 @@ func (r *runner) FoldingRange(t *testing.T, data tests.FoldingRanges) {
continue
}
r.foldingRanges(t, "foldingRange-lineFolding", uri, ranges)

view.SetOptions(original)
}
}

Expand Down
7 changes: 6 additions & 1 deletion internal/lsp/source/view.go
Expand Up @@ -245,8 +245,13 @@ type View interface {
// Note: the process env contains cached module and filesystem state.
RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error, opts *imports.Options) error

// Options returns a copy of the ViewOptions for this view.
// Options returns a copy of the Options for this view.
Options() Options

// SetOptions sets the options of this view to new values.
// Warning: Do not use this, unless in a test.
// This function does not correctly invalidate the view when needed.
SetOptions(Options)
}

// File represents a source file of any type.
Expand Down

0 comments on commit c7d52e4

Please sign in to comment.