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

Hot reload on save, only if the current file had changes. #3995

Closed
p4-k4 opened this issue May 27, 2022 · 6 comments
Closed

Hot reload on save, only if the current file had changes. #3995

p4-k4 opened this issue May 27, 2022 · 6 comments
Labels
in commands Relates to commands (usually invoked from the command Palette) in editor Relates to code editing or language features is enhancement
Milestone

Comments

@p4-k4
Copy link

p4-k4 commented May 27, 2022

Hey there, as the title states - It would be nice to have the option to only hot reload on save; when the current file had changes and was indeed saved.

The problem is 1. An unnecessary hot reload is triggered and 2. the debug console is polluted in hot reload logging.

What happens currently is that even though a files changes have been saved (the file does not required another save), if we (for whatever reason) save again it will trigger an unnecessary hot reload.

Admiringly, I have a habit of saving incrementally - Vim/Evil users will understand, I'm sure I'm not the only one.

I did try to check to see if such an option exists, but it appears not? Thanks

@DanTup
Copy link
Member

DanTup commented May 30, 2022

I have a habit of saving incrementally

I'm not sure I fully understand the request here. By incremental do you mean while you're making changes, or that you press Save a lot even when not making changes?

Having explicit saves (even when no changes) trigger a reload is useful when using auto-save, as the hot-reload-on-auto-save can be disabled (so it doesn't trigger lots), but you can use Ctrl+S as a way to force a reload.

It's possible to add a new setting to control this, but I'd like to be sure I understand exactly what the user case first. Thanks!

@DanTup DanTup added the awaiting info Requires more information from the customer to progress label May 30, 2022
@p4-k4
Copy link
Author

p4-k4 commented May 30, 2022

I'm not sure I fully understand the request here. By incremental do you mean while you're making changes, or that you press Save a lot even when not making changes?

By this I mean I make regular saves in very small increments by habit (and I'm sure I can vouch for many others who do the same).

An automatic hot-reload-on-save is good but that should only occur if a file has indeed been saved, otherwise it's pointless to trigger a hot-reload when the latest changes have already been pushed. This is how emacs-lsp/dart-lsp behaves which is a small thing but caught my attention when trying out dart-code recently.

The options for dart-code always and manual (except never) currently trigger a hot reload regardless of the saved state of the currently opened file.

Perhaps always can trigger a hot reload regardless, that makes sense. But maybe manual can be triggered only if the current file has outstanding changes. Hope that makes sense.

@DanTup
Copy link
Member

DanTup commented May 30, 2022

By this I mean I make regular saves in very small increments by habit

Is this while you're reviewing/reading code? Or while you're doing something else?

I'd like to understand the situations in which this happens as it may affect the solution (for example I'm wondering whether it would be best to support it for both always and manual, or if just for always is enough.. we do need to support the current behaviour when using manual as it's there to support auto-save users that want to manually trigger reloads ad not have them on every auto-save).

Thanks!

@p4-k4
Copy link
Author

p4-k4 commented May 30, 2022

Good question, and yes it could be either while reading or as I'm working, I'm constantly triggering saves. As a vim/evil key binds user, this is quite common, if you ever watch a vim user - they'll habitually be pressing<leader>, f, s like there's no tomorrow.

Regardless, I feel it doesn't make sense to persistently hot-reload-on-save for either manual oralways (UNLESS a file has just had changes saved, then yes let's trigger a hot-reload).

Either way, manual or always still clog up the debug console unnecessarily - We're in some cases forced to scroll up past the noise to get to console messages that actually do matter.

It makes more sense to me to just hot-reload only once a file has had changes saved that have not already been pushed. That would be regardless of if the save was manual/automatic.

Hope that helps

@DanTup
Copy link
Member

DanTup commented May 30, 2022

I feel it doesn't make sense to persistently hot-reload-on-save for either manual or always (UNLESS a file has just had changes saved, then yes let's trigger a hot-reload).

We do need to support this for manual, it's important for supporting auto-save users that do not want hot-reload triggering on every auto-save (eg. whenever the pause typing for a few hundred milliseconds), but they do want to then be able to press Save (even though the changes were already auto-saved) to trigger the reload.

I think rather than adding a new setting, I may adjust the options to the existing setting:

  • never - "Do not reload when saving"
  • manual - "Reload for manual saves (requires pressing Save explicitly if using autosave)"
  • manualIfDirty - "Reload for manual saves (requires pressing Save explicitly if using autosave) only if the saved file had changes"
  • all (previously always) - "Reload for all saves, manual or automatic"
  • allIfDirty - "Reload for all saves, manual or automatic only if the saved file had changes"

The old value of "always" will continue to work, although report that it's not longer valid (so that existing users don't have their behaviour changed).

@DanTup DanTup removed the awaiting info Requires more information from the customer to progress label May 30, 2022
@DanTup DanTup added this to the v3.42.0 milestone May 30, 2022
@DanTup DanTup added in editor Relates to code editing or language features in commands Relates to commands (usually invoked from the command Palette) labels May 30, 2022
@p4-k4
Copy link
Author

p4-k4 commented May 30, 2022

Those all look good, each acknowledging the various use cases. allIfDirty would directly address what I had mentioned. Cool!

@DanTup DanTup closed this as completed in 7ef6828 May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in commands Relates to commands (usually invoked from the command Palette) in editor Relates to code editing or language features is enhancement
Projects
None yet
Development

No branches or pull requests

2 participants