Skip to content
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

Migrate collection item and argument scopes #2081

Merged
merged 22 commits into from
Dec 8, 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 @@ -214,6 +214,37 @@ class InsertionDelimiter extends QueryPredicateOperator<InsertionDelimiter> {
}
}

/**
* A predicate operator that sets the insertion delimiter of {@link nodeInfo} to
* either {@link insertionDelimiterConsequence} or
* {@link insertionDelimiterAlternative} depending on whether
* {@link conditionNodeInfo} is single or multiline, respectively. For example,
*
* ```scm
* (#single-or-multi-line-delimiter! @foo @bar ", " ",\n")
* ```
*
* will set the insertion delimiter of the `@foo` capture to `", "` if the
* `@bar` capture is a single line and `",\n"` otherwise.
*/
class SingleOrMultilineDelimiter extends QueryPredicateOperator<SingleOrMultilineDelimiter> {
name = "single-or-multi-line-delimiter!" as const;
schema = z.tuple([q.node, q.node, q.string, q.string]);

run(
nodeInfo: MutableQueryCapture,
conditionNodeInfo: MutableQueryCapture,
insertionDelimiterConsequence: string,
insertionDelimiterAlternative: string,
) {
nodeInfo.insertionDelimiter = conditionNodeInfo.range.isSingleLine
? insertionDelimiterConsequence
: insertionDelimiterAlternative;

return true;
}
}

