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 support for automatic reloading the config when it has been changed (based on #50) #69

Closed
wants to merge 2 commits into from

Conversation

marcusramberg
Copy link
Contributor

Automatic reloading is very useful, and I've been using it locally, but #50 seems to be abandoned, so I forked it and tried to address the remaining review comments.

Copy link
Owner

@muesli muesli left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up! Left a few more remarks.

deck.go Show resolved Hide resolved
deck.go Outdated Show resolved Hide resolved
deck.go Outdated Show resolved Hide resolved
deck.go Outdated Show resolved Hide resolved
deck.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@muesli
Copy link
Owner

muesli commented Feb 8, 2022

A general note: file watching is difficult and I fear there's a bit more work required to make this work universally. Depending on the editor you use to edit a .deck file, the file watcher can actually break deckmaster. The reason for that is that some editors don't just overwrite a file, they actually remove the old file and then rename a temporary file to match the name of the edited file. In this case we'll receive an fsnotify event, but loading the deck again will fail as the file has vanished.

Solving this is tricky: you'll have to watch the entire directory the .deck file resides in to monitor things like deletions, file creations and move operations.

@muesli muesli added the enhancement New feature or request label Feb 8, 2022
@marcusramberg
Copy link
Contributor Author

marcusramberg commented Feb 8, 2022

A general note: file watching is difficult and I fear there's a bit more work required to make this work universally. Depending on the editor you use to edit a .deck file, the file watcher can actually break deckmaster. The reason for that is that some editors don't just overwrite a file, they actually remove the old file and then rename a temporary file to match the name of the edited file. In this case we'll receive an fsnotify event, but loading the deck again will fail as the file has vanished.

Solving this is tricky: you'll have to watch the entire directory the .deck file resides in to monitor things like deletions, file creations and move operations.

I'm not sure which editors this would be a problem for, only been testing on nvim and it seems to work very consistently. Did a test autoincrementing a number 250 times (mostly to check that we're not leaking on reload), and seemed to reload consistently every time. An idea to address this issue could be adding a small delay (50-100ms?) (incremented every new event) before saving?

@marcusramberg
Copy link
Contributor Author

I guess there's no interest in progressing this PR so I will just close it.

@muesli
Copy link
Owner

muesli commented Dec 23, 2022

Yeah, to be honest I'm not sure it's worth properly solving this. It would require a bunch of nasty code with tons of (platform and filesystem specific) edge-cases. Thanks for looking into it, tho!

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

Successfully merging this pull request may close these issues.

2 participants