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

Add save on idle 🚀 #5303

Closed
wants to merge 5 commits into from
Closed

Add save on idle 🚀 #5303

wants to merge 5 commits into from

Conversation

ArchUsr64
Copy link
Contributor

  • Implements an auto-save like feature on idle timer trigger events.
  • Useful for immediate feedback from rust-analyzer as most diagnostics are only refreshed on file save.
  • Configured as a boolean idle-save in the [editor] section of the config file, false by default.

Video showcasing the feature with rust-analyzer language server (idle-timeout is set to 0)

2022-12-27.01-48-24.mp4

@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Dec 26, 2022
@the-mikedavis
Copy link
Member

There was some discussion on the PR that added auto-save on focus loss: #3178 (comment)

This could be added to the auto-save configuration so that you could configure either or both events for auto saving:

auto-save = ["focus-lost", "idle"]

I'm also thinking that the idle-timeout is too short for this - it will save very aggressively. An auto-save timer should probably have a much longer duration like 10+ seconds

@ArchUsr64 ArchUsr64 marked this pull request as draft December 27, 2022 17:47
@ArchUsr64
Copy link
Contributor Author

This could be added to the auto-save configuration so that you could configure either or both events for auto saving:

auto-save = ["focus-lost", "idle"]

I'm also thinking that the idle-timeout is too short for this - it will save very aggressively. An auto-save timer should probably have a much longer duration like 10+ seconds

Yeah, I think having a separate timer and all relevant configuration being done in auto-save option would be preferred.

Though I'm not sure what should be the behavior in case of a boolean value for auto-save, which should be handled properly in order to not break existing configurations.

Perhaps, true could just be an alias for current behavior of focus-lost and have false disable both to avoid interference with existing configs.

@ChemicalXandco ChemicalXandco mentioned this pull request Dec 31, 2022
@the-mikedavis the-mikedavis linked an issue Dec 31, 2022 that may be closed by this pull request
@kirawi kirawi added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels May 13, 2023
@ArchUsr64 ArchUsr64 closed this by deleting the head repository Jul 22, 2023
@connortsui20
Copy link
Contributor

What was the reason why this was dropped? This seems like a good change...

@pascalkuthe
Copy link
Member

Author probably lost interest. Either way I do think this needs a seperate timers, piling more on idle tineout is not a good idea and idle-timeout is also way to short for autosave. I agree with @the-mikedavis this needs atleast a 10s timer. Probably even longer. The config also needs to be changed as mentioned above.

For the timer this should probably be based on #8021 which makes it really easy to add new event based timers. In this case the timer should be reset whenever a document revision is created (saving in insert mode is broken so that should not.happen there).

@ArchUsr64
Copy link
Contributor Author

What was the reason why this was dropped? This seems like a good change...

The implementation was shotty at best, I'm looking to pick this back up once I have some time on hand.

@mersenne-twister
Copy link

mersenne-twister commented May 16, 2024

this needs atleast a 10s timer.

I personally feel that it would be great to have a shorter timer, of maybe a second or two, for the purpose of code diagnostics; though this will of course vary a lot.

Regardless, given this variation, I think it would definitely make sense to have it be a configuration option, with a reasonable default of 10+ s.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-save
6 participants