fix(tasks): refactor editor command handling and improve error reporting#9752
Conversation
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to the editor invocation path and uses the already-present shell_words crate in a way consistent with the rest of the codebase. The fix correctly addresses the root cause (whole-string program lookup instead of split invocation), reuses an existing well-tested dependency, and is covered by new unit tests. No regressions are introduced in the surrounding task creation or path-display logic. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "Merge branch 'main' into issues/9731" | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Refactors mise tasks edit editor invocation to correctly handle EDITOR/VISUAL values that include arguments (fix for #9731) and improves associated error handling.
Changes:
- Replace direct
cmd!($EDITOR, file)invocation withopen_in_editor()that splits the editor command into program + args. - Add
split_editor_command()usingshell_words::splitand introduce an explicit error for emptyEDITOR/VISUAL. - Add a unit test covering editor values with arguments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request enhances the task editing functionality by correctly parsing the EDITOR environment variable when it contains arguments. It introduces a helper function split_editor_command using shell_words and adds corresponding unit tests. Review feedback suggests refactoring the variable assignment for better conciseness and using an iterator-based approach in split_editor_command to improve efficiency and idiomaticity.
# Problem While trying to understand an issue with the value of VISUAL in `mise -v tasks edit <task>` (see jdx#9731, already addressed by PR jdx#9752), I found the `debug!` message generated by `cmd::cmd(..)` to be less helpful than it could be. With `VISUAL='cat -n'`, the following information got logged: ``` log ... DEBUG $ cat -n /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` Here, `cat -n` was interpreted as the program to execute. However, the `DEBUG:` line, doesn't show that. # Solution Add single-qu While trying to understand an issue with the value of VISUAL in `mise -v tasks edit <task>` (see jdx#9731, already addressed by PR jdx#9752), I found the `debug!` message generated by `cmd::cmd(..)` to be less helpful than it could be. With `VISUAL='cat -n'`, the following information got logged: ``` log ... DEBUG $ cat -n /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` Here, `cat -n` was interpreted as the program to execute. However, the `DEBUG:` line, doesn't show that. # Solution Add single-quotes around `program` and `args` when generating the `debug!` message. This changes the logged information to: ```log ... DEBUG $ 'cat -n' '/home/kt/work/oss/mise/tasks.toml' Error: 0: No such file or directory (os error 2) ... ``` This clearly shows the reason for the following error message. Note: The `Error` reported by `duct::cmd(..)`: ```log Error: 0: No such file or directory (os error 2) ``` is also not helpful. This [issue from 2023 in the *duct repository*](oconnor663/duct.rs#109) describes that problem (and would potentially solve the issue addressed by this PR). But so far, nobody stepped up to provide a fix.
# Problem While trying to understand an issue with the value of VISUAL in `mise -v tasks edit <task>` (see jdx#9731, already addressed by PR jdx#9752), I found the `debug!` message generated by `cmd::cmd(..)` to be less helpful than it could be. With `VISUAL='cat -n'`, the following information got logged: ``` log ... DEBUG $ cat -n /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` Here, `cat -n` was interpreted as the program to execute. However, the `DEBUG ...` line, doesn't show that. # Solution Add single-quotes around `program` and `args` when generating the `debug!` message. This changes the logged information to: ```log ... DEBUG $ 'cat -n' '/home/kt/work/oss/mise/tasks.toml' Error: 0: No such file or directory (os error 2) ... ``` This hopefully helps to understand the reason for the following error message (from `duct::cmd(..)`). --- Note: The error reported by `duct::cmd(..)`: ```log Error: 0: No such file or directory (os error 2) ``` also leaves room for improvement. This was already discussed in this [issue from 2023 in the *duct repository*](oconnor663/duct.rs#109) and would potentially solve the issue addressed by this commit/PR. But so far, nobody stepped up to provide a fix.
# Problem While trying to understand an issue with the value of VISUAL in `mise -v tasks edit <task>` (see jdx#9731, already addressed by PR jdx#9752), I found the `debug!` message generated by `cmd::cmd(..)` to be less helpful than it could be. With `VISUAL='cat -n'`, the following information got logged: ``` log ... DEBUG $ cat -n /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` Here, `cat -n` was interpreted as the program to execute. However, the `DEBUG ...` line, doesn't show that. # Solution Add single-quotes around `program` and `args` when generating the `debug!` message. This changes the logged information to: ```log ... DEBUG $ 'cat -n' '/home/kt/work/oss/mise/tasks.toml' Error: 0: No such file or directory (os error 2) ... ``` This hopefully helps to understand the reason for the following error message (from `duct::cmd(..)`). --- Note: The error reported by `duct::cmd(..)`: ```log Error: 0: No such file or directory (os error 2) ``` also leaves room for improvement. This was already discussed in [issue 109 (Include the command that failed in the error)](oconnor663/duct.rs#109) from 2023 in the *duct.rs repository*. A fix for this issue would potentially solve the problem addressed by this commit/PR. But so far, nobody stepped up to provide that fix.
# Problem While trying to understand an issue with the value of VISUAL in `mise -v tasks edit <task>` (see jdx#9731, already addressed by PR jdx#9752), I found the `debug!` message generated by `cmd::cmd(..)` to be less helpful than it could be. With `VISUAL='cat -n'`, the following information got logged: ``` log ... DEBUG $ cat -n /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` Here, `cat -n` was interpreted as the program to execute. However, the `DEBUG ...` line, doesn't show that. # Solution Use shell_escape::escape on `program` and `args` when generating the `debug!` message. This changes the logged information to: ```log ... DEBUG $ 'cat -n' /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` This hopefully helps to understand the reason for the following error message (from `duct::cmd(..)`). --- Note: The error reported by `duct::cmd(..)`: ```log Error: 0: No such file or directory (os error 2) ``` also leaves room for improvement. This was already discussed in [issue 109 (Include the command that failed in the error)](oconnor663/duct.rs#109) from 2023 in the *duct.rs repository*. A fix for this issue would potentially solve the problem addressed by this commit/PR. But so far, nobody stepped up to provide that fix.
# Problem While trying to understand an issue with the value of VISUAL in `mise -v tasks edit <task>` (see jdx#9731, already addressed by PR jdx#9752), I found the `debug!` message generated by `cmd::cmd(..)` to be less helpful than it could be. With `VISUAL='cat -n'`, the following information got logged: ``` log ... DEBUG $ cat -n /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` Here, `cat -n` was interpreted as the program to execute. However, the `DEBUG ...` line, doesn't show that. # Solution Call `shell_escape::escape(..)` on `program` and `args` when generating the `debug!` message. This changes the logged information to: ```log ... DEBUG $ 'cat -n' /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` This hopefully helps to understand the reason for the following error message (from `duct::cmd(..)`). --- Note: The error reported by `duct::cmd(..)`: ```log Error: 0: No such file or directory (os error 2) ``` also leaves room for improvement. This was already discussed in [issue 109 (Include the command that failed in the error)](oconnor663/duct.rs#109) from 2023 in the *duct.rs repository*. A fix for this issue would potentially solve the problem addressed by this commit/PR. But so far, nobody stepped up to provide that fix.
…9777) # Problem While trying to understand an issue with the value of VISUAL in `mise -v tasks edit <task>` (see #9731, already addressed by PR #9752), I found the `debug!` message generated by `cmd::cmd(..)` to be less helpful than it could be. With `VISUAL='cat -n'`, the following information got logged: ``` log ... DEBUG $ cat -n /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` Here, `cat -n` was interpreted as the program to execute. However, the `DEBUG ...` line, doesn't show that. # Solution Call `shell_escape::escape(..)` on `program` and `args` when generating the `debug!` message. This changes the logged information to: ```log ... DEBUG $ 'cat -n' /home/kt/work/oss/mise/tasks.toml Error: 0: No such file or directory (os error 2) ... ``` This hopefully helps to understand the reason for the following error message from `duct::cmd(..)`). --- Note: The error reported by `duct::cmd(..)`: ```log Error: 0: No such file or directory (os error 2) ``` also leaves room for improvement. This was already discussed in [issue 109: Include the command that failed in the error](oconnor663/duct.rs#109) from 2023 in the *duct.rs repository*. A fix for this issue would potentially solve the problem addressed by this commit/PR. But so far, nobody stepped up to provide that fix.
Fixes an issue mentioned in #9731 with EDITOR/VISUAL value using arguments.