From 5bef32f3ed6bca11e9824cae475a0b35f5c804c1 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 20 Mar 2025 14:44:07 +0200 Subject: [PATCH] When determining Python task context, do not consider worktree-less files as an error (#27183) Makes Python plugin to output ![image](https://github.com/user-attachments/assets/4960bc48-21b7-4392-82b9-18bfd1dd9cd0) for standalone Python files now, instead of nothing as now. Before the change, no task context was created for the standalone file due to `VariableName::RelativeFile` lookup considered as an error. Now, Zed continues and constructs whatever possible context instead. That `pytest` task seems odd, as the logic fixed here needs a relative path (hence, a worktree) to consider unit tests. We do not have variables at the moment the associated tasks are queried for: https://github.com/zed-industries/zed/blob/14920ab910c6d0208d23ce6b6e2ed644e6f20f2e/crates/languages/src/python.rs#L359-L363 https://github.com/zed-industries/zed/blob/14920ab910c6d0208d23ce6b6e2ed644e6f20f2e/crates/languages/src/python.rs#L417-L446 so we cannot filter this the same way the PR does. Maybe, we can use a `VariableName::RelativeFile` instead of `VariableName::File` there? Release Notes: - Show tasks from Python plugin for standalone files --- crates/languages/src/python.rs | 51 ++++++++++++++-------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/crates/languages/src/python.rs b/crates/languages/src/python.rs index 4365ce6b02f308e1f9bda419dab7f1e024d48062..1219083b0c06a87fa6fb8715463735779f403dd5 100644 --- a/crates/languages/src/python.rs +++ b/crates/languages/src/python.rs @@ -328,14 +328,9 @@ impl ContextProvider for PythonContextProvider { toolchains: Arc, cx: &mut gpui::App, ) -> Task> { - let test_target = { - let test_runner = selected_test_runner(location.buffer.read(cx).file(), cx); - - let runner = match test_runner { - TestRunner::UNITTEST => self.build_unittest_target(variables), - TestRunner::PYTEST => self.build_pytest_target(variables), - }; - runner + let test_target = match selected_test_runner(location.buffer.read(cx).file(), cx) { + TestRunner::UNITTEST => self.build_unittest_target(variables), + TestRunner::PYTEST => self.build_pytest_target(variables), }; let worktree_id = location.buffer.read(cx).file().map(|f| f.worktree_id(cx)); @@ -352,7 +347,9 @@ impl ContextProvider for PythonContextProvider { String::from("python3") }; let toolchain = (PYTHON_ACTIVE_TOOLCHAIN_PATH, active_toolchain); - Ok(task::TaskVariables::from_iter([test_target?, toolchain])) + Ok(task::TaskVariables::from_iter( + test_target.into_iter().chain([toolchain]), + )) }) } @@ -464,10 +461,9 @@ impl PythonContextProvider { fn build_unittest_target( &self, variables: &task::TaskVariables, - ) -> Result<(VariableName, String)> { - let python_module_name = python_module_name_from_relative_path( - variables.get(&VariableName::RelativeFile).unwrap_or(""), - ); + ) -> Option<(VariableName, String)> { + let python_module_name = + python_module_name_from_relative_path(variables.get(&VariableName::RelativeFile)?); let unittest_class_name = variables.get(&VariableName::Custom(Cow::Borrowed("_unittest_class_name"))); @@ -478,28 +474,25 @@ impl PythonContextProvider { let unittest_target_str = match (unittest_class_name, unittest_method_name) { (Some(class_name), Some(method_name)) => { - format!("{}.{}.{}", python_module_name, class_name, method_name) + format!("{python_module_name}.{class_name}.{method_name}") } - (Some(class_name), None) => format!("{}.{}", python_module_name, class_name), + (Some(class_name), None) => format!("{python_module_name}.{class_name}"), (None, None) => python_module_name, - (None, Some(_)) => return Ok((VariableName::Custom(Cow::Borrowed("")), String::new())), // should never happen, a TestCase class is the unit of testing + // should never happen, a TestCase class is the unit of testing + (None, Some(_)) => return None, }; - let unittest_target = ( + Some(( PYTHON_TEST_TARGET_TASK_VARIABLE.clone(), unittest_target_str, - ); - - Ok(unittest_target) + )) } fn build_pytest_target( &self, variables: &task::TaskVariables, - ) -> Result<(VariableName, String)> { - let file_path = variables - .get(&VariableName::RelativeFile) - .ok_or_else(|| anyhow!("No file path given"))?; + ) -> Option<(VariableName, String)> { + let file_path = variables.get(&VariableName::RelativeFile)?; let pytest_class_name = variables.get(&VariableName::Custom(Cow::Borrowed("_pytest_class_name"))); @@ -509,20 +502,18 @@ impl PythonContextProvider { let pytest_target_str = match (pytest_class_name, pytest_method_name) { (Some(class_name), Some(method_name)) => { - format!("{}::{}::{}", file_path, class_name, method_name) + format!("{file_path}::{class_name}::{method_name}") } (Some(class_name), None) => { - format!("{}::{}", file_path, class_name) + format!("{file_path}::{class_name}") } (None, Some(method_name)) => { - format!("{}::{}", file_path, method_name) + format!("{file_path}::{method_name}") } (None, None) => file_path.to_string(), }; - let pytest_target = (PYTHON_TEST_TARGET_TASK_VARIABLE.clone(), pytest_target_str); - - Ok(pytest_target) + Some((PYTHON_TEST_TARGET_TASK_VARIABLE.clone(), pytest_target_str)) } }