-
Notifications
You must be signed in to change notification settings - Fork 53
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
Fix: jumpToHeading shouldn't match within codeblocks (also improve jumpToLink a bit) #233
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,24 @@ | ||
import { jumpToPattern } from "../utils/jumpToPattern"; | ||
import { Editor as CodeMirrorEditor } from "codemirror"; | ||
import { EditorPosition } from "obsidian"; | ||
import { isWithinMatch, jumpToPattern } from "../utils/jumpToPattern"; | ||
import { MotionFn } from "../utils/vimApi"; | ||
|
||
const HEADING_REGEX = /^#+ /gm; | ||
/** Naive Regex for a Markdown heading (H1 through H6). "Naive" because it does not account for | ||
* whether the match is within a codeblock (e.g. it could be a Python comment, not a heading). | ||
*/ | ||
const NAIVE_HEADING_REGEX = /^#{1,6} /gm; | ||
|
||
/** Regex for a Markdown fenced codeblock, which begins with some number >=3 of backticks at the | ||
* start of a line. It either ends on the nearest future line that starts with at least as many | ||
* backticks (\1 back-reference), or extends to the end of the string if no such future line exists. | ||
*/ | ||
const FENCED_CODEBLOCK_REGEX = /(^```+)(.*?^\1|.*)/gms; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small note that the |
||
|
||
/** | ||
* Jumps to the repeat-th next heading. | ||
*/ | ||
export const jumpToNextHeading: MotionFn = (cm, cursorPosition, { repeat }) => { | ||
return jumpToPattern({ | ||
cm, | ||
cursorPosition, | ||
repeat, | ||
regex: HEADING_REGEX, | ||
direction: "next", | ||
}); | ||
return jumpToHeading({ cm, cursorPosition, repeat, direction: "next" }); | ||
}; | ||
|
||
/** | ||
|
@@ -24,11 +29,47 @@ export const jumpToPreviousHeading: MotionFn = ( | |
cursorPosition, | ||
{ repeat } | ||
) => { | ||
return jumpToHeading({ cm, cursorPosition, repeat, direction: "previous" }); | ||
}; | ||
|
||
/** | ||
* Jumps to the repeat-th heading in the given direction. | ||
* | ||
* Under the hood, we use the naive heading regex to find all headings, and then filter out those | ||
* that are within codeblocks. `codeblockMatches` is passed in a closure to avoid repeated | ||
* computation. | ||
*/ | ||
function jumpToHeading({ | ||
cm, | ||
cursorPosition, | ||
repeat, | ||
direction, | ||
}: { | ||
cm: CodeMirrorEditor; | ||
cursorPosition: EditorPosition; | ||
repeat: number; | ||
direction: "next" | "previous"; | ||
}): EditorPosition { | ||
const codeblockMatches = findAllCodeblocks(cm); | ||
const filterMatch = (match: RegExpExecArray) => !isMatchWithinCodeblock(match, codeblockMatches); | ||
return jumpToPattern({ | ||
cm, | ||
cursorPosition, | ||
repeat, | ||
regex: HEADING_REGEX, | ||
direction: "previous", | ||
regex: NAIVE_HEADING_REGEX, | ||
filterMatch, | ||
direction, | ||
}); | ||
}; | ||
} | ||
|
||
function findAllCodeblocks(cm: CodeMirrorEditor): RegExpExecArray[] { | ||
const content = cm.getValue(); | ||
return [...content.matchAll(FENCED_CODEBLOCK_REGEX)]; | ||
} | ||
|
||
function isMatchWithinCodeblock( | ||
match: RegExpExecArray, | ||
codeblockMatches: RegExpExecArray[] | ||
): boolean { | ||
return codeblockMatches.some((codeblockMatch) => isWithinMatch(codeblockMatch, match.index)); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,23 @@ | ||
import { jumpToPattern } from "../utils/jumpToPattern"; | ||
import { MotionFn } from "../utils/vimApi"; | ||
|
||
const WIKILINK_REGEX_STRING = "\\[\\[[^\\]\\]]+?\\]\\]"; | ||
const MARKDOWN_LINK_REGEX_STRING = "\\[[^\\]]+?\\]\\([^)]+?\\)"; | ||
const LINK_REGEX_STRING = `${WIKILINK_REGEX_STRING}|${MARKDOWN_LINK_REGEX_STRING}`; | ||
const WIKILINK_REGEX_STRING = "\\[\\[.*?\\]\\]"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also went ahead and improved the regexes here a little to be more concise and more accurate (e.g. I don't think the original |
||
const MARKDOWN_LINK_REGEX_STRING = "\\[.*?\\]\\(.*?\\)"; | ||
const URL_REGEX_STRING = "\\w+://\\S+"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept this at a simple URL regex - technically URL validation is way more complex than this, but I think that could affect both code readability and potentially regex performace. This won't match urls without protocols (e.g. won't match |
||
|
||
/** | ||
* Regex for a link (which can be a wikilink, a markdown link, or a standalone URL). | ||
*/ | ||
const LINK_REGEX_STRING = `${WIKILINK_REGEX_STRING}|${MARKDOWN_LINK_REGEX_STRING}|${URL_REGEX_STRING}`; | ||
const LINK_REGEX = new RegExp(LINK_REGEX_STRING, "g"); | ||
|
||
/** | ||
* Jumps to the repeat-th next link. | ||
* | ||
* Note that `jumpToPattern` uses `String.matchAll`, which internally updates `lastIndex` after each | ||
* match; and that `LINK_REGEX` matches wikilinks / markdown links first. So, this won't catch | ||
* non-standalone URLs (e.g. the URL in a markdown link). This should be a good thing in most cases; | ||
* otherwise it could be tedious (as a user) for each markdown link to contain two jumpable spots. | ||
*/ | ||
export const jumpToNextLink: MotionFn = (cm, cursorPosition, { repeat }) => { | ||
return jumpToPattern({ | ||
|
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.
Also went ahead and adjusted the heading regex to only match H1-H6, since H7/above don't exist in Markdown