cargo-watch icon indicating copy to clipboard operation
cargo-watch copied to clipboard

Wrong behavior when using system commands with non-bash or non-cmd.exe shells

Open Halkcyon opened this issue 3 years ago • 7 comments

I'm attempting to use shells besides those in the title, but there is wrong behavior present in this extension.

image

In PowerShell or nushell, neither && nor & are supported. When attempting to use PowerShell v7+, & causes cargo-watch to spawn a background job instead.

When attempting to use the cmd:trail form via -- $COMMAND_LINE, it still uses the wrongly-formed & echo after the command.


platform: Windows 10 cargo: v1.61.0 cargo-watch: v8.1.1

Halkcyon avatar Jun 14 '22 02:06 Halkcyon

Oh, that's interesting and I completely missed that. 🙃 I'm going to have to sit down and figure that one out.

As a workaround, you can do something like:

cargo watch -n -- powershell.exe -e 'your command line'

As a side note, you're writing

cargo watch -x build --use-shell=powershell.exe -s 'ni .trigger'

obviously --use-shell currently makes cargo watch uses powershell.exe for the entire thing, but would you be wanting/expecting cargo watch to use different shell for different commands or was that just an artifact of how you wrote the command?

passcod avatar Jun 15 '22 00:06 passcod

I don't see the -n as an option in the help text? My goal here was trying to get web server hot reloading on code change following a suggestion in the documentation to create a temporary .trigger file.

I think it makes sense that use-shell affects the whole invocation, but I'm not sure how you would support arbitrarily connected commands. In my nushell/Windows PowerShell example, you can only join commands using ; but that's just a separator and won't fail on a non-0 RC.

Ultimately, I think the & echo "... behavior is surprising and perhaps unnecessary with use-shell?

Halkcyon avatar Jun 15 '22 02:06 Halkcyon

Ugh, I keep forgetting option differences with watchexec. You're right, there is no -n, so there's no workaround after all

passcod avatar Jun 15 '22 03:06 passcod

I'm not sure how you would support arbitrarily connected commands

Ultimately the watchexec runtime will just support running multiple processes natively, so it won't matter. For now, cargo watch tries to approximate it, and, as you've found, sometimes runs into the wall.

passcod avatar Jun 15 '22 03:06 passcod

Do you have an idea you'd like to explore? I'd suggest not appending any extra command output when using the -- form @passcod. I can add a contribution to correct that behavior and anticipate minor impact to people who want to see that echo output

Halkcyon avatar Jun 21 '22 02:06 Halkcyon

Right, sorry, an update: as of 5 days ish ago the Watchexec runtime supports this natively (it was not super hard to implement). I've also decided to move Cargo Watch to the newer runtime sooner rather than later. So what will happen is:

  • [x] I'll finish releasing the Watchexec runtime. There's three new crates that need to be just slightly polished and published.
  • [x] I'll incorporate the new bits in Watchexec CLI 1.20.0.
  • [ ] I'll finish porting Cargo Watch to the new Watchexec runtime.
  • [ ] I'll release Cargo Watch 9.0.0, which will fix this bug.

~~I'm anticipating this to take 1-2 weeks. Update: aiming for a weekend release by 10 July.~~ Well, that didn't go as planned.

However, if you want to, you can put up a PR that does exactly what you've just said, and I'll release that as 8.1.3 asap, with the understanding that exact behaviour will probably change slightly when 9 gets released.

passcod avatar Jun 21 '22 02:06 passcod

Workaround will be in 8.2.0

passcod avatar Dec 29 '22 08:12 passcod