When including note with links to non-existent files, getting "cyclic link detected" warning

[Aristarkh4]

Describe the bug

Attempting to include a note via ![[included note]] syntax. The included note contains placeholder link to a non-existent note. That results in the original note not being rendered and producing a "Cyclic link detected for wikilink: included note" warning.

Small Reproducible Example

https://github.com/Aristarkh4/foam-inclusion-nonexistenfiles-bug/tree/master

Steps to Reproduce the Bug or Issue

1. Get the bug example repository.
2. Open in VSCode with Foam extension installed
3. Open "montly.md" note
4. Open the markdown preview of the note.

Expected behavior

As a user I would expect the inclusion of the note to be successful with placeholder link being rendered as it would be rendered in a greyed-out state. Instead, I'm getting the "Cyclic link detected for wikilink: included note" warning.

Screenshots or Videos

The "monthly" note:

The "weekly" code (included note):

The failing preview of "monthly" note:

Operating System Version

OS: Windows

Visual Studio Code Version

1.81.1

Additional context

No response

[riccardoferretti]

Thanks for reporting the issue. As we recently changed the code in that area let me check with @badsketch in case this rings a bell

[badsketch]

Nice catch! I took a look and it seems to stem from an edge case when extracting text for a note during embedding. Wikilinks that are part of the note we are embedding get converted to a link relative to the current note using withLinksRelativeToWorkspaceRoot(). The function doesn't handle a case when the note is non-existent. As a result, the exception bubbles up to wikilink-embed.ts and we never refsStack.pop(), so refsStack retains existing notes and leads to cases like the description.

1. A quick fix is we simply no-op when it encounters a wikilink that hasn't been created yet. So instead of converting the wikilink to a link to the note, it would literally be rendered to [[daily1]] as text.

2. A better approach might be what we do in wikilink-navigation.ts where we generate a placeholder link if it's not found. It gets a little hairy since we need to do a text replace and that involves working with ranges. Might take someone like me a little longer, if that's okay. 🙂

Also related thought - haven't tested this, but do our panels (orphans, placeholders, connections) properly ignore elements in embedded notes? (ex. if a.md has 2 placeholder wikilinks and b.md embeds a.md, then our placeholders panel won't be double counting the ones in b.md resulting in 4 placeholders, will they?

[riccardoferretti]

Thanks for properly dissecting what's happening there, makes sense.
I agree approach n.2 looks more solid, I think it's worth the extra complexity.
Let me know if you need any support, thanks!

[badsketch]

I realized we currently return the bare text if the note doesn't exist, so in the case of embedding a note which is embedding another note that doesn't exist, maybe n.1 would be more consistent with existing behavior after all?

A placeholder for both ![[does-not-exist]] and ![[does-not-exist-either]] would be consistent and look nicer, but maybe we don't want it to look nice? Seeing a bare unrendered ![[note]] in the preview would encourage the user to create the new note whereas a placeholder might end up overlooked because it blends in so well. wdyt?

[riccardoferretti]

I got confused between the wikilink and the embed.
I wonder when a user would embed a placeholder, but let's assume there are good use cases.

To me there are 2 options at that point:

1. we keep the text as is (what you suggested). A bit lighter weight
2. we create an embed style for the placeholder (similar to what we do for attachments). A bit heavier, but also conveys the intention of the user of "having some content embedded there" (I hope I am making sense)

Given that an embed is not a link I totally agree we shouldn't just turn the placeholder into a link.

Thoughts?

[badsketch]

Both options make sense to me!

Happy to implement n.2, but figured it would take more discussions and design iterations 🙂

Raised a PR implementing n.1 as a quick fix since I think the issue could be annoying in a case where a user accidentally embeds a note that doesn't exist. Then they create it. But now the ref stack permanently has this note in there. Now any new note that tries to embed the note will result in a cyclic link error unless they restart Foam.