Skip to content

Implement extension side of destinations #1630

Closed
@pokey

Description

@pokey

Extension side

  • Add getDestination(insertionMode: InsertionMode) to Target, and have default implementation in BaseTarget
  • 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 use Destination
  • 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 be Destination and stop implementing Target for this type, removing all unnecessary attributes
  • Remove edit from Target
  • 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

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions