Skip to content

Fix Rename events in watch#18115

Merged
cptpiepmatz merged 9 commits intonushell:mainfrom
Bahex:watch-it
Apr 26, 2026
Merged

Fix Rename events in watch#18115
cptpiepmatz merged 9 commits intonushell:mainfrom
Bahex:watch-it

Conversation

@Bahex
Copy link
Copy Markdown
Member

@Bahex Bahex commented Apr 24, 2026

Description

  • Added tests, which we couldn't do cleanly before streaming watch
  • Remove the intermediate WatchEventRecord type previously used for easier conversion to Value.

    IntoValue was not widely implemented for std types at the time of initial implementation, and an intermediate type was necessary.

  • Simplify TryFrom<DebouncedEvent> for WatchEvent and fix the order of paths in Rename events.
  • Detect Rename events where the source or destination path is outside of the watched directory.

User-facing changes (Release notes)

Order of the paths reported for Rename events are now correct, whereas previously they were reported as if $new_path was renamed to $path.

Additionally if one of these paths happened to be outside of the watched directory, previously the event was not reported at all, now they are reported with the path outside of the watched directory as null.

Additional notes

We should think about deprecating the optional closure parameter and only supporting the streaming version. The closure form

  • does not compose with other commands
  • can't be stopped without user intervention...
  • ...which also makes it harder to test

Removing the closure parameter would also make it possible to have a rest parameter which could be used to watch multiple paths.

Bahex added 5 commits April 25, 2026 02:16
IntoValue was not widely implemented for std types at the time of 
initial implementation, and an intermediate type was necessary.
Also refactor WatchEvent's TryFrom impl for conciseness
Bahex added 2 commits April 25, 2026 18:32
Previous values worked on linux and windows, but for some reason caused failures on macOS in CI
@Bahex Bahex force-pushed the watch-it branch 2 times, most recently from 77bd72a to 1471088 Compare April 25, 2026 18:14
@cptpiepmatz cptpiepmatz added the notes:fixes Noted in "Bug fixes" section label Apr 25, 2026
@cptpiepmatz cptpiepmatz self-requested a review April 25, 2026 19:53
Comment thread crates/nu-command/src/filesystem/watch.rs Outdated
Comment thread crates/nu-command/tests/commands/watch.rs Outdated
@cptpiepmatz
Copy link
Copy Markdown
Member

This also needs the notes for the changelog.

Copy link
Copy Markdown
Member

@cptpiepmatz cptpiepmatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good now, thx

@cptpiepmatz cptpiepmatz merged commit 164b0d1 into nushell:main Apr 26, 2026
17 checks passed
@github-actions github-actions Bot added this to the v0.113.0 milestone Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:fixes Noted in "Bug fixes" section

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants