@@ -2,7 +2,7 @@
use std::{
any::TypeId,
- cmp,
+ cmp::{self, Reverse},
path::{Path, PathBuf},
sync::Arc,
};
@@ -11,7 +11,7 @@ use collections::{HashMap, VecDeque};
use gpui::{AppContext, Context, Model, ModelContext, Subscription};
use itertools::{Either, Itertools};
use language::Language;
-use task::{ResolvedTask, TaskContext, TaskId, TaskSource, TaskTemplate};
+use task::{ResolvedTask, TaskContext, TaskId, TaskSource, TaskTemplate, VariableName};
use util::{post_inc, NumericPrefixWithSuffix};
use worktree::WorktreeId;
@@ -198,7 +198,7 @@ impl Inventory {
&self,
language: Option<Arc<Language>>,
worktree: Option<WorktreeId>,
- task_context: TaskContext,
+ task_context: &TaskContext,
cx: &mut AppContext,
) -> (
Vec<(TaskSourceKind, ResolvedTask)>,
@@ -242,7 +242,7 @@ impl Inventory {
.chain(language_tasks)
.filter_map(|(kind, task)| {
let id_base = kind.to_id_base();
- Some((kind, task.resolve_task(&id_base, task_context.clone())?))
+ Some((kind, task.resolve_task(&id_base, task_context)?))
})
.map(|(kind, task)| {
let lru_score = task_usage
@@ -299,22 +299,14 @@ fn task_lru_comparator(
(kind_b, task_b, lru_score_b): &(TaskSourceKind, ResolvedTask, u32),
) -> cmp::Ordering {
lru_score_a
+ // First, display recently used templates above all.
.cmp(&lru_score_b)
+ // Then, ensure more specific sources are displayed first.
.then(task_source_kind_preference(kind_a).cmp(&task_source_kind_preference(kind_b)))
- .then(
- kind_a
- .worktree()
- .is_none()
- .cmp(&kind_b.worktree().is_none()),
- )
- .then(kind_a.worktree().cmp(&kind_b.worktree()))
- .then(
- kind_a
- .abs_path()
- .is_none()
- .cmp(&kind_b.abs_path().is_none()),
- )
- .then(kind_a.abs_path().cmp(&kind_b.abs_path()))
+ // After that, display first more specific tasks, using more template variables.
+ // Bonus points for tasks with symbol variables.
+ .then(task_variables_preference(task_a).cmp(&task_variables_preference(task_b)))
+ // Finally, sort by the resolved label, but a bit more specifically, to avoid mixing letters and digits.
.then({
NumericPrefixWithSuffix::from_numeric_prefixed_str(&task_a.resolved_label)
.cmp(&NumericPrefixWithSuffix::from_numeric_prefixed_str(
@@ -333,6 +325,15 @@ fn task_source_kind_preference(kind: &TaskSourceKind) -> u32 {
}
}
+fn task_variables_preference(task: &ResolvedTask) -> Reverse<usize> {
+ let task_variables = task.substituted_variables();
+ Reverse(if task_variables.contains(&VariableName::Symbol) {
+ task_variables.len() + 1
+ } else {
+ task_variables.len()
+ })
+}
+
#[cfg(test)]
mod test_inventory {
use gpui::{AppContext, Context as _, Model, ModelContext, TestAppContext};
@@ -421,12 +422,12 @@ mod test_inventory {
let (used, current) = inventory.used_and_current_resolved_tasks(
None,
worktree,
- TaskContext::default(),
+ &TaskContext::default(),
cx,
);
used.into_iter()
.chain(current)
- .map(|(_, task)| task.original_task.label)
+ .map(|(_, task)| task.original_task().label.clone())
.collect()
})
}
@@ -445,7 +446,7 @@ mod test_inventory {
let id_base = task_source_kind.to_id_base();
inventory.task_scheduled(
task_source_kind.clone(),
- task.resolve_task(&id_base, TaskContext::default())
+ task.resolve_task(&id_base, &TaskContext::default())
.unwrap_or_else(|| panic!("Failed to resolve task with name {task_name}")),
);
});
@@ -460,7 +461,7 @@ mod test_inventory {
let (used, current) = inventory.used_and_current_resolved_tasks(
None,
worktree,
- TaskContext::default(),
+ &TaskContext::default(),
cx,
);
let mut all = used;
@@ -1,13 +1,15 @@
use std::path::PathBuf;
use anyhow::{bail, Context};
-use collections::HashMap;
+use collections::{HashMap, HashSet};
use schemars::{gen::SchemaSettings, JsonSchema};
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use util::{truncate_and_remove_front, ResultExt};
-use crate::{ResolvedTask, SpawnInTerminal, TaskContext, TaskId, ZED_VARIABLE_NAME_PREFIX};
+use crate::{
+ ResolvedTask, SpawnInTerminal, TaskContext, TaskId, VariableName, ZED_VARIABLE_NAME_PREFIX,
+};
/// A template definition of a Zed task to run.
/// May use the [`VariableName`] to get the corresponding substitutions into its fields.
@@ -78,30 +80,64 @@ impl TaskTemplate {
///
/// Every [`ResolvedTask`] gets a [`TaskId`], based on the `id_base` (to avoid collision with various task sources),
/// and hashes of its template and [`TaskContext`], see [`ResolvedTask`] fields' documentation for more details.
- pub fn resolve_task(&self, id_base: &str, cx: TaskContext) -> Option<ResolvedTask> {
+ pub fn resolve_task(&self, id_base: &str, cx: &TaskContext) -> Option<ResolvedTask> {
if self.label.trim().is_empty() || self.command.trim().is_empty() {
return None;
}
- let TaskContext {
- cwd,
- task_variables,
- } = cx;
- let task_variables = task_variables.into_env_variables();
+
+ let mut variable_names = HashMap::default();
+ let mut substituted_variables = HashSet::default();
+ let task_variables = cx
+ .task_variables
+ .0
+ .iter()
+ .map(|(key, value)| {
+ let key_string = key.to_string();
+ if !variable_names.contains_key(&key_string) {
+ variable_names.insert(key_string.clone(), key.clone());
+ }
+ (key_string, value.as_str())
+ })
+ .collect::<HashMap<_, _>>();
let truncated_variables = truncate_variables(&task_variables);
let cwd = match self.cwd.as_deref() {
- Some(cwd) => Some(substitute_all_template_variables_in_str(
- cwd,
- &task_variables,
- )?),
+ Some(cwd) => {
+ let substitured_cwd = substitute_all_template_variables_in_str(
+ cwd,
+ &task_variables,
+ &variable_names,
+ &mut substituted_variables,
+ )?;
+ Some(substitured_cwd)
+ }
None => None,
}
.map(PathBuf::from)
- .or(cwd);
- let shortened_label =
- substitute_all_template_variables_in_str(&self.label, &truncated_variables)?;
- let full_label = substitute_all_template_variables_in_str(&self.label, &task_variables)?;
- let command = substitute_all_template_variables_in_str(&self.command, &task_variables)?;
- let args = substitute_all_template_variables_in_vec(self.args.clone(), &task_variables)?;
+ .or(cx.cwd.clone());
+ let shortened_label = substitute_all_template_variables_in_str(
+ &self.label,
+ &truncated_variables,
+ &variable_names,
+ &mut substituted_variables,
+ )?;
+ let full_label = substitute_all_template_variables_in_str(
+ &self.label,
+ &task_variables,
+ &variable_names,
+ &mut substituted_variables,
+ )?;
+ let command = substitute_all_template_variables_in_str(
+ &self.command,
+ &task_variables,
+ &variable_names,
+ &mut substituted_variables,
+ )?;
+ let args = substitute_all_template_variables_in_vec(
+ &self.args,
+ &task_variables,
+ &variable_names,
+ &mut substituted_variables,
+ )?;
let task_hash = to_hex_hash(&self)
.context("hashing task template")
@@ -110,10 +146,16 @@ impl TaskTemplate {
.context("hashing task variables")
.log_err()?;
let id = TaskId(format!("{id_base}_{task_hash}_{variables_hash}"));
- let mut env = substitute_all_template_variables_in_map(self.env.clone(), &task_variables)?;
- env.extend(task_variables);
+ let mut env = substitute_all_template_variables_in_map(
+ &self.env,
+ &task_variables,
+ &variable_names,
+ &mut substituted_variables,
+ )?;
+ env.extend(task_variables.into_iter().map(|(k, v)| (k, v.to_owned())));
Some(ResolvedTask {
id: id.clone(),
+ substituted_variables,
original_task: self.clone(),
resolved_label: full_label.clone(),
resolved: Some(SpawnInTerminal {
@@ -134,7 +176,7 @@ impl TaskTemplate {
const MAX_DISPLAY_VARIABLE_LENGTH: usize = 15;
-fn truncate_variables(task_variables: &HashMap<String, String>) -> HashMap<String, String> {
+fn truncate_variables(task_variables: &HashMap<String, &str>) -> HashMap<String, String> {
task_variables
.iter()
.map(|(key, value)| {
@@ -153,25 +195,29 @@ fn to_hex_hash(object: impl Serialize) -> anyhow::Result<String> {
Ok(hex::encode(hasher.finalize()))
}
-fn substitute_all_template_variables_in_str(
+fn substitute_all_template_variables_in_str<A: AsRef<str>>(
template_str: &str,
- task_variables: &HashMap<String, String>,
+ task_variables: &HashMap<String, A>,
+ variable_names: &HashMap<String, VariableName>,
+ substituted_variables: &mut HashSet<VariableName>,
) -> Option<String> {
- let substituted_string = shellexpand::env_with_context(&template_str, |var| {
+ let substituted_string = shellexpand::env_with_context(template_str, |var| {
// Colons denote a default value in case the variable is not set. We want to preserve that default, as otherwise shellexpand will substitute it for us.
let colon_position = var.find(':').unwrap_or(var.len());
let (variable_name, default) = var.split_at(colon_position);
- let append_previous_default = |ret: &mut String| {
- if !default.is_empty() {
- ret.push_str(default);
+ if let Some(name) = task_variables.get(variable_name) {
+ if let Some(substituted_variable) = variable_names.get(variable_name) {
+ substituted_variables.insert(substituted_variable.clone());
}
- };
- if let Some(mut name) = task_variables.get(variable_name).cloned() {
+
+ let mut name = name.as_ref().to_owned();
// Got a task variable hit
- append_previous_default(&mut name);
+ if !default.is_empty() {
+ name.push_str(default);
+ }
return Ok(Some(name));
} else if variable_name.starts_with(ZED_VARIABLE_NAME_PREFIX) {
- bail!("Unknown variable name: {}", variable_name);
+ bail!("Unknown variable name: {variable_name}");
}
// This is an unknown variable.
// We should not error out, as they may come from user environment (e.g. $PATH). That means that the variable substitution might not be perfect.
@@ -187,24 +233,44 @@ fn substitute_all_template_variables_in_str(
}
fn substitute_all_template_variables_in_vec(
- mut template_strs: Vec<String>,
- task_variables: &HashMap<String, String>,
+ template_strs: &[String],
+ task_variables: &HashMap<String, &str>,
+ variable_names: &HashMap<String, VariableName>,
+ substituted_variables: &mut HashSet<VariableName>,
) -> Option<Vec<String>> {
- for variable in template_strs.iter_mut() {
- let new_value = substitute_all_template_variables_in_str(&variable, task_variables)?;
- *variable = new_value;
+ let mut expanded = Vec::with_capacity(template_strs.len());
+ for variable in template_strs {
+ let new_value = substitute_all_template_variables_in_str(
+ variable,
+ task_variables,
+ variable_names,
+ substituted_variables,
+ )?;
+ expanded.push(new_value);
}
- Some(template_strs)
+ Some(expanded)
}
fn substitute_all_template_variables_in_map(
- keys_and_values: HashMap<String, String>,
- task_variables: &HashMap<String, String>,
+ keys_and_values: &HashMap<String, String>,
+ task_variables: &HashMap<String, &str>,
+ variable_names: &HashMap<String, VariableName>,
+ substituted_variables: &mut HashSet<VariableName>,
) -> Option<HashMap<String, String>> {
let mut new_map: HashMap<String, String> = Default::default();
for (key, value) in keys_and_values {
- let new_value = substitute_all_template_variables_in_str(&value, task_variables)?;
- let new_key = substitute_all_template_variables_in_str(&key, task_variables)?;
+ let new_value = substitute_all_template_variables_in_str(
+ &value,
+ task_variables,
+ variable_names,
+ substituted_variables,
+ )?;
+ let new_key = substitute_all_template_variables_in_str(
+ &key,
+ task_variables,
+ variable_names,
+ substituted_variables,
+ )?;
new_map.insert(new_key, new_value);
}
Some(new_map)
@@ -246,7 +312,7 @@ mod tests {
},
] {
assert_eq!(
- task_with_blank_property.resolve_task(TEST_ID_BASE, TaskContext::default()),
+ task_with_blank_property.resolve_task(TEST_ID_BASE, &TaskContext::default()),
None,
"should not resolve task with blank label and/or command: {task_with_blank_property:?}"
);
@@ -266,6 +332,7 @@ mod tests {
let resolved_task = task_template
.resolve_task(TEST_ID_BASE, task_cx)
.unwrap_or_else(|| panic!("failed to resolve task {task_without_cwd:?}"));
+ assert_substituted_variables(&resolved_task, Vec::new());
resolved_task
.resolved
.clone()
@@ -274,30 +341,23 @@ mod tests {
})
};
+ let cx = TaskContext {
+ cwd: None,
+ task_variables: TaskVariables::default(),
+ };
assert_eq!(
- resolved_task(
- &task_without_cwd,
- TaskContext {
- cwd: None,
- task_variables: TaskVariables::default(),
- }
- )
- .cwd,
+ resolved_task(&task_without_cwd, &cx).cwd,
None,
"When neither task nor task context have cwd, it should be None"
);
let context_cwd = Path::new("a").join("b").join("c");
+ let cx = TaskContext {
+ cwd: Some(context_cwd.clone()),
+ task_variables: TaskVariables::default(),
+ };
assert_eq!(
- resolved_task(
- &task_without_cwd,
- TaskContext {
- cwd: Some(context_cwd.clone()),
- task_variables: TaskVariables::default(),
- }
- )
- .cwd
- .as_deref(),
+ resolved_task(&task_without_cwd, &cx).cwd.as_deref(),
Some(context_cwd.as_path()),
"TaskContext's cwd should be taken on resolve if task's cwd is None"
);
@@ -307,30 +367,22 @@ mod tests {
task_with_cwd.cwd = Some(task_cwd.display().to_string());
let task_with_cwd = task_with_cwd;
+ let cx = TaskContext {
+ cwd: None,
+ task_variables: TaskVariables::default(),
+ };
assert_eq!(
- resolved_task(
- &task_with_cwd,
- TaskContext {
- cwd: None,
- task_variables: TaskVariables::default(),
- }
- )
- .cwd
- .as_deref(),
+ resolved_task(&task_with_cwd, &cx).cwd.as_deref(),
Some(task_cwd.as_path()),
"TaskTemplate's cwd should be taken on resolve if TaskContext's cwd is None"
);
+ let cx = TaskContext {
+ cwd: Some(context_cwd.clone()),
+ task_variables: TaskVariables::default(),
+ };
assert_eq!(
- resolved_task(
- &task_with_cwd,
- TaskContext {
- cwd: Some(context_cwd.clone()),
- task_variables: TaskVariables::default(),
- }
- )
- .cwd
- .as_deref(),
+ resolved_task(&task_with_cwd, &cx).cwd.as_deref(),
Some(task_cwd.as_path()),
"TaskTemplate's cwd should be taken on resolve if TaskContext's cwd is not None"
);
@@ -400,14 +452,14 @@ mod tests {
for i in 0..15 {
let resolved_task = task_with_all_variables.resolve_task(
TEST_ID_BASE,
- TaskContext {
+ &TaskContext {
cwd: None,
task_variables: TaskVariables::from_iter(all_variables.clone()),
},
).unwrap_or_else(|| panic!("Should successfully resolve task {task_with_all_variables:?} with variables {all_variables:?}"));
match &first_resolved_id {
- None => first_resolved_id = Some(resolved_task.id),
+ None => first_resolved_id = Some(resolved_task.id.clone()),
Some(first_id) => assert_eq!(
&resolved_task.id, first_id,
"Step {i}, for the same task template and context, there should be the same resolved task id"
@@ -423,6 +475,10 @@ mod tests {
format!("test label for 1234 and {long_value}"),
"Resolved task label should be substituted with variables and those should not be shortened"
);
+ assert_substituted_variables(
+ &resolved_task,
+ all_variables.iter().map(|(name, _)| name.clone()).collect(),
+ );
let spawn_in_terminal = resolved_task
.resolved
@@ -478,7 +534,7 @@ mod tests {
let removed_variable = not_all_variables.remove(i);
let resolved_task_attempt = task_with_all_variables.resolve_task(
TEST_ID_BASE,
- TaskContext {
+ &TaskContext {
cwd: None,
task_variables: TaskVariables::from_iter(not_all_variables),
},
@@ -495,11 +551,11 @@ mod tests {
args: vec!["$PATH".into()],
..Default::default()
};
- let resolved = task
- .resolve_task(TEST_ID_BASE, TaskContext::default())
- .unwrap()
- .resolved
+ let resolved_task = task
+ .resolve_task(TEST_ID_BASE, &TaskContext::default())
.unwrap();
+ assert_substituted_variables(&resolved_task, Vec::new());
+ let resolved = resolved_task.resolved.unwrap();
assert_eq!(resolved.label, task.label);
assert_eq!(resolved.command, task.command);
assert_eq!(resolved.args, task.args);
@@ -514,7 +570,74 @@ mod tests {
..Default::default()
};
assert!(task
- .resolve_task(TEST_ID_BASE, TaskContext::default())
+ .resolve_task(TEST_ID_BASE, &TaskContext::default())
.is_none());
}
+
+ #[test]
+ fn test_symbol_dependent_tasks() {
+ let task_with_all_properties = TaskTemplate {
+ label: "test_label".to_string(),
+ command: "test_command".to_string(),
+ args: vec!["test_arg".to_string()],
+ env: HashMap::from_iter([("test_env_key".to_string(), "test_env_var".to_string())]),
+ ..TaskTemplate::default()
+ };
+ let cx = TaskContext {
+ cwd: None,
+ task_variables: TaskVariables::from_iter(Some((
+ VariableName::Symbol,
+ "test_symbol".to_string(),
+ ))),
+ };
+
+ for (i, symbol_dependent_task) in [
+ TaskTemplate {
+ label: format!("test_label_{}", VariableName::Symbol.template_value()),
+ ..task_with_all_properties.clone()
+ },
+ TaskTemplate {
+ command: format!("test_command_{}", VariableName::Symbol.template_value()),
+ ..task_with_all_properties.clone()
+ },
+ TaskTemplate {
+ args: vec![format!(
+ "test_arg_{}",
+ VariableName::Symbol.template_value()
+ )],
+ ..task_with_all_properties.clone()
+ },
+ TaskTemplate {
+ env: HashMap::from_iter([(
+ "test_env_key".to_string(),
+ format!("test_env_var_{}", VariableName::Symbol.template_value()),
+ )]),
+ ..task_with_all_properties.clone()
+ },
+ ]
+ .into_iter()
+ .enumerate()
+ {
+ let resolved = symbol_dependent_task
+ .resolve_task(TEST_ID_BASE, &cx)
+ .unwrap_or_else(|| panic!("Failed to resolve task {symbol_dependent_task:?}"));
+ assert_eq!(
+ resolved.substituted_variables,
+ HashSet::from_iter(Some(VariableName::Symbol)),
+ "(index {i}) Expected the task to depend on symbol task variable: {resolved:?}"
+ )
+ }
+ }
+
+ #[track_caller]
+ fn assert_substituted_variables(resolved_task: &ResolvedTask, mut expected: Vec<VariableName>) {
+ let mut resolved_variables = resolved_task
+ .substituted_variables
+ .iter()
+ .cloned()
+ .collect::<Vec<_>>();
+ resolved_variables.sort_by_key(|var| var.to_string());
+ expected.sort_by_key(|var| var.to_string());
+ assert_eq!(resolved_variables, expected)
+ }
}
@@ -29,20 +29,19 @@ pub fn init(cx: &mut AppContext) {
})
{
if action.reevaluate_context {
- let mut original_task = last_scheduled_task.original_task;
+ let mut original_task = last_scheduled_task.original_task().clone();
if let Some(allow_concurrent_runs) = action.allow_concurrent_runs {
original_task.allow_concurrent_runs = allow_concurrent_runs;
}
if let Some(use_new_terminal) = action.use_new_terminal {
original_task.use_new_terminal = use_new_terminal;
}
- let cwd = task_cwd(workspace, cx).log_err().flatten();
- let task_context = task_context(workspace, cwd, cx);
+ let task_context = task_context(workspace, cx);
schedule_task(
workspace,
task_source_kind,
&original_task,
- task_context,
+ &task_context,
false,
cx,
)
@@ -77,8 +76,7 @@ fn spawn_task_or_modal(workspace: &mut Workspace, action: &Spawn, cx: &mut ViewC
None => {
let inventory = workspace.project().read(cx).task_inventory().clone();
let workspace_handle = workspace.weak_handle();
- let cwd = task_cwd(workspace, cx).log_err().flatten();
- let task_context = task_context(workspace, cwd, cx);
+ let task_context = task_context(workspace, cx);
workspace.toggle_modal(cx, |cx| {
TasksModal::new(inventory, task_context, workspace_handle, cx)
})
@@ -98,13 +96,12 @@ fn spawn_task_with_name(name: String, cx: &mut ViewContext<Workspace>) {
});
let (task_source_kind, target_task) =
tasks.into_iter().find(|(_, task)| task.label == name)?;
- let cwd = task_cwd(workspace, cx).log_err().flatten();
- let task_context = task_context(workspace, cwd, cx);
+ let task_context = task_context(workspace, cx);
schedule_task(
workspace,
task_source_kind,
&target_task,
- task_context,
+ &task_context,
false,
cx,
);
@@ -148,11 +145,8 @@ fn active_item_selection_properties(
(worktree_id, language)
}
-fn task_context(
- workspace: &Workspace,
- cwd: Option<PathBuf>,
- cx: &mut WindowContext<'_>,
-) -> TaskContext {
+fn task_context(workspace: &Workspace, cx: &mut WindowContext<'_>) -> TaskContext {
+ let cwd = task_cwd(workspace, cx).log_err().flatten();
let current_editor = workspace
.active_item(cx)
.and_then(|item| item.act_as::<Editor>(cx));
@@ -253,7 +247,7 @@ fn schedule_task(
workspace: &Workspace,
task_source_kind: TaskSourceKind,
task_to_resolve: &TaskTemplate,
- task_cx: TaskContext,
+ task_cx: &TaskContext,
omit_history: bool,
cx: &mut ViewContext<'_, Workspace>,
) {
@@ -338,7 +332,7 @@ mod tests {
use ui::VisualContext;
use workspace::{AppState, Workspace};
- use crate::{task_context, task_cwd};
+ use crate::task_context;
#[gpui::test]
async fn test_default_language_context(cx: &mut TestAppContext) {
@@ -433,7 +427,7 @@ mod tests {
this.add_item_to_center(Box::new(editor2.clone()), cx);
assert_eq!(this.active_item(cx).unwrap().item_id(), editor2.entity_id());
assert_eq!(
- task_context(this, task_cwd(this, cx).unwrap(), cx),
+ task_context(this, cx),
TaskContext {
cwd: Some("/dir".into()),
task_variables: TaskVariables::from_iter([
@@ -450,7 +444,7 @@ mod tests {
this.change_selections(None, cx, |selections| selections.select_ranges([14..18]))
});
assert_eq!(
- task_context(this, task_cwd(this, cx).unwrap(), cx),
+ task_context(this, cx),
TaskContext {
cwd: Some("/dir".into()),
task_variables: TaskVariables::from_iter([
@@ -467,7 +461,7 @@ mod tests {
// Now, let's switch the active item to .ts file.
this.activate_item(&editor1, cx);
assert_eq!(
- task_context(this, task_cwd(this, cx).unwrap(), cx),
+ task_context(this, cx),
TaskContext {
cwd: Some("/dir".into()),
task_variables: TaskVariables::from_iter([