Skip to content

Conversation

@nshcr
Copy link
Contributor

@nshcr nshcr commented May 3, 2025

No description provided.

@pjkaufman
Copy link
Collaborator

Hey @nshcr , this seems fine to me. The only real issue I see is that the user's data.json values are not getting mapped accordingly, so this could cause users to lose their setting data. Here is one such example of moving the settings data in the current logic:

obsidian-linter/src/main.ts

Lines 636 to 642 in b4dd3d9

// move a recently moved setting to its new location
if ('lintOnFileContentChangeDelay' in this.settings) {
this.settings.ruleConfigs['yaml-timestamp']['update-on-file-contents-updated'] = this.settings['lintOnFileContentChangeDelay'];
delete this.settings['lintOnFileContentChangeDelay'];
updateMade = true;
}

Would you want to add that logic to this PR?

@nshcr
Copy link
Contributor Author

nshcr commented May 11, 2025

@pjkaufman Thanks for reviewing the code!
I forgot to handle user data migration, that was my oversight. I've added the migration logic as you suggested and tested it in my local vault, where the old settings are correctly replaced.
Let me know if anything else needs adjusting, I'm happy to keep improving the PR.

@pjkaufman
Copy link
Collaborator

This looks good @nshcr . I will go ahead and merge this. Thanks for making the change.

@pjkaufman pjkaufman merged commit 06fc2f3 into platers:master May 11, 2025
1 check passed
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.

2 participants