tasks: Show error for unknown ZED_ variables#45621
tasks: Show error for unknown ZED_ variables#45621dinocosta merged 8 commits intozed-industries:mainfrom
ZED_ variables#45621Conversation
bdfb70e to
12487b5
Compare
Introduce `TaskTemplate.unknown_variables`, which determines whether any invalid variable name starting with `ZED_` is being used in the `label`, `command`, `args`, `env` or `cwd` and, if that's the case, returns a vector with all of the unique and invalid variable names. Update `Inventory::update_file_based_tasks` in order to use `TaskTemplate.unknown_variables` when parsing the templates, keeping track of which ones have invalid variables, while keeping the valid ones around. If any validation errors are found, an `Err` is returned, which will trigger a validation error dialog to be shown. This approach replaces the runtime validation, which showed errors when opening the task picker, as I believe validating at save-time might be a better UX, as users are immediately alerted about invalid variables without having to open the task picker, and seems to be a simpler solution as we don't need a new subscription, new event and we avoid upgrading a `cx.background_spawn` to a `cx.spawn`, which runs in the main thread so it can block the UI thread, so we can continue keeping the task resolution off the UI thread.
dinocosta
left a comment
There was a problem hiding this comment.
Hey @playdohface 👋
Thank you so much for suggesting these changes! The idea of having a TaskResolutionError seemed really nice, but while reviewing I ended up exploring a different approach, hope you don't mind! 😅
I've kept all changes in a single commit so we can easily revert if you see something that worries you.
The main difference is that validation of ZED_ variables now happens when the file is saved, immediately displaying an error to the user. I like that the feedback is more immediate, as users don't have to open the task picker to discover that invalid variables are being used, and valid tasks in the same file are still loaded and work normally.
This approach also avoids adding a new subscription and upgrading cx.background_spawn to cx.spawn, which would run task resolution on the UI thread.
Here's a quick screen recording of the new approach. Let me know what you think!
CleanShot.2026-01-19.at.22.47.27.mp4
|
Hey @dinocosta thanks for looking into this, and I think your solution is better UX in the end, I'm all for it 👍 |
- Introduce `project::ToastLink` struct - Add optional `link` field to `project::Event::Toast`, allowing toasts to include a button that opens a URL - Update the toast shown when the local tasks parsing fails, to include a link to Zed's Task Variables documentation
ZED_ variablesZED_ variables
|
Sorry for the delay @playdohface ! Was checking the design with the team and adding back the button that link's to Zed's Tasks Documentation – https://zed.dev/docs/tasks . Merging this now! 🙌 |
Closes #23275
This makes resolving task variables a Result rather than an Option, and shows an error when tasks are not loaded because of unresolvable "special" variables in user defined tasks. Extension/Language tasks will still silently be omitted, as for these an unresolved capture-variable usually means the task is not applicable.
Release Notes:
ZED_*variables being silently omitted