Skip to content

Commit

Permalink
pylsp: Prefer version from user venv (#21069)
Browse files Browse the repository at this point in the history
Closes #ISSUE

Release Notes:

- pylsp will now use version installed in user venv, if one is
available.
  • Loading branch information
osiewicz authored Nov 24, 2024
1 parent 20bffaf commit e85848a
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 23 deletions.
9 changes: 7 additions & 2 deletions crates/language/src/language.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,14 @@ impl CachedLspAdapter {
pub async fn get_language_server_command(
self: Arc<Self>,
delegate: Arc<dyn LspAdapterDelegate>,
toolchains: Arc<dyn LanguageToolchainStore>,
binary_options: LanguageServerBinaryOptions,
cx: &mut AsyncAppContext,
) -> Result<LanguageServerBinary> {
let cached_binary = self.cached_binary.lock().await;
self.adapter
.clone()
.get_language_server_command(delegate, binary_options, cached_binary, cx)
.get_language_server_command(delegate, toolchains, binary_options, cached_binary, cx)
.await
}

Expand Down Expand Up @@ -281,6 +282,7 @@ pub trait LspAdapter: 'static + Send + Sync {
fn get_language_server_command<'a>(
self: Arc<Self>,
delegate: Arc<dyn LspAdapterDelegate>,
toolchains: Arc<dyn LanguageToolchainStore>,
binary_options: LanguageServerBinaryOptions,
mut cached_binary: futures::lock::MutexGuard<'a, Option<LanguageServerBinary>>,
cx: &'a mut AsyncAppContext,
Expand All @@ -298,7 +300,7 @@ pub trait LspAdapter: 'static + Send + Sync {
// because we don't want to download and overwrite our global one
// for each worktree we might have open.
if binary_options.allow_path_lookup {
if let Some(binary) = self.check_if_user_installed(delegate.as_ref(), cx).await {
if let Some(binary) = self.check_if_user_installed(delegate.as_ref(), toolchains, cx).await {
log::info!(
"found user-installed language server for {}. path: {:?}, arguments: {:?}",
self.name().0,
Expand Down Expand Up @@ -357,6 +359,7 @@ pub trait LspAdapter: 'static + Send + Sync {
async fn check_if_user_installed(
&self,
_: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
None
Expand Down Expand Up @@ -1665,6 +1668,7 @@ impl LspAdapter for FakeLspAdapter {
async fn check_if_user_installed(
&self,
_: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
Some(self.language_server_binary.clone())
Expand All @@ -1673,6 +1677,7 @@ impl LspAdapter for FakeLspAdapter {
fn get_language_server_command<'a>(
self: Arc<Self>,
_: Arc<dyn LspAdapterDelegate>,
_: Arc<dyn LanguageToolchainStore>,
_: LanguageServerBinaryOptions,
_: futures::lock::MutexGuard<'a, Option<LanguageServerBinary>>,
_: &'a mut AsyncAppContext,
Expand Down
1 change: 1 addition & 0 deletions crates/language_extension/src/extension_lsp_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ impl LspAdapter for ExtensionLspAdapter {
fn get_language_server_command<'a>(
self: Arc<Self>,
delegate: Arc<dyn LspAdapterDelegate>,
_: Arc<dyn LanguageToolchainStore>,
_: LanguageServerBinaryOptions,
_: futures::lock::MutexGuard<'a, Option<LanguageServerBinary>>,
_: &'a mut AsyncAppContext,
Expand Down
1 change: 1 addition & 0 deletions crates/languages/src/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ impl super::LspAdapter for CLspAdapter {
async fn check_if_user_installed(
&self,
delegate: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
let path = delegate.which(Self::SERVER_NAME.as_ref()).await?;
Expand Down
1 change: 1 addition & 0 deletions crates/languages/src/go.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl super::LspAdapter for GoLspAdapter {
async fn check_if_user_installed(
&self,
delegate: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
let path = delegate.which(Self::SERVER_NAME.as_ref()).await?;
Expand Down
37 changes: 17 additions & 20 deletions crates/languages/src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ impl LspAdapter for PythonLspAdapter {
async fn check_if_user_installed(
&self,
delegate: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
let node = delegate.which("node".as_ref()).await?;
Expand Down Expand Up @@ -753,33 +754,29 @@ impl LspAdapter for PyLspAdapter {

async fn check_if_user_installed(
&self,
_: &dyn LspAdapterDelegate,
_: &AsyncAppContext,
delegate: &dyn LspAdapterDelegate,
toolchains: Arc<dyn LanguageToolchainStore>,
cx: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
// We don't support user-provided pylsp, as global packages are discouraged in Python ecosystem.
None
let venv = toolchains
.active_toolchain(
delegate.worktree_id(),
LanguageName::new("Python"),
&mut cx.clone(),
)
.await?;
let pylsp_path = Path::new(venv.path.as_ref()).parent()?.join("pylsp");
pylsp_path.exists().then(|| LanguageServerBinary {
path: venv.path.to_string().into(),
arguments: vec![pylsp_path.into()],
env: None,
})
}

async fn fetch_latest_server_version(
&self,
_: &dyn LspAdapterDelegate,
) -> Result<Box<dyn 'static + Any + Send>> {
// let uri = "https://pypi.org/pypi/python-lsp-server/json";
// let mut root_manifest = delegate
// .http_client()
// .get(&uri, Default::default(), true)
// .await?;
// let mut body = Vec::new();
// root_manifest.body_mut().read_to_end(&mut body).await?;
// let as_str = String::from_utf8(body)?;
// let json = serde_json::Value::from_str(&as_str)?;
// let latest_version = json
// .get("info")
// .and_then(|info| info.get("version"))
// .and_then(|version| version.as_str().map(ToOwned::to_owned))
// .ok_or_else(|| {
// anyhow!("PyPI response did not contain version info for python-language-server")
// })?;
Ok(Box::new(()) as Box<_>)
}

Expand Down
1 change: 1 addition & 0 deletions crates/languages/src/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl LspAdapter for RustLspAdapter {
async fn check_if_user_installed(
&self,
delegate: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
let path = delegate.which("rust-analyzer".as_ref()).await?;
Expand Down
1 change: 1 addition & 0 deletions crates/languages/src/vtsls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ impl LspAdapter for VtslsLspAdapter {
async fn check_if_user_installed(
&self,
delegate: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
let env = delegate.shell_env().await;
Expand Down
9 changes: 8 additions & 1 deletion crates/project/src/lsp_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5523,10 +5523,16 @@ impl LspStore {
.unwrap_or_default(),
allow_binary_download,
};
let toolchains = self.toolchain_store(cx);
cx.spawn(|_, mut cx| async move {
let binary_result = adapter
.clone()
.get_language_server_command(delegate.clone(), lsp_binary_options, &mut cx)
.get_language_server_command(
delegate.clone(),
toolchains,
lsp_binary_options,
&mut cx,
)
.await;

delegate.update_status(adapter.name.clone(), LanguageServerBinaryStatus::None);
Expand Down Expand Up @@ -7783,6 +7789,7 @@ impl LspAdapter for SshLspAdapter {
async fn check_if_user_installed(
&self,
_: &dyn LspAdapterDelegate,
_: Arc<dyn LanguageToolchainStore>,
_: &AsyncAppContext,
) -> Option<LanguageServerBinary> {
Some(self.binary.clone())
Expand Down

0 comments on commit e85848a

Please sign in to comment.