Closed
Description
Extension side
- Add
getDestination(insertionMode: InsertionMode)
toTarget
, and have default implementation inBaseTarget
- Handle destination list by processing targets for each destination, then wrapping each output in a destination with proper insertion mode
- Change type signature of
run
for actions that expect a destination to useDestination
- In the case of
"swap"
, it expects two actual targets (not destinations), but internally converts them to destinations when using them as such. - Change
PositionTarget
to beDestination
and stop implementingTarget
for this type, removing all unnecessary attributes - Remove
edit
fromTarget
- Might be worth tackling Switch "change" implementation to use
editNew
#773 while here
On the api surface, we'd modify ActionCommand
something like the following:
interface PartialSimpleActionDescriptor {
name: SimpleActionName;
target: PartialTargetDescriptor;
}
interface PartialBringMoveActionDescriptor {
name: "bring" | "move";
source: PartialTargetDescriptor;
destination: PartialDestinationDescriptor;
}
interface PartialSwapActionDescriptor {
name: "swap";
target1: PartialTargetDescriptor;
target2: PartialTargetDescriptor;
}
interface PartialWrapWithPairedDelimiterActionDescriptor {
name: "wrapWithPairedDelimiter";
left: string;
right: string;
target: PartialTargetDescriptor;
}
type PartialActionDescriptor =
| PartialSimpleActionDescriptor
| PartialBringMoveActionDescriptor
| PartialSwapActionDescriptor
| PartialWrapWithPairedDelimiterActionDescriptor;
We probably don't need to create corresponding full action descriptor types, because we can probably just use local variables in the process of handling the action. Eg
function runAction(partialActionDescriptor: PartialActionDescriptor) {
switch (partialActionDescriptor.name) {
case "bring":
case "move":
const [sourceDescriptor, destinationTargetDescriptor] = inferFullTargets([
partialActionDescriptor.source,
partialActionDescriptor.destination.target,
]);
// Note: we don't need to worry about final stages here because bring and move don't use them!
const source = pipelineRunner.run(sourceDescriptor);
const destination = pipelineRunner
.run(destinationTargetDescriptor)
.getDestination(partialActionDescriptor.destination.insertionMode);
return action.run(source, destination);
case "wrapWithPairedDelimiter":
// Some duplication with default case, but not tooo bad
const [targetDescriptor] = inferFullTargets([
partialActionDescriptor.target,
]);
const target = pipelineRunner.run(targetDescriptor);
return action.run(
target,
partialActionDescriptor.left,
partialActionDescriptor.right
);
default:
// TODO: Could prob expose `inferFullTarget` (singular) that takes single descriptor with no previousTargets
// eg const targetDescriptor = inferFullTarget(partialActionDescriptor.target)
const [targetDescriptor] = inferFullTargets([
partialActionDescriptor.target,
]);
// TODO: Do we want to handle final stages in the generic case? Most
// actions don't have them anyway so we might want special cases for them
const target = pipelineRunner.run(targetDescriptor);
return action.run(target);
}
}
Fwiw if we don't need post-inference action descriptor types, then maybe we should remove Partial
from these names, eg just BringMoveActionDescriptor
Note that it's also tempting to just inline the whole ActionDescriptor
into the Command
object itself. Not sure what the nested object is buying us here
Metadata
Metadata
Assignees
Labels
No labels