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

Experimental fix for [issue 2312] #2528

Conversation

JustSimplyKyle
Copy link

This pr is an experimental fix for #2312
it doesn't change the underlying logic, just the events that's tracked.
(this is just me adding random events to the track list until it fixes the hot-reload issue)
This works under helix and vscode, not sure if neovim has the same behaviour
@Andrew15-5 mind testing?

@JustSimplyKyle JustSimplyKyle changed the title Experimental fix for issue https://github.com/DioxusLabs/dioxus/issues/2312 Experimental fix for [issue 2312](https://github.com/DioxusLabs/dioxus/issues/2312) Jun 16, 2024
@JustSimplyKyle JustSimplyKyle changed the title Experimental fix for [issue 2312](https://github.com/DioxusLabs/dioxus/issues/2312) Experimental fix for [issue 2312] Jun 16, 2024
@Andrew15-5
Copy link
Contributor

If we have an image or something called img.png and then we want to rename it. If the file name is changed to image.png first in the Rust code, then the dx will show the unknown file error. After renaming the file (mv img.png image.png) the watcher won't see it and the workflow will be broken. Maybe re-saving the Rust file will fix this.

This is just an example that I come up with on the spot. This means that the potential FS manipulations can degrade the DX for everyone just so that you can use the default saving behavior in Neovim. I don't think this is worth it, if what I describe above is true. I don't really even know why other watchers (although I haven't used much of them, or I just can't remember them) don't complain about the Neovim's backup saving shenanigans. I only faced this problem with Rust/notify-based watchers (Dioxus/Typst CLI).

I can test this later (in a few days?), now I'm mostly busy.

@jkelleyrtp
Copy link
Member

jkelleyrtp commented Jul 23, 2024

In our recent CLI overhaul I incorporated your changes into our watcher - will test against the original issue.

Thanks!

I'll make sure to adjust the commit to have your name on it for the release notes / contributor graph :)

@jkelleyrtp jkelleyrtp closed this Jul 23, 2024
@Andrew15-5
Copy link
Contributor

#2312 (comment)

So, for that commit this isn't even necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants