feat(notes): link notes to each other with markdown links #1973
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Notes\Service; | ||
|
|
||
| use OCP\IURLGenerator; | ||
| use Psr\Log\LoggerInterface; | ||
|
|
||
| /** | ||
| * Keeps the visible label of note-to-note links in step with the note's title. | ||
| * | ||
| * Notes are linked with ordinary markdown links whose target is the note's id: | ||
| * | ||
| * [Shopping list](/index.php/apps/notes/note/42) | ||
| * | ||
| * Because the target is an id, renaming note 42 can never break a link to it — | ||
| * only the label goes stale. This service refreshes those labels. | ||
| * | ||
| * Two deliberate limits, because this edits notes the user is not looking at: | ||
| * | ||
| * - Only labels that still match the *old* title are rewritten. If somebody | ||
| * wrote `[my weekly shop](…/note/42)` that is their wording, not a stale | ||
| * copy of the title, and it is left alone. | ||
| * - Only explicit renames call this. Titles also change through `autotitle`, | ||
| * which fires while a new note is being typed, and rewriting the whole | ||
| * collection on every one of those would be both wasteful and surprising. | ||
| */ | ||
| class NoteLinkService { | ||
| public function __construct( | ||
| private NotesService $notesService, | ||
| private IURLGenerator $urlGenerator, | ||
| private LoggerInterface $logger, | ||
| ) { | ||
| } | ||
|
|
||
| /** | ||
| * Rewrite `[$oldTitle](<link to $noteId>)` to use `$newTitle`. | ||
| * | ||
| * Never throws: a rename must not fail because a link could not be tidied | ||
| * up. Notes that cannot be read or written are skipped. | ||
| * | ||
| * @return int number of notes changed | ||
| */ | ||
| public function refreshLinkLabels(string $userId, int $noteId, string $oldTitle, string $newTitle): int { | ||
| if ($oldTitle === '' || $oldTitle === $newTitle) { | ||
| return 0; | ||
| } | ||
|
|
||
| $pattern = $this->linkPattern($noteId, $oldTitle); | ||
| // ${1} is the target, ${2} any trailing slashes it was written with | ||
| $replacement = '[' . $this->escapeReplacement($newTitle) . '](${1}${2})'; | ||
|
Contributor
There was a problem hiding this comment. I'm worried about this replacement. What if I edit a note to a title with containing
Contributor
There was a problem hiding this comment. escapeReplacement only escapes the preg_replace specials
Contributor
There was a problem hiding this comment. maybe adding something like the replace |
||
| $changed = 0; | ||
|
|
||
| foreach ($this->notesService->getAll($userId)['notes'] as $note) { | ||
| if ($note->getId() === $noteId) { | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| $content = $note->getContent(); | ||
| $updated = preg_replace($pattern, $replacement, $content); | ||
| if ($updated === null || $updated === $content) { | ||
| continue; | ||
| } | ||
| $note->setContent($updated); | ||
|
Contributor
There was a problem hiding this comment. do we need to lock while doing this or nah? why or why not? |
||
| $changed++; | ||
| } catch (\Throwable $e) { | ||
| // a read-only note, or one that vanished mid-walk | ||
| $this->logger->debug('Could not refresh note links in ' . $note->getId(), ['exception' => $e]); | ||
| } | ||
| } | ||
|
|
||
| return $changed; | ||
| } | ||
|
|
||
| /** | ||
| * Matches a markdown link whose label is $oldTitle and whose target is any | ||
| * spelling of the route to $noteId — absolute or root-relative, with or | ||
| * without the /index.php prefix. | ||
| */ | ||
| private function linkPattern(int $noteId, string $oldTitle): string { | ||
| $targets = []; | ||
| foreach (['/apps/notes/note/', '/index.php/apps/notes/note/'] as $path) { | ||
| $absolute = $this->urlGenerator->getAbsoluteURL($path . $noteId); | ||
| $targets[] = $absolute; | ||
| $relative = parse_url($absolute, PHP_URL_PATH); | ||
| if (is_string($relative) && $relative !== '') { | ||
| $targets[] = $relative; | ||
| } | ||
| } | ||
|
|
||
| $targets = array_map( | ||
| static fn (string $target): string => preg_quote($target, '/'), | ||
| array_values(array_unique($targets)), | ||
| ); | ||
|
|
||
| return '/\[' . preg_quote($oldTitle, '/') . '\]\(\s*(' . implode('|', $targets) . ')(\/*)\s*\)/u'; | ||
|
Contributor
There was a problem hiding this comment. same here. What if i have a note titled |
||
| } | ||
|
|
||
| /** | ||
| * `$` and `\` carry meaning in a preg_replace replacement, so a title | ||
| * containing them would otherwise corrupt the link. | ||
| */ | ||
| private function escapeReplacement(string $value): string { | ||
| return str_replace(['\\', '$'], ['\\\\', '\\$'], $value); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| import { generateUrl } from '@nextcloud/router' | ||
|
|
||
| /** | ||
| * Notes are linked with ordinary markdown links pointing at 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. The target is the | ||
| * file id rather than the title, so renaming a note cannot break a link to it. | ||
| * | ||
| * This mirrors the URL NoteReferenceProvider already matches on the server, so | ||
| * a link pasted into Talk or a Text document still resolves to a rich preview. | ||
| */ | ||
|
|
||
| /** | ||
| * @param {number|string} noteId id of the note to link to | ||
| * @return {string} URL for a markdown link | ||
| */ | ||
| export function noteLinkUrl(noteId) { | ||
| return generateUrl('apps/notes/note/{noteId}', { noteId }) | ||
| } | ||
|
|
||
| /** | ||
| * A markdown link to a note, ready to paste into another note. | ||
|
Contributor
There was a problem hiding this comment. Is it really ready to past into another note? Cuz this gives a relative link that is likely not immediately useful 🤔 |
||
| * | ||
| * @param {object} note the note to link to | ||
| * @return {string} markdown | ||
| */ | ||
| export function noteLinkMarkdown(note) { | ||
| // ']' would end the label early, so it is the one character worth escaping | ||
| const label = String(note?.title ?? '').replaceAll(']', '\\]') | ||
|
|
||
| return `[${label}](${noteLinkUrl(note.id)})` | ||
| } | ||
|
|
||
| /** | ||
| * The note a link points at, or null when it points somewhere else. | ||
| * | ||
| * Accepts every shape the link may have been written in — absolute or | ||
| * root-relative, with or without the `/index.php` prefix, and under a | ||
| * subdirectory install — by resolving it and matching on the path. Links to | ||
| * another origin are never treated as note links, so a URL that merely looks | ||
| * like one cannot make Notes navigate. | ||
| * | ||
| * @param {string} href the link target | ||
| * @return {number|null} note id, or null | ||
| */ | ||
| export function parseNoteLink(href) { | ||
| if (!href) { | ||
| return null | ||
| } | ||
|
|
||
| let url | ||
| try { | ||
| url = new URL(href, window.location.href) | ||
| } catch { | ||
| return null | ||
| } | ||
|
|
||
| if (url.origin !== window.location.origin) { | ||
| return null | ||
| } | ||
|
|
||
| const match = url.pathname.match(/\/apps\/notes\/note\/(\d+)\/*$/) | ||
|
|
||
| return match ? Number(match[1]) : null | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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(...)andlinkPattern(...)sit outside the per note try/catch, and they could throw