obsidian-tasks-group / obsidian-tasks

Task management for the Obsidian knowledge base.
https://publish.obsidian.md/tasks/
MIT License
2.51k stars 231 forks source link

Detect external settings changes and reload settings, for example when settings have been synced in from another machine #2684

Open claremacrae opened 8 months ago

claremacrae commented 8 months ago

⚠️ Please check that this feature request hasn't been suggested before.

🔖 Feature description

Currently it's easy for edits to settings to be lost when the any file-syncing service copies settings data between devices:

On Machine 1, save settings. Changes get synced to Machine 2.

Obsidian is not restarted on Machine 2, so the plugin is still running with the settings pre sync.

User changes settings on Machine 2. Settings from Machine 1 still not present. Settings get synced back to Machine 1.

Next time Obsidian is restarted on Machine 1, it has only the settings edits on Machine 2.

✔️ Solution

Detect changes in data.json file, and reload settings

❓ Alternatives

No response

📝 Additional Context

This is really a placeholder for me to capture useful information on ways to implement this.

  1. Obsidian 1.5.7 API added a way to detect external changes to settings: https://github.com/obsidianmd/obsidian-api/blob/master/CHANGELOG.md#v157

    image

  2. javalent's Calendarium plugin implements the listening like this:

    https://github.com/javalent/calendarium/blob/c97948a4966c88b6fdd520995a52b258380ef5f6/src/settings/settings.service.ts#L120-L182

claremacrae commented 8 months ago

A side-effect of implementing this is it should be possible to remove the requirement to reload Obsidian when setting many of the settings in the Tasks plugin.

claremacrae commented 8 months ago

From liam in Discord

Yeah, you shouldn't need much, it's mainly a callback to update your in-memory settings and refresh any views that need refreshing

onExternalSettingsChange() {
    // reload in-memory settings
    this.settings = Object.assign({}, DEFAULT_SETTINGS, await this.loadData());

    // If you have views or other on-screen displays, refresh them...
}

javalent's code was written when the api was still a bit buggy so there's some overly defensive code there that couldn't be needed anymore

claremacrae commented 8 months ago

javalent's code was written when the api was still a bit buggy so there's some overly defensive code there that couldn't be needed anymore

Later comment:

that was a very old alpha build, I wouldnt worry about that

claremacrae commented 8 months ago

Additional comment Discord - about someone else's code:

Okay, I investigated and I can reproduce the issue. Your plugin is calling saveSettings on every keystroke, which causes a lot of file writes to happen very quickly. Our detection for if a change is internal or external is failing to account for this, so it thinks the file is being modified outside of obsidian, which is why you're seeing those calls to onExternalSettingsChange. I have some improvements to the detection algorithm for a future build, but in the meantime, I suggest you wrap your saveSettings function in a debounce to avoid this issue (and it's just good practice anyway!)