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.

Sign up for GitHub

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

Jump to bottom

Assert creation of scope handlers #2684

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Assert creation of scope handlers #2684

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed7591e
Assert creation of scope handlers
AndreasArvidsson Nov 19, 2024
5a8d208
Rename
AndreasArvidsson Nov 20, 2024
39710bd
Add bind
AndreasArvidsson Nov 20, 2024
4b8ced9
Renamed to maybe
AndreasArvidsson Dec 1, 2024
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
2 changes: 1 addition & 1 deletion packages/cursorless-engine/src/processTargets/modifiers/ContainingScopeStage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class ContainingScopeStage implements ModifierStage {
run(target: Target): Target[] {
const { scopeType, ancestorIndex = 0 } = this.modifier;

const scopeHandler = this.scopeHandlerFactory.create(
const scopeHandler = this.scopeHandlerFactory.tryCreate(
scopeType,
target.editor.document.languageId,
);
Expand Down
4 changes: 2 additions & 2 deletions packages/cursorless-engine/src/processTargets/modifiers/EveryScopeStage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class EveryScopeStage implements ModifierStage {
const { scopeType } = this.modifier;
const { editor, isReversed } = target;

const scopeHandler = this.scopeHandlerFactory.create(
const scopeHandler = this.scopeHandlerFactory.tryCreate(
scopeType,
editor.document.languageId,
);
Expand Down Expand Up @@ -108,7 +108,7 @@ export class EveryScopeStage implements ModifierStage {
scopeHandlerFactory: ScopeHandlerFactory,
target: Target,
): Range[] {
const iterationScopeHandler = scopeHandlerFactory.create(
const iterationScopeHandler = scopeHandlerFactory.tryCreate(
scopeHandler.iterationScopeType,
target.editor.document.languageId,
);
Expand Down
4 changes: 0 additions & 4 deletions packages/cursorless-engine/src/processTargets/modifiers/PreferredScopeStage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ export class PreferredScopeStage implements ModifierStage {
target.editor.document.languageId,
);

if (scopeHandler == null) {
throw Error(`Couldn't create scope handler for: ${scopeType.type}`);
}

const closestTargets = getClosestScopeTargets(target, scopeHandler);

if (closestTargets == null) {
Expand Down
2 changes: 1 addition & 1 deletion packages/cursorless-engine/src/processTargets/modifiers/RelativeScopeStage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class RelativeScopeStage implements ModifierStage {
) {}

run(target: Target): Target[] {
const scopeHandler = this.scopeHandlerFactory.create(
const scopeHandler = this.scopeHandlerFactory.tryCreate(
this.modifier.scopeType,
target.editor.document.languageId,
);
Expand Down
4 changes: 2 additions & 2 deletions packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/BoundedScopeHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ abstract class BoundedBaseScopeHandler extends BaseScopeHandler {
this.targetScopeHandler = this.scopeHandlerFactory.create(
this.targetScopeType,
this.languageId,
)!;
);
this.surroundingPairInteriorScopeHandler = this.scopeHandlerFactory.create(
{
type: "surroundingPairInterior",
delimiter: "any",
},
this.languageId,
)!;
);
}

get iterationScopeType(): ScopeType {
Expand Down
2 changes: 1 addition & 1 deletion packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NestedScopeHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export abstract class NestedScopeHandler extends BaseScopeHandler {
this._searchScopeHandler = this.scopeHandlerFactory.create(
this.searchScopeType,
this.languageId,
)!;
);
}

return this._searchScopeHandler;
Expand Down
19 changes: 6 additions & 13 deletions packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/OneOfScopeHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,18 @@ export class OneOfScopeHandler extends BaseScopeHandler {
languageId: string,
): ScopeHandler {
const scopeHandlers: ScopeHandler[] = scopeType.scopeTypes.map(
(scopeType) => {
const handler = scopeHandlerFactory.create(scopeType, languageId);
if (handler == null) {
throw new Error(`No available scope handler for '${scopeType.type}'`);
}
return handler;
},
(scopeType) => scopeHandlerFactory.create(scopeType, languageId),
);

const iterationScopeType = (): CustomScopeType => ({
type: "custom",
scopeHandler: new OneOfScopeHandler(
undefined,
scopeHandlers.map(
(scopeHandler) =>
scopeHandlerFactory.create(
scopeHandler.iterationScopeType,
languageId,
)!,
scopeHandlers.map((scopeHandler) =>
scopeHandlerFactory.create(
scopeHandler.iterationScopeType,
languageId,
),
),
() => {
throw new Error("Not implemented");
Expand Down
7 changes: 6 additions & 1 deletion packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@ import type { ScopeType } from "@cursorless/common";
import type { CustomScopeType, ScopeHandler } from "./scopeHandler.types";

export interface ScopeHandlerFactory {
create(
tryCreate(
scopeType: ScopeType | CustomScopeType,
languageId: string,
): ScopeHandler | undefined;

create(
scopeType: ScopeType | CustomScopeType,
languageId: string,
): ScopeHandler;
}
14 changes: 13 additions & 1 deletion ...s/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ import type { CustomScopeType, ScopeHandler } from "./scopeHandler.types";
*/
export class ScopeHandlerFactoryImpl implements ScopeHandlerFactory {
constructor(private languageDefinitions: LanguageDefinitions) {
this.tryCreate = this.tryCreate.bind(this);
this.create = this.create.bind(this);
}

create(
tryCreate(
Copy link
Member

Choose a reason for hiding this comment

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

I believe we usually use maybe for things like this, eg maybeCreate

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

scopeType: ScopeType | CustomScopeType,
languageId: string,
): ScopeHandler | undefined {
Expand Down Expand Up @@ -114,4 +115,15 @@ export class ScopeHandlerFactoryImpl implements ScopeHandlerFactory {
?.getScopeHandler(scopeType);
}
}

create(
Copy link
Member

Choose a reason for hiding this comment

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

I would be tempted to call this createStrict or createOrThrow. Once we've migrated all scopes, this will never throw, right? So maybe we just leave it 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this shouldn't really throw. It's just there because I don't like doing ! assertion in typescript if we can get around it.

scopeType: ScopeType | CustomScopeType,
languageId: string,
): ScopeHandler {
const handler = this.tryCreate(scopeType, languageId);
if (handler == null) {
throw new Error(`Couldn't create scope handler for '${scopeType.type}'`);
}
return handler;
}
}
2 changes: 1 addition & 1 deletion ...odifiers/scopeHandlers/SurroundingPairScopeHandler/SurroundingPairInteriorScopeHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class SurroundingPairInteriorScopeHandler extends BaseScopeHandler {
requireStrongContainment: true,
},
this.languageId,
)!;
);
}

get iterationScopeType() {
Expand Down
9 changes: 6 additions & 3 deletions packages/cursorless-engine/src/scopeProviders/ScopeRangeProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class ScopeRangeProvider {
editor: TextEditor,
{ scopeType, visibleOnly }: ScopeRangeConfig,
): ScopeRanges[] {
const scopeHandler = this.scopeHandlerFactory.create(
const scopeHandler = this.scopeHandlerFactory.tryCreate(
scopeType,
editor.document.languageId,
);
Expand All @@ -50,13 +50,16 @@ export class ScopeRangeProvider {
{ scopeType, visibleOnly, includeNestedTargets }: IterationScopeRangeConfig,
): IterationScopeRanges[] {
const { languageId } = editor.document;
const scopeHandler = this.scopeHandlerFactory.create(scopeType, languageId);
const scopeHandler = this.scopeHandlerFactory.tryCreate(
scopeType,
languageId,
);

if (scopeHandler == null) {
return [];
}

const iterationScopeHandler = this.scopeHandlerFactory.create(
const iterationScopeHandler = this.scopeHandlerFactory.tryCreate(
scopeHandler.iterationScopeType,
languageId,
);
Expand Down
12 changes: 9 additions & 3 deletions packages/cursorless-engine/src/scopeProviders/ScopeSupportChecker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ export class ScopeSupportChecker {
*/
getScopeSupport(editor: TextEditor, scopeType: ScopeType): ScopeSupport {
const { languageId } = editor.document;
const scopeHandler = this.scopeHandlerFactory.create(scopeType, languageId);
const scopeHandler = this.scopeHandlerFactory.tryCreate(
scopeType,
languageId,
);

if (scopeHandler == null) {
return getLegacyScopeSupport(languageId, scopeType);
Expand All @@ -53,13 +56,16 @@ export class ScopeSupportChecker {
scopeType: ScopeType,
): ScopeSupport {
const { languageId } = editor.document;
const scopeHandler = this.scopeHandlerFactory.create(scopeType, languageId);
const scopeHandler = this.scopeHandlerFactory.tryCreate(
scopeType,
languageId,
);

if (scopeHandler == null) {
return getLegacyScopeSupport(languageId, scopeType);
}

const iterationScopeHandler = this.scopeHandlerFactory.create(
const iterationScopeHandler = this.scopeHandlerFactory.tryCreate(
scopeHandler.iterationScopeType,
languageId,
);
Expand Down
Loading