feat: remove frontmatter

[Mara-Li]

Using last version of ObsidianAPI, the use of getFrontMatterInfo allow to remove easily the frontmatter

See #309

[Mara-Li]

I noticed the way you handle the frontmatter to find frontmatter tags. We need to use the frontmatter API to get it, as parsing with --- can rapidly lead to error (for example, wrong frontmatter, text starting with line, or simply a header with --- syntax). Typically, a file without frontmatter with lead to an error

[logancyang]

I noticed the way you handle the frontmatter to find frontmatter tags. We need to use the frontmatter API to get it, as parsing with --- can rapidly lead to error (for example, wrong frontmatter, text starting with line, or simply a header with --- syntax). Typically, a file without frontmatter with lead to an error

I agree. The util from their API must be doing better handling for sure. It just doesn't justify a breaking change IMHO. Most users with older versions of Obsidian will simply think the plugin is broken and they won't actively go and find our message "you have to upgrade your Obsidian to continue using it". So the bad UX falls onto the plugin.

I'm pushing v2.5.0 soon with Vault QA mode. Perhaps that could justify a breaking change. But in general, I really dislike breaking changes from an API that does not come from a major version bump.

[Mara-Li]

No worry, I totally understand. That's why I changed for another method that use less recent API (reading directly the frontmatter using app.metadatache)

[logancyang]

Thanks! Could you rebase on master and update the tests accordingly? Should be good to merge after.

[Mara-Li]

For the test, I don't know how to fix them, to be honest!

[Mara-Li]

Also, getMarkdownFiles doesn't need to be async, why do you use await here?

[Mara-Li]

Hello, I added the last suggestion in #309 I don't know how to update the test tbh

[logancyang]

@Lisandra-dev thanks for the update.

1. Were you able to pull from master and rebase? This PR shows changes from master, but it should not.
2. You can update the tests and add more test cases for your change, since the function is updated to handle these cases better. It's helpful to check for cases where it failed with the old way and works with your new change. If you don't know how to write those tests, you can just ask GPT4. It's quite good at it.

[Mara-Li]

1. What do you mean?
2. The problem is that I need to integrate the App in mocks and i really don't know how to do it... When I try to just call Obsidian.App() I have App is not a constructor :/

[logancyang]

1. What do you mean?
2. The problem is that I need to integrate the App in mocks and i really don't know how to do it... When I try to just call Obsidian.App() I have App is not a constructor :/

1. This PR should contain only your changes, not mine. For example, tests/customPromptProcessor.test.ts and a lot of other file changes are merged from master. You can rebase your commits on top of the newest master so those changes shouldn't be in this PR anymore.
2. The obsidian mock is inside __mocks__/obsidian.js, you can add your mocks as needed.

[Mara-Li]

As I said, I don't know how to create __mocks__

[Noobgam]

@Lisandra-dev as for mocking, I took notice of your PR here and merged a rough sketch into my fork. (thanks for your contribution)

https://github.com/Noobgam/obsidian-copilot/pull/35/files#diff-fd006daabf381c84fffc8940f478f913bb04353b644854edb10d2d100586cf70

You can try to follow the same steps and see how far you can get, I didn't focus on tests much, but the PR includes basic mocking of metadataCache that will allow you to do anything you need (and 1 simple happy-path test)

[Noobgam]

Also, getMarkdownFiles doesn't need to be async, why do you use await here?

To answer your previous question since Logan didn't: It is a bug in his mocking and lack of linter setup in the repository. The method signature is not async, whereas mocks defined are async. This is a bug and in order to fix this you need to change the way mocks are implemented. At runtime there isn't a big difference when you have this additional await, but it does get highlighted in most IDEs automatically.

You can also take a look at the PR in my fork, that issue is addressed there as well (the main difference in that regard is removed Promise.resolve from mocks)