Note content pusher

[lizard-heart]

I am submitting a new Community Plugin

Repo URL

Link to my plugin: https://github.com/lizard-heart/obsidian-note-content-pusher

Release Checklist

• [x] I have tested the plugin on
  • [ ] Windows
  • [x] macOS
  • [ ] Linux
  • [ ] Android (if applicable)
  • [x] iOS (if applicable)
• [x] My GitHub release contains all required files
  • [x] main.js
  • [x] manifest.json
  • [ ] styles.css (optional)
• [x] GitHub release name matches the exact version number specified in my manifest.json (Note: Use the exact version number, don't include a prefix v)
• [x] The id in my manifest.json matches the id in the community-plugins.json file.
• [x] My README.md describes the plugin's purpose and provides clear usage instructions.
• [x] I have read the tips in https://github.com/obsidianmd/obsidian-releases/blob/master/plugin-review.md and have self-reviewed my plugin to avoid these common pitfalls.
• [x] I have added a license in the LICENSE file.
• [x] My project respects and is compatible with the original license of any code from other plugins that I'm using. I have given proper attribution to these other projects in my README.md.

[liamcain]

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L56 I was going to say there's no need to get the App instance from the window object, but looks like this line is unused. It can be removed.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L17-L27 I suggest removing this helper function. There's no need to keep accessing the app instance from the window object, and it's not doing anything other than wrapping the vault.create function

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L57 you will need to check that view isn't null.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L61-L65 don't use var, these should be let.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L118 if you use a regular for loop instead of forEach here, you can early exit so you don't need to keep iterating after you found your existing file.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L95-L125 you should be able to simplify all of this code by just using getFirstLinkpathDest. You'll be able to find the best file match without having to loop through all of the files yourself.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L42 You're calling this.automaticPush every time the metadataCache changes, but your never checking that the change in the metadataCache was for the active file. This means it's doing a lot of unnecessary work.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L195 view can be null

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L87 you're reading the contents from CodeMirror, but writing it directly to disk. You should use codemirror transactions instead of do all the modifications with the Editor instance. This will avoid issues with third-party sync and maintain the editor state so the user won't lose things like folds.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L146 since you're writing to this file, you should use read instead of cachedRead

[lizard-heart]

Thank you for the feedback. I have resolved the changes suggested in the newest release.

[liamcain]

var filesWithName you still have many instances of var. You can just replace all instances of var with let. In javascript, var has some often unintended behavior, so it's almost always a better idea to use let.

https://github.com/lizard-heart/obsidian-note-content-pusher/blob/b8ac45708a00e7afdd3f6b7feddca032756a80f8/src/main.ts#L95-L125 you should be able to simplify all of this code by just using getFirstLinkpathDest. You'll be able to find the best file match without having to loop through all of the files yourself.

It looks like this wasn't addressed. Did you run into issues using getFirstLinkpathDest?

[lizard-heart]

Yes, I wasn't able to get getFirstLinkpathDest to work with exactly the behavior I wanted. I have updated the newest release (1.0.4) to change all instances of var to let.

[liamcain]

Hi, I'm really sorry for such a long wait getting this merged! This pull request slipped through the cracks and I didn't see your last comment about it being ready for re-review.

---

So small bits of feedback.

• var there's still quite a few instances of var instead of let. I suggest changing those too.
• I mentioned it above, but I suggest looking into using the Editor for manipulating text instead of using Vault.modify. it will be a lot more efficient and have fewer issues (like user losing their cursor/fold position/selections.