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

Support new and old frontmatter api #100

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xt0rted
Copy link

@xt0rted xt0rted commented Sep 3, 2023

The current implementation works fine in the desktop app, but the mobile app still uses the old api. I patched my local copy and this seems to work on both now.

Fixes #99

@@ -28,7 +28,7 @@ export default class MetaEditParser {
if (!frontmatter) return [];

//@ts-ignore - this is part of the new Obsidian API as of v1.4.1
const {start, end} = fileCache?.frontmatterPosition;
const {start, end} = fileCache?.frontmatterPosition ?? fileCache?.frontmatter?.position;
Copy link
Author

@xt0rted xt0rted Sep 3, 2023

Choose a reason for hiding this comment

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

This isn't ideal because if anything's null/undefined then destructuring will throw an exception, but that would happen even before this change.

FYI: v1.4.0 of the obsidian types includes both of these properties now obsidianmd/obsidian-api@90517f4

@xt0rted
Copy link
Author

xt0rted commented Sep 3, 2023

After some more testing this doesn't look like a full fix. I need to debug it some more to see what else needs to be adjusted.

@xt0rted
Copy link
Author

xt0rted commented Sep 3, 2023

What was happening is on desktop unset values were remaining unset, but on mobile they were coming through as a literal string of null, and a new position property was being added in. The second commit ensures the old code path is taken on mobile while desktop uses the new code path. The results look to be the same in both applications.

//@ts-ignore
const frontmatterPosition = this.app.metadataCache.getFileCache(file).frontmatterPosition;

if (property.type === MetaType.YAML && frontmatterPosition) {
const updatedMetaData = `---\n${this.updateYamlProperty(property, newValue, file)}\n---`;
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
const updatedMetaData = `---\n${this.updateYamlProperty(property, newValue, file)}\n---`;
const updatedMetaData = `---\n${this.updateYamlProperty(property, newValue, file)}---`;

On both desktop & mobile an extra blank line is being added to the frontmatter because of this \n. I removed it in my local copy but wasn't sure if it should be part of this PR.

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.

Latest release broken on Obsidian Mobile
1 participant