Skip to content

Commit

Permalink
Move node dependencies out of commons and engine (#2500)
Browse files Browse the repository at this point in the history
This is all about getting node out of commons. A lot of our file system
interaction had to do with running tests and reading/writing test
fixtures so I created a separate node test package.

To avoid circular dependencies I had to move a bunch of types from the
engine to commons. This make it so we now have changes in imports in a
bunch of different files. Most of the changes are quite mechanical or
boilerplate. The only actual new code is the two storage providers to
`ScopeTestRecorder` and `TestCaseRecorder`. The latter one is quite
lacking in implementation. This is just about unblocking cursorless
everywhere. A better abstraction is something we can workshop together.

And before you ask: no I can't split this into smaller pr. It feels like
a small miracle that I could solve all of the circular dependency
problems I had already :)

I wonder if we in a follow up want to move everything test related to
the test package? Generally I prefer to have test as close to the source
code as possible but in this case it's getting problematic to keeping
track of all the dependencies. If we had a test package we could make
sure that a lot of the dependencies only existed in that package and we
could add lint rules preventing them in the other packages.

## Checklist

- [/] I have added
[tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)
- [/] I have updated the
[docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and
[cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet)
- [/] I have not broken the cheatsheet

---------

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
  • Loading branch information
3 people authored Jul 13, 2024
1 parent b708cc4 commit b9590b4
Show file tree
Hide file tree
Showing 99 changed files with 449 additions and 367 deletions.
7 changes: 1 addition & 6 deletions packages/common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,16 @@
"license": "MIT",
"dependencies": {
"lodash-es": "^4.17.21",
"pathe": "^1.1.2",
"vscode-uri": "^3.0.8"
},
"devDependencies": {
"@types/chai": "^4.3.14",
"@types/js-yaml": "^4.0.9",
"@types/lodash-es": "4.17.0",
"@types/mocha": "^10.0.6",
"@types/sinon": "^17.0.3",
"chai": "^5.1.0",
"cross-spawn": "7.0.3",
"fast-check": "3.17.0",
"js-yaml": "^4.1.0",
"mocha": "^10.3.0",
"sinon": "^17.0.1"
"mocha": "^10.3.0"
},
"types": "./out/index.d.ts",
"exports": {
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/ide/normalized/NormalizedIDE.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { getFixturePath } from "../../index";
import { GeneralizedRange } from "../../types/GeneralizedRange";
import { TextEditor } from "../../types/TextEditor";
import FakeConfiguration from "../fake/FakeConfiguration";
Expand All @@ -17,6 +16,7 @@ export class NormalizedIDE extends PassthroughIDEBase {
original: IDE,
public fakeIde: FakeIDE,
private isSilent: boolean,
private cursorlessSnippetsDir: string,
) {
super(original);

Expand Down Expand Up @@ -46,7 +46,7 @@ export class NormalizedIDE extends PassthroughIDEBase {
hatStability: this.configuration.getOwnConfiguration(
"experimental.hatStability",
),
snippetsDir: getFixturePath("cursorless-snippets"),
snippetsDir: this.cursorlessSnippetsDir,
keyboardTargetFollowsSelection: false,
});
}
Expand Down
12 changes: 6 additions & 6 deletions packages/common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ export * from "./util";
export * from "./ide/util/messages";
export { getKey, splitKey } from "./util/splitKey";
export { hrtimeBigintToSeconds } from "./util/timeUtils";
export * from "./util/walkSync";
export * from "./util/walkAsync";
export * from "./util/disposableFrom";
export * from "./util/camelCaseToAllDown";
export { Notifier } from "./util/Notifier";
Expand Down Expand Up @@ -47,11 +45,16 @@ export * from "./types/TextEditorEdit";
export * from "./types/TextEditorOptions";
export * from "./types/TextLine";
export * from "./types/Token";
export * from "./types/SpokenFormType";
export * from "./types/HatTokenMap";
export * from "./types/ScopeProvider";
export * from "./types/SpokenForm";
export * from "./types/commandHistory";
export * from "./types/TalonSpokenForms";
export * from "./types/TestHelpers";
export * from "./types/TreeSitter";
export * from "./util/textFormatters";
export * from "./util/regex";
export * from "./util/serializedMarksToTokenHats";
export * from "./types/snippet.types";
export * from "./testUtil/fromPlainObject";
Expand All @@ -61,18 +64,15 @@ export * from "./types/GeneralizedRange";
export * from "./types/RangeOffsets";
export * from "./util/omitByDeep";
export * from "./util/range";
export * from "./util/object";
export * from "./util/uniqWithHash";
export * from "./testUtil/testConstants";
export * from "./testUtil/getFixturePaths";
export * from "./testUtil/getCursorlessRepoRoot";
export * from "./testUtil/runRecordedTest";
export * from "./testUtil/serialize";
export * from "./testUtil/shouldUpdateFixtures";
export * from "./testUtil/TestCaseSnapshot";
export * from "./testUtil/serializeTestFixture";
export * from "./testUtil/getSnapshotForComparison";
export * from "./testUtil/asyncSafety";
export * from "./testUtil/getScopeTestPathsRecursively";
export * from "./util/typeUtils";
export * from "./ide/types/hatStyles.types";
export * from "./errors";
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/testUtil/getSnapshotForComparison.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { storedTargetKeys } from "../StoredTargetKey";
import SpyIDE from "../ide/spy/SpyIDE";
import { ReadOnlyHatMap } from "../types/HatTokenMap";
import type { TestHelpers } from "../types/TestHelpers";
import { marksToPlainObject } from "../util/toPlainObject";
import { ExcludableSnapshotField, TestCaseSnapshot } from "./TestCaseSnapshot";
import { extractTargetedMarks } from "./extractTargetedMarks";
import { TestHelpers } from "./runRecordedTest";

/**
* Get the state of the editor to compare with the expected state of a test case
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { Notifier } from "@cursorless/common";
import {
SpokenFormMapKeyTypes,
SpokenFormType,
} from "../spokenForms/SpokenFormType";
import { SpokenFormMapKeyTypes, SpokenFormType } from "./SpokenFormType";

/**
* Interface representing a communication mechanism whereby Talon can provide
Expand Down
32 changes: 32 additions & 0 deletions packages/common/src/types/TestHelpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import type { FakeCommandServerApi } from "../FakeCommandServerApi";
import type { IDE } from "../ide/types/ide.types";
import type {
ExcludableSnapshotField,
ExtraSnapshotField,
TestCaseSnapshot,
} from "../testUtil/TestCaseSnapshot";
import type { SerializedMarks, TargetPlainObject } from "../util/toPlainObject";
import type { HatTokenMap } from "./HatTokenMap";
import type { TextEditor } from "./TextEditor";

export interface TestHelpers {
hatTokenMap: HatTokenMap;

// FIXME: Remove this once we have a better way to get this function
// accessible from our tests
takeSnapshot(
excludeFields: ExcludableSnapshotField[],
extraFields: ExtraSnapshotField[],
editor: TextEditor,
ide: IDE,
marks: SerializedMarks | undefined,
): Promise<TestCaseSnapshot>;

setStoredTarget(
editor: TextEditor,
key: string,
targets: TargetPlainObject[] | undefined,
): void;

commandServerApi: FakeCommandServerApi;
}
5 changes: 5 additions & 0 deletions packages/common/src/types/TextDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ export interface TextDocument {
*/
readonly uri: URI;

/**
* The last portion of the uri.path.
*/
readonly filename: string;

/**
* The identifier of the language associated with this document.
*/
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
4 changes: 2 additions & 2 deletions packages/cursorless-cheatsheet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
},
"dependencies": {
"@cursorless/common": "workspace:*",
"@cursorless/node-common": "workspace:*",
"immer": "^10.0.4",
"lodash-es": "^4.17.21",
"node-html-parser": "^6.1.12",
"pathe": "^1.1.2"
"node-html-parser": "^6.1.12"
},
"devDependencies": {
"@types/lodash-es": "4.17.0"
Expand Down
5 changes: 3 additions & 2 deletions packages/cursorless-cheatsheet/src/Cheatsheet.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { getCursorlessRepoRoot, type IDE } from "@cursorless/common";
import { type IDE } from "@cursorless/common";
import { getCursorlessRepoRoot } from "@cursorless/node-common";
import { readFile, writeFile } from "node:fs/promises";
import { produce } from "immer";
import { sortBy } from "lodash-es";
import { parse } from "node-html-parser";
import * as path from "pathe";
import * as path from "path";

/**
* The argument expected by the cheatsheet command.
Expand Down
3 changes: 3 additions & 0 deletions packages/cursorless-cheatsheet/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
"references": [
{
"path": "../common"
},
{
"path": "../node-common"
}
]
}
5 changes: 1 addition & 4 deletions packages/cursorless-engine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,18 @@
"license": "MIT",
"dependencies": {
"@cursorless/common": "workspace:*",
"glob": "^10.3.10",
"@cursorless/node-common": "workspace:*",
"immer": "^10.0.4",
"immutability-helper": "^3.1.1",
"itertools": "^2.2.5",
"lodash-es": "^4.17.21",
"moo": "0.5.2",
"nearley": "2.20.1",
"node-html-parser": "^6.1.12",
"pathe": "^1.1.2",
"sbd": "^1.0.19",
"uuid": "^9.0.1",
"zod": "3.22.4"
},
"devDependencies": {
"@types/glob": "^8.1.0",
"@types/js-yaml": "^4.0.9",
"@types/lodash-es": "4.17.0",
"@types/mocha": "^10.0.6",
Expand Down
2 changes: 1 addition & 1 deletion packages/cursorless-engine/src/actions/Actions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { TreeSitter } from "@cursorless/common";
import { Snippets } from "../core/Snippets";
import { RangeUpdater } from "../core/updateSelections/RangeUpdater";
import { ModifierStageFactory } from "../processTargets/ModifierStageFactory";
import { TreeSitter } from "../typings/TreeSitter";
import { BreakLine } from "./BreakLine";
import { Bring, Move, Swap } from "./BringMoveSwap";
import Call from "./Call";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { FlashStyle, Range } from "@cursorless/common";
import { FlashStyle, Range, matchAll } from "@cursorless/common";
import type { Snippets } from "../../core/Snippets";
import { Offsets } from "../../processTargets/modifiers/surroundingPair/types";
import { ide } from "../../singletons/ide.singleton";
import type { Target } from "../../typings/target.types";
import { matchAll } from "../../util/regex";
import { ensureSingleTarget, flashTargets } from "../../util/targetUtils";
import type { ActionReturnValue } from "../actions.types";
import Substituter from "./Substituter";
import { constructSnippetBody } from "./constructSnippetBody";
import { editText } from "./editText";
import Substituter from "./Substituter";

/**
* This action can be used to automatically create a snippet from a target. Any
Expand Down
5 changes: 2 additions & 3 deletions packages/cursorless-engine/src/actions/ShowParseTree.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { TreeSitter } from "@cursorless/common";
import { FlashStyle, Range, TextDocument } from "@cursorless/common";
import * as path from "pathe";
import type { Tree, TreeCursor } from "web-tree-sitter";
import type { TreeSitter } from "../typings/TreeSitter";
import { ide } from "../singletons/ide.singleton";
import type { Target } from "../typings/target.types";
import { flashTargets } from "../util/targetUtils";
Expand Down Expand Up @@ -43,7 +42,7 @@ function parseTree(
parseCursor(resultPlayground, resultQuery, contentRange, tree.walk(), 0);

return [
`## ${path.basename(document.uri.path)} [${contentRange}]\n`,
`## ${document.filename} [${contentRange}]\n`,
`\`\`\`${document.languageId}`,
document.getText(contentRange),
"```",
Expand Down
3 changes: 1 addition & 2 deletions packages/cursorless-engine/src/actions/incrementDecrement.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { Range, TextEditor } from "@cursorless/common";
import { MatchedText, Range, TextEditor, matchText } from "@cursorless/common";
import { PlainTarget } from "../processTargets/targets";
import { SelectionWithEditor } from "../typings/Types";
import { Destination, Target } from "../typings/target.types";
import { MatchedText, matchText } from "../util/regex";
import { runForEachEditor } from "../util/targetUtils";
import { Actions } from "./Actions";
import { ActionReturnValue } from "./actions.types";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { RangeOffsets, leftAnchored, rightAnchored } from "@cursorless/common";
import { invariant } from "immutability-helper";
import { leftAnchored, rightAnchored } from "../../util/regex";
import { ChangeEventInfo, FullRangeInfo } from "../../typings/updateSelections";
import { RangeOffsets } from "@cursorless/common";

/**
* Gets updated offsets for the range `rangeInfo` after the change described by
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { RangeOffsets, leftAnchored, rightAnchored } from "@cursorless/common";
import { invariant } from "immutability-helper";
import { leftAnchored, rightAnchored } from "../../util/regex";
import { ChangeEventInfo, FullRangeInfo } from "../../typings/updateSelections";
import { RangeOffsets } from "@cursorless/common";

/**
* Gets updated offsets for the range `rangeInfo` after the change described by
Expand Down
4 changes: 2 additions & 2 deletions packages/cursorless-engine/src/cursorlessEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
ScopeProvider,
ensureCommandShape,
type RawTreeSitterQueryProvider,
type TalonSpokenForms,
type TreeSitter,
} from "@cursorless/common";
import { KeyboardTargetUpdater } from "./KeyboardTargetUpdater";
import {
Expand Down Expand Up @@ -37,9 +39,7 @@ import { ScopeRangeProvider } from "./scopeProviders/ScopeRangeProvider";
import { ScopeRangeWatcher } from "./scopeProviders/ScopeRangeWatcher";
import { ScopeSupportChecker } from "./scopeProviders/ScopeSupportChecker";
import { ScopeSupportWatcher } from "./scopeProviders/ScopeSupportWatcher";
import { type TalonSpokenForms } from "./scopeProviders/TalonSpokenForms";
import { injectIde } from "./singletons/ide.singleton";
import { TreeSitter } from "./typings/TreeSitter";

interface Props {
ide: IDE;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import type {
SpokenFormEntry,
TalonSpokenForms,
} from "../scopeProviders/TalonSpokenForms";
import type { SpokenFormEntry, TalonSpokenForms } from "@cursorless/common";

export class DisabledTalonSpokenForms implements TalonSpokenForms {
getSpokenFormEntries(): Promise<SpokenFormEntry[]> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { TextDocument, Range } from "@cursorless/common";
import type { SyntaxNode, Tree, Language } from "web-tree-sitter";
import type { TreeSitter } from "../typings/TreeSitter";
import type { Range, TextDocument, TreeSitter } from "@cursorless/common";
import type { Language, SyntaxNode, Tree } from "web-tree-sitter";

export class DisabledTreeSitter implements TreeSitter {
getTree(_document: TextDocument): Tree {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { SpokenFormGenerator } from "./generateSpokenForm";
import { CustomSpokenFormGenerator } from "../api/CursorlessEngineApi";
import { CustomSpokenForms } from "../spokenForms/CustomSpokenForms";
import { TalonSpokenForms } from "../scopeProviders/TalonSpokenForms";
import { TalonSpokenForms } from "@cursorless/common";

/**
* Simple facade that combines the {@link CustomSpokenForms} and
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { SpokenFormMapEntry } from "../spokenForms/SpokenFormMap";
import {
SpokenFormMapKeyTypes,
SpokenFormType,
} from "../spokenForms/SpokenFormType";
import { SpokenFormMapKeyTypes, SpokenFormType } from "@cursorless/common";

/**
* A component of a spoken form used internally during spoken form generation.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { CompositeKeyMap } from "@cursorless/common";
import { SpeakableSurroundingPairName } from "../../spokenForms/SpokenFormType";
import { SpeakableSurroundingPairName } from "@cursorless/common";
import { SpokenFormComponentMap } from "../getSpokenFormComponentMap";
import { CustomizableSpokenFormComponentForType } from "../SpokenFormComponent";
import { surroundingPairsDelimiters } from "./surroundingPairsDelimiters";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SpeakableSurroundingPairName } from "../../spokenForms/SpokenFormType";
import { SpeakableSurroundingPairName } from "@cursorless/common";

export const surroundingPairsDelimiters: Record<
SpeakableSurroundingPairName,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import {
TestCaseFixtureLegacy,
getRecordedTestPaths,
serializeTestFixture,
shouldUpdateFixtures,
} from "@cursorless/common";
import { getRecordedTestPaths } from "@cursorless/node-common";
import * as yaml from "js-yaml";
import assert from "node:assert";
import { promises as fsp } from "node:fs";
import { canonicalizeAndValidateCommand } from "../core/commandVersionUpgrades/canonicalizeAndValidateCommand";
import { getHatMapCommand } from "./getHatMapCommand";
import { SpokenFormGenerator } from ".";
import { defaultSpokenFormInfoMap } from "../spokenForms/defaultSpokenFormMap";
import { canonicalizeAndValidateCommand } from "../core/commandVersionUpgrades/canonicalizeAndValidateCommand";
import { SpokenFormMap, mapSpokenForms } from "../spokenForms/SpokenFormMap";
import { defaultSpokenFormInfoMap } from "../spokenForms/defaultSpokenFormMap";
import { getHatMapCommand } from "./getHatMapCommand";

/**
* A spoken form map to use for testing. Just uses default spoken forms, but
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
PartialSpokenFormTypes,
SpokenFormMapKeyTypes,
SpokenFormType,
} from "../spokenForms/SpokenFormType";
} from "@cursorless/common";
import { CustomizableSpokenFormComponentForType } from "./SpokenFormComponent";

/**
Expand Down
Loading

0 comments on commit b9590b4

Please sign in to comment.