export const queryPredicateOperators = [
new Log(),
new NotType(),
Expand All @@ -224,5 +255,6 @@ export const queryPredicateOperators = [
new ShrinkToMatch(),
new AllowMultiple(),
new InsertionDelimiter(),
new SingleOrMultilineDelimiter(),
new HasMultipleChildrenOfType(),
];
5 changes: 0 additions & 5 deletions packages/cursorless-engine/src/languages/getNodeMatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { patternMatchers as ruby } from "./ruby";
import rust from "./rust";
import scala from "./scala";
import { patternMatchers as scss } from "./scss";
import { patternMatchers as typescript } from "./typescript";

export function getNodeMatcher(
languageId: string,
Expand Down Expand Up @@ -59,8 +58,6 @@ export const languageMatchers: Record<
clojure,
go,
java,
javascript: typescript,
javascriptreact: typescript,
latex,
markdown,
php,
Expand All @@ -69,8 +66,6 @@ export const languageMatchers: Record<
scala,
scss,
rust,
typescript,
typescriptreact: typescript,
};

function matcherIncludeSiblings(matcher: NodeMatcher): NodeMatcher {
Expand Down
53 changes: 3 additions & 50 deletions packages/cursorless-engine/src/languages/markdown.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import { Range, SimpleScopeTypeType, TextEditor } from "@cursorless/common";
import { SimpleScopeTypeType, TextEditor } from "@cursorless/common";
import type { SyntaxNode } from "web-tree-sitter";
import {
NodeFinder,
NodeMatcherAlternative,
SelectionWithContext,
} from "../typings/Types";
import { getMatchesInRange } from "../util/getMatchesInRange";
import { NodeFinder, NodeMatcherAlternative } from "../typings/Types";
import { leadingSiblingNodeFinder, patternFinder } from "../util/nodeFinders";
import { createPatternMatchers, matcher } from "../util/nodeMatchers";
import {
extendUntilNextMatchingSiblingOrLast,
selectWithLeadingDelimiter,
} from "../util/nodeSelectors";
import { extendUntilNextMatchingSiblingOrLast } from "../util/nodeSelectors";
import { shrinkRangeToFitContent } from "../util/selectionUtils";

const HEADING_MARKER_TYPES = [
Expand Down Expand Up @@ -69,48 +61,9 @@ function sectionMatcher(...patterns: string[]) {
return matcher(leadingSiblingNodeFinder(finder), sectionExtractor);
}

const itemLeadingDelimiterExtractor = selectWithLeadingDelimiter(
"list_marker_parenthesis",
"list_marker_dot",
"list_marker_star",
"list_marker_minus",
"list_marker_plus",
);

function excludeTrailingNewline(editor: TextEditor, range: Range) {
const matches = getMatchesInRange(/\r?\n\s*$/g, editor, range);

if (matches.length > 0) {
return new Range(range.start, matches[0].start);
}

return range;
}

function itemExtractor(
editor: TextEditor,
node: SyntaxNode,
): SelectionWithContext {
const { selection } = itemLeadingDelimiterExtractor(editor, node);
const line = editor.document.lineAt(selection.start);
const leadingRange = new Range(line.range.start, selection.start);
const indent = editor.document.getText(leadingRange);

return {
context: {
containingListDelimiter: `\n${indent}`,
leadingDelimiterRange: leadingRange,
},
selection: excludeTrailingNewline(editor, selection).toSelection(
selection.isReversed,
),
};
}

const nodeMatchers: Partial<
Record<SimpleScopeTypeType, NodeMatcherAlternative>
> = {
collectionItem: matcher(patternFinder("list_item.paragraph!"), itemExtractor),
section: sectionMatcher("atx_heading"),
sectionLevelOne: sectionMatcher("atx_heading.atx_h1_marker"),
sectionLevelTwo: sectionMatcher("atx_heading.atx_h2_marker"),
Expand Down
11 changes: 0 additions & 11 deletions packages/cursorless-engine/src/languages/typescript.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,20 @@ export class EveryScopeStage implements ModifierStage {
if (scopes == null) {
// If target had no explicit range, or was contained by a single target
// instance, expand to iteration scope before overlapping
scopes = this.getDefaultIterationRange(
scopeHandler,
this.scopeHandlerFactory,
target,
).flatMap((iterationRange) =>
getScopesOverlappingRange(scopeHandler, editor, iterationRange),
);
try {
Copy link
Member Author

@AndreasArvidsson AndreasArvidsson Dec 2, 2023

Choose a reason for hiding this comment

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

getDefaultIterationRange throws an error so we never got to the legacy implementation below

Copy link
Member

Choose a reason for hiding this comment

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

Is it a NoContainingScopeError? If so I'd argue we should look for that error specifically

Copy link
Member Author

Choose a reason for hiding this comment

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

True. Fixed now

Copy link
Member

Choose a reason for hiding this comment

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

Just commenting from the future to say that we removed this :D #2740

scopes = this.getDefaultIterationRange(
scopeHandler,
this.scopeHandlerFactory,
target,
).flatMap((iterationRange) =>
getScopesOverlappingRange(scopeHandler, editor, iterationRange),
);
} catch (error) {
if (!(error instanceof NoContainingScopeError)) {
throw error;
}
scopes = [];
}
}

if (scopes.length === 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
} from "@cursorless/common";
import { LanguageDefinitions } from "../../../languages/LanguageDefinitions";
import { Target } from "../../../typings/target.types";
import { getInsertionDelimiter } from "../../../util/nodeSelectors";
import { getRangeLength } from "../../../util/rangeUtils";
import { ModifierStage } from "../../PipelineStages.types";
import { ScopeTypeTarget } from "../../targets";
Expand Down Expand Up @@ -109,25 +108,34 @@ export class ItemStage implements ModifierStage {
itemInfo: ItemInfo,
removalRange?: Range,
) {
const delimiter = getInsertionDelimiter(
target.editor,
const insertionDelimiter = getInsertionDelimiter(
itemInfo.leadingDelimiterRange,
itemInfo.trailingDelimiterRange,
", ",
);
return new ScopeTypeTarget({
scopeTypeType: this.modifier.scopeType.type as SimpleScopeTypeType,
editor: target.editor,
isReversed: target.isReversed,
contentRange: itemInfo.contentRange,
delimiter,
insertionDelimiter,
leadingDelimiterRange: itemInfo.leadingDelimiterRange,
trailingDelimiterRange: itemInfo.trailingDelimiterRange,
removalRange,
});
}
}

function getInsertionDelimiter(
leadingDelimiterRange?: Range,
trailingDelimiterRange?: Range,
): string {
return (leadingDelimiterRange != null &&
!leadingDelimiterRange.isSingleLine) ||
(trailingDelimiterRange != null && !trailingDelimiterRange.isSingleLine)
? ",\n"
: ", ";
}

/** Filter item infos by content range and domain intersection */
function filterItemInfos(target: Target, itemInfos: ItemInfo[]): ItemInfo[] {
return itemInfos.filter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
true,
);

const rawPrefixRange = getRelatedRange(
match,
scopeTypeType,
"prefix",
true,
);
const prefixRange =
rawPrefixRange != null
? new Range(rawPrefixRange.start, contentRange.start)
Copy link
Member

Choose a reason for hiding this comment

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

We should prob only do this if we decide to go ahead with #2108

Copy link
Member Author

Choose a reason for hiding this comment

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

I really don't like having to do unnecessary syntax, but sure we can await that discussion.

: undefined;

return {
editor,
domain,
Expand All @@ -80,11 +91,12 @@ export class TreeSitterScopeHandler extends BaseTreeSitterScopeHandler {
editor,
isReversed,
contentRange,
prefixRange,
removalRange,
leadingDelimiterRange,
trailingDelimiterRange,
interiorRange,
delimiter: insertionDelimiter,
insertionDelimiter,
}),
],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class LegacyContainingSyntaxScopeStage implements ModifierStage {
contentRange: contentSelection,
removalRange: removalRange,
interiorRange: interiorRange,
delimiter: containingListDelimiter,
insertionDelimiter: containingListDelimiter,
leadingDelimiterRange,
trailingDelimiterRange,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import {
EditNewActionType,
Target,
} from "../../typings/target.types";
import { union } from "../../util/rangeUtils";

export class DestinationImpl implements Destination {
public readonly contentRange: Range;
private readonly isLineDelimiter: boolean;
private readonly isBefore: boolean;
private readonly indentationString: string;
private readonly insertionPrefix?: string;

constructor(
public readonly target: Target,
Expand All @@ -24,12 +26,15 @@ export class DestinationImpl implements Destination {
) {
this.contentRange = getContentRange(target.contentRange, insertionMode);
this.isBefore = insertionMode === "before";
// It's only considered a line if the delimiter is only new line symbols
this.isLineDelimiter = /^(\n)+$/.test(target.insertionDelimiter);
this.isLineDelimiter = target.insertionDelimiter.includes("\n");
this.indentationString =
indentationString ?? this.isLineDelimiter
? getIndentationString(target.editor, target.contentRange)
: "";
this.insertionPrefix =
target.prefixRange != null
? target.editor.document.getText(target.prefixRange)
: undefined;
}

get contentSelection(): Selection {
Expand Down Expand Up @@ -65,9 +70,10 @@ export class DestinationImpl implements Destination {

getEditNewActionType(): EditNewActionType {
if (
this.insertionDelimiter === "\n" &&
this.insertionMode === "after" &&
this.target.contentRange.isSingleLine
this.target.contentRange.isSingleLine &&
this.insertionDelimiter === "\n" &&
this.insertionPrefix == null
) {
// If the target that we're wrapping is not a single line, then we
// want to compute indentation based on the entire target. Otherwise,
Expand Down Expand Up @@ -110,43 +116,46 @@ export class DestinationImpl implements Destination {

private getEditRange() {
const position = (() => {
const contentPosition = this.isBefore
? this.contentRange.start
: this.contentRange.end;
const insertionPosition = this.isBefore
? union(this.target.contentRange, this.target.prefixRange).start
: this.target.contentRange.end;

if (this.isLineDelimiter) {
const line = this.editor.document.lineAt(contentPosition);
const line = this.editor.document.lineAt(insertionPosition);
const nonWhitespaceCharacterIndex = this.isBefore
? line.firstNonWhitespaceCharacterIndex
: line.lastNonWhitespaceCharacterIndex;

// Use the full line to include indentation
if (contentPosition.character === nonWhitespaceCharacterIndex) {
// Use the full line with included indentation and trailing whitespaces
if (insertionPosition.character === nonWhitespaceCharacterIndex) {
return this.isBefore ? line.range.start : line.range.end;
}
}

return contentPosition;
return insertionPosition;
})();

return new Range(position, position);
}

private getEditText(text: string) {
const insertionText = this.indentationString + text;
const insertionText =
this.indentationString + (this.insertionPrefix ?? "") + text;

return this.isBefore
? insertionText + this.insertionDelimiter
: this.insertionDelimiter + insertionText;
}

private updateRange(range: Range, text: string) {
const baseStartOffset = this.editor.document.offsetAt(range.start);
const baseStartOffset =
this.editor.document.offsetAt(range.start) +
this.indentationString.length +
(this.insertionPrefix?.length ?? 0);

const startIndex = this.isBefore
? baseStartOffset + this.indentationString.length
: baseStartOffset +
this.getLengthOfInsertionDelimiter() +
this.indentationString.length;
? baseStartOffset
: baseStartOffset + this.getLengthOfInsertionDelimiter();

const endIndex = startIndex + text.length;

Expand All @@ -160,11 +169,10 @@ export class DestinationImpl implements Destination {
// Went inserting a new line with eol `CRLF` each `\n` will be converted to
// `\r\n` and therefore the length is doubled.
if (this.editor.document.eol === "CRLF") {
// This function is only called when inserting after a range. Therefore we
// only care about leading new lines in the insertion delimiter.
const match = this.insertionDelimiter.match(/^\n+/);
const nlCount = match?.[0].length ?? 0;
return this.insertionDelimiter.length + nlCount;
const matches = this.insertionDelimiter.match(/\n/g);
if (matches != null) {
return this.insertionDelimiter.length + matches.length;
}
}
return this.insertionDelimiter.length;
}
Expand Down
Loading