Skip to content

feat(notes): link notes to each other with markdown links - #1973

Open
karlitschek wants to merge 1 commit into
mainfrom
feat/noid/note-links
Open

feat(notes): link notes to each other with markdown links#1973
karlitschek wants to merge 1 commit into
mainfrom
feat/noid/note-links

Conversation

@karlitschek

Copy link
Copy Markdown
Member

There was no way to link one note to another. Now there is, using ordinary markdown links whose target is the note's route:

[Shopping list](/index.php/apps/notes/note/42)

No custom syntax, so every editor renders them — the rich editor, the markdown preview, and anything else that reads the file. This is the same URL NoteReferenceProvider already matches, so a link pasted into Talk or a Text document still resolves to a rich preview.

The target is the file id rather than the title, so renaming a note cannot break a link to it. Only the visible label goes stale.

  • "Copy link to note" in the note's action menu puts the markdown on the clipboard, label included, so linking is a paste rather than hand-writing a URL. It sits in the note list row menu, which is present in every editor mode.
  • Clicking a note link routes inside the app instead of reloading the page. In rich mode that goes through Text's openLinkHandler hook; note that providing a handler replaces Text's default, so the fallback for every other link — open in a new tab — is reimplemented rather than lost. In preview mode the click is delegated from the preview container, because the rendered HTML is replaced wholesale on every edit.
  • Modified clicks (ctrl, meta, shift, alt, middle button) are left alone so "open in new tab" keeps working, and links to another origin are never treated as note links.

Stale labels are refreshed on rename (NoteLinkService), with two limits, because this edits notes the user is not looking at:

  • Only labels that still match the old title are rewritten. A link written as my weekly shop is the author's wording, not a stale copy of the title, and is left alone.
  • Only explicit renames trigger it. Titles also change through autotitle, which fires while a new note is being typed, and sweeping the whole collection on each of those would be wasteful and surprising. The two are separate controller paths, so only updateProperty('title') is hooked.

A rename therefore costs one pass over the notes folder. That is the same O(n) content read as search, acceptable for an explicit, infrequent action.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

There was no way to link one note to another. Now there is, using ordinary
markdown links whose target is the note's route:

    [Shopping list](/index.php/apps/notes/note/42)

No custom syntax, so every editor renders them — the rich editor, the
markdown preview, and anything else that reads the file. This is the same URL
NoteReferenceProvider already matches, so a link pasted into Talk or a Text
document still resolves to a rich preview.

The target is the file id rather than the title, so renaming a note cannot
break a link to it. Only the visible label goes stale.

* "Copy link to note" in the note's action menu puts the markdown on the
  clipboard, label included, so linking is a paste rather than hand-writing a
  URL. It sits in the note list row menu, which is present in every editor
  mode.
* Clicking a note link routes inside the app instead of reloading the page. In
  rich mode that goes through Text's openLinkHandler hook; note that providing
  a handler replaces Text's default, so the fallback for every other link —
  open in a new tab — is reimplemented rather than lost. In preview mode the
  click is delegated from the preview container, because the rendered HTML is
  replaced wholesale on every edit.
* Modified clicks (ctrl, meta, shift, alt, middle button) are left alone so
  "open in new tab" keeps working, and links to another origin are never
  treated as note links.

Stale labels are refreshed on rename (NoteLinkService), with two limits,
because this edits notes the user is not looking at:

* Only labels that still match the old title are rewritten. A link written as
  [my weekly shop](…/note/42) is the author's wording, not a stale copy of the
  title, and is left alone.
* Only explicit renames trigger it. Titles also change through autotitle,
  which fires while a new note is being typed, and sweeping the whole
  collection on each of those would be wasteful and surprising. The two are
  separate controller paths, so only updateProperty('title') is hooked.

A rename therefore costs one pass over the notes folder. That is the same
O(n) content read as search, acceptable for an explicit, infrequent action.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@karlitschek
karlitschek requested review from joshtrichards and silverkszlo and a lite review from Copilot and removed request for enjeck, joshtrichards and silverkszlo August 6, 2026 18:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@enjeck

enjeck commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Might fix #1131 and #951 and #1524

@enjeck enjeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, it would be nice to have this as it's a popular feature request!

Some questions:


$pattern = $this->linkPattern($noteId, $oldTitle);
// ${1} is the target, ${2} any trailing slashes it was written with
$replacement = '[' . $this->escapeReplacement($newTitle) . '](${1}${2})';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm worried about this replacement. What if I edit a note to a title with containing ]?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

escapeReplacement only escapes the preg_replace specials $ and \. It does not escape ]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe adding something like the replace [ in noteLinkMarkdown helps

array_values(array_unique($targets)),
);

return '/\[' . preg_quote($oldTitle, '/') . '\]\(\s*(' . implode('|', $targets) . ')(\/*)\s*\)/u';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here. What if i have a note titled Test]

Comment on lines +45 to +46
* Never throws: a rename must not fail because a link could not be tidied
* up. Notes that cannot be read or written are skipped.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure about this. getAll(...) and linkPattern(...) sit outside the per note try/catch, and they could throw

if ($updated === null || $updated === $content) {
continue;
}
$note->setContent($updated);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need to lock while doing this or nah? why or why not?

Comment thread src/noteLinks.js
}

/**
* A markdown link to a note, ready to paste into another note.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it really ready to past into another note? Cuz this gives a relative link that is likely not immediately useful 🤔

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.

3 participants