Skip to content
This repository was archived by the owner on Feb 22, 2020. It is now read-only.

Conversation

@chrismwendt
Copy link
Contributor

@chrismwendt chrismwendt commented Jul 20, 2019

Depends on sourcegraph/code-intel-extensions#124 v7.0.0

The main idea is to decide which way to provide code intel on each request in priority order: LSIF, language server, basic-code-intel.

Test plan:

  • Basic-code-intel only
  • Language server, no LSIF
  • Language server, with LSIF (no data)
  • Language server, with LSIF (with data)
  • LSIF (no data)
  • LSIF (with data)

}

const sendRequest: SendRequest = async ({ rootURI, requestType, request, useCache }) => {
const sendRequest: SendRequest<any> = async ({ rootURI, requestType, request, useCache }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The any here (and elsewhere in this file) doesn't make this less type safe than it was before.

})
)
.subscribe()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

registerWhile was simply moved. Ignore the diff here.

definition: wrapMaybe(definition),
references: wrapMaybe(references),
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

initLSP is just a refactored version of the old activateUsingWebSockets.

definition: handler.definition.bind(handler),
references: handler.references.bind(handler),
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's inside initBasicCodeIntel already existed, I just made a new function out of it.

null
),
})
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the core of this change. The idea is to try LSIF, then language server, then basic-code-intel in that order. If one of them succeeds, stop and use its result.

asyncFirst tries calling each function in the array in turn, stopping when it got a non-undefined result. null is the default value (in case none of the functions returned non-undefined, which is actually impossible because the basicCodeIntel functions always return a result).

@chrismwendt
Copy link
Contributor Author

Although the diff is large, most of it is code that just got moved around. Since the core change is small, the dependent PR got reviewed, and I manually tested a bunch of configurations, I feel confident merging this before getting review. Feel free to still review, we can improve things further later.

@chrismwendt chrismwendt merged commit 0a16ba3 into master Jul 20, 2019
@chrismwendt chrismwendt deleted the lsif branch July 20, 2019 05:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants