Skip to content

LSP support for multifile spec - #868

Open
Marat-Tim wants to merge 2 commits into
daveshanley:mainfrom
Marat-Tim:multifile-lsp
Open

LSP support for multifile spec#868
Marat-Tim wants to merge 2 commits into
daveshanley:mainfrom
Marat-Tim:multifile-lsp

Conversation

@Marat-Tim

Copy link
Copy Markdown
Contributor

Previously, LSP worked on each file separately. That is, we opened a file, and only the opened file was linted. This doesn't work with multi-file specs, because externalized parts of the spec are often invalid without context. In this pull request, I'm adding the ability to specify a primary file that will always be linted when calling language-server. Next, we determine the user's current file and send only those errors to LSP that belong to the current file.

@Marat-Tim

Copy link
Copy Markdown
Contributor Author

@daveshanley

@daveshanley

Copy link
Copy Markdown
Owner

a new release is coming, will review and try and include if this looks good.

Comment thread language-server/server.go
var content []byte
if s.lintRequest.MainSpecPath != "" {
specFileName = s.lintRequest.MainSpecPath
reportLoadResult, _ := loader.LoadFileAsReportOrSpecWithClient(specFileName, s.httpClient)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ignores the loader error and then dereferences reportLoadResult. If the root spec is missing, unreadable, or a remote fetch fails, the LSP will panic during diagnostics.

Comment thread language-server/server.go

var specFileName string
var content []byte
if s.lintRequest.MainSpecPath != "" {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Stops using the editor buffer whenever MainSpecPath is set and reloads the primary spec from disk every time.

This breaks normal LSP behavior, any unsaved edits in the main spec or a referenced file will not affect diagnostics until the file is saved.

Comment thread cmd/language_server.go
mainSpecPath = ""
} else {
var err error
mainSpecPath, err = filepath.Abs(args[0])

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If someone passes a remote spec URL, it becomes a bogus local path. This is despite the loader/http-client changes supporting remote loading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants