Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression on task_name when spawning task with spaces in name from keybinding #24832

Closed
danieljimenez-mycase opened this issue Feb 13, 2025 · 8 comments · Fixed by #24898
Closed
Assignees
Labels
regression A bug that was introduced in a recent release

Comments

@danieljimenez-mycase
Copy link

danieljimenez-mycase commented Feb 13, 2025

Summary

Setting task_name with spaces does not work with keybindings

1.add task to task.json

{
  "label": "start lazygit",
  "command": "lazygit -p $ZED_WORKTREE_ROOT"
}

add keybinding

{
  "context": "Workspace",
  "bindings": {
    "alt-g": [
      "task::Spawn",
      { "task_name": "start lazygit", "reveal_target": "center" }
    ]
  }
}
  1. use keybinding

Actual Behavior:
opens task selector

Expected Behavior:
opens task with that name

I fixed this by using "_" instead of spaces. I got this idea because the auto config update changed it to be that.
Do we need to update the docs instead?

Zed Version and System Specs

Zed: v0.173.8 (Zed)
OS: macOS 14.7.4
Memory: 16 GiB
Architecture: aarch64

@danieljimenez-mycase danieljimenez-mycase changed the title Regression on task_name when spawning task from keybinding Regression on task_name when spawning task with spaces in name from keybinding Feb 13, 2025
@497e0bdf29873
Copy link

As documented in #24833, also dashes seem to be unallowed. The auto-update should tell what changes it made, and such task name restrictions documented.

@0xtimsb 0xtimsb self-assigned this Feb 13, 2025
@injust
Copy link
Contributor

injust commented Feb 14, 2025

I also can't use any uppercase letters in the task name, and as a result variables don't work either (e.g. $ZED_STEM).

@maxbrunsfeld maxbrunsfeld added the regression A bug that was introduced in a recent release label Feb 14, 2025
@maxbrunsfeld
Copy link
Collaborator

@osiewicz Do you know of any changes recently that could have caused a regression with task names containing spaces?

@osiewicz
Copy link
Contributor

osiewicz commented Feb 14, 2025

Not that I'm aware of. I spun up a quick debug session and it seems that the action disposition itself has a task name with spaces replaced? I'm not sure what's up with that.
E.g. the following key binding:

      "shift-f": [
        "task::Spawn",
        { "task_name": "a b" }
      ]

Deserializes as:

[crates/tasks_ui/src/tasks_ui.rs:96:5] &action = ByName {
    task_name: "a_b",
    reveal_target: None,
}

which is obviously wrong and seems to be the culprit of a problem.

@0xtimsb
Copy link
Member

0xtimsb commented Feb 14, 2025

I think I know why this is happening. Seems related to in-memory migrator turning binding values to snake case. Picking this up.

@osiewicz
Copy link
Contributor

Yep, the settings load just fine for me when I disable the keymap migration.

@maxbrunsfeld
Copy link
Collaborator

maxbrunsfeld commented Feb 14, 2025

@danieljimenez-mycase @497e0bdf29873 Thanks for the report; we'll need to publish a patch release for this. We'll let you know when it is out.

gcp-cherry-pick-bot bot pushed a commit that referenced this issue Feb 14, 2025
Closes #24832

Only turns specified deperecated keys and values to snake case.

Release Notes:

- Fixed issue where keybindings would open task selector instead of
spawn that task.
gcp-cherry-pick-bot bot pushed a commit that referenced this issue Feb 14, 2025
Closes #24832

Only turns specified deperecated keys and values to snake case.

Release Notes:

- Fixed issue where keybindings would open task selector instead of
spawn that task.
0xtimsb added a commit that referenced this issue Feb 14, 2025
Cherry-picked migrator: Fix keymap task_name regression  (#24898)

Closes #24832

Only turns specified deperecated keys and values to snake case.

Release Notes:

- Fixed issue where keybindings would open task selector instead of
spawn that task.

Co-authored-by: smit <[email protected]>
0xtimsb added a commit that referenced this issue Feb 14, 2025
Cherry-picked migrator: Fix keymap task_name regression  (#24898)

Closes #24832

Only turns specified deperecated keys and values to snake case.

Release Notes:

- Fixed issue where keybindings would open task selector instead of
spawn that task.

Co-authored-by: smit <[email protected]>
@dazsmith
Copy link

After a recent update (I guess this one), when I opened my keymap.json file, zed automatically notified me that the file needed to be updated to a new format. I accepted without further thought. It turns out that the task_names had spaces replaced by underscores, eg. pdflatex build $\mapsto$ pdflatex_build. Fine, but the same thing was not done automatically in the tasks.json file. More seriously, because I had not read this issue, it took me a long time to understand why my keymaps were causing the tasks modal to appear instead of just running the tasks as they used to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression A bug that was introduced in a recent release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants