Skip to content

Migrate markdown scopes #2043

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

Merged
merged 2 commits into from
Nov 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ const textFragmentExtractors: Record<
jsonStringTextFragmentExtractor,
),
latex: fullDocumentTextFragmentExtractor,
markdown: fullDocumentTextFragmentExtractor,
ruby: constructDefaultTextFragmentExtractor(
"ruby",
rubyStringTextFragmentExtractor,
Expand Down
39 changes: 2 additions & 37 deletions packages/cursorless-engine/src/languages/markdown.ts
Original file line number Diff line number Diff line change
@@ -1,48 +1,19 @@
import { Range, Selection, TextEditor } from "@cursorless/common";
import { Range, SimpleScopeTypeType, TextEditor } from "@cursorless/common";
import type { SyntaxNode } from "web-tree-sitter";
import { getMatchesInRange } from "../util/getMatchesInRange";
import { SimpleScopeTypeType } from "@cursorless/common";
import {
NodeFinder,
NodeMatcherAlternative,
SelectionWithContext,
} from "../typings/Types";
import { getMatchesInRange } from "../util/getMatchesInRange";
import { leadingSiblingNodeFinder, patternFinder } from "../util/nodeFinders";
import { createPatternMatchers, matcher } from "../util/nodeMatchers";
import {
extendUntilNextMatchingSiblingOrLast,
getNodeRange,
selectWithLeadingDelimiter,
} from "../util/nodeSelectors";
import { shrinkRangeToFitContent } from "../util/selectionUtils";

/**
* Given a node representing the text of a section heading (without leading
* marker), will return the content range as the text without the leading
* whitespace, and the outside range includes the leading marker, so that
* "chuck name" deletes the heading
* @param editor The editor containing the node
* @param node The node to extract from; will be the content of the heading without the leading marker
* @returns The selection with context
*/
function nameExtractor(
editor: TextEditor,
node: SyntaxNode,
): SelectionWithContext {
const range = getNodeRange(node);
const contentRange = range.isEmpty
? range
: range.with(range.start.translate(0, 1));
const removalRange = getNodeRange(node.parent!);

return {
selection: new Selection(contentRange.start, contentRange.end),
context: {
removalRange,
},
};
}

const HEADING_MARKER_TYPES = [
"atx_h1_marker",
"atx_h2_marker",
Expand Down Expand Up @@ -139,12 +110,6 @@ function itemExtractor(
const nodeMatchers: Partial<
Record<SimpleScopeTypeType, NodeMatcherAlternative>
> = {
list: ["list"],
comment: "html_block",
name: matcher(
leadingSiblingNodeFinder(patternFinder("atx_heading[heading_content]")),
nameExtractor,
),
collectionItem: matcher(patternFinder("list_item.paragraph!"), itemExtractor),
section: sectionMatcher("atx_heading"),
sectionLevelOne: sectionMatcher("atx_heading.atx_h1_marker"),
Expand Down
19 changes: 19 additions & 0 deletions queries/markdown.scm
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
(document) @textFragment

(html_block) @comment

;;!! * hello * stuff
;;! ^^^^^^^^^^^^^^^
(list) @list

;;!! # Title
;;! ^^^^^
;;!! xxxxxxx
;;! -------
(section
(atx_heading
(_)
heading_content: (_) @name
) @_.removal
(#shrink-to-match! @name "^\\s*(?<keep>.*)$")
) @_.domain
Comment on lines +13 to +19
Copy link
Member

Choose a reason for hiding this comment

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

Huh is it really this simple? I guess I only needed the leading sibling finder for when we were searching for a section heading of a specific level? If so I'd be tempted to just drop support for specific levels; I can't imagine anyone is using that 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

Oh I guess this is technically wrong. I believe "grand name" would be incorrect, because the domain should include all subheadings

Copy link
Member Author

Choose a reason for hiding this comment

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

All the test passed. That was kind of my benchmark.

I would be happy to drop the specific section levels.

I think that is something we can worry about when we try to implement that logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Already looks like subsections are nested

(document
  (section
    (atx_heading
      (atx_h1_marker)
      heading_content: (inline)
    )
    (paragraph
      (inline)
    )
    (section
      (atx_heading
        (atx_h2_marker)
        heading_content: (inline)
      )
      (paragraph
        (inline)
      )
    )
  )
)

Copy link
Member

Choose a reason for hiding this comment

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

Oh amazing. I believe all that sibling logic is a holdover from the old markdown parser we used to use then. We should be able to drop most of that and do something much simpler now