Skip to content

Revisit action.getFinalStages #1619

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

Open
Tracked by #1616
pokey opened this issue Jul 12, 2023 · 0 comments
Open
Tracked by #1616

Revisit action.getFinalStages #1619

pokey opened this issue Jul 12, 2023 · 0 comments
Labels
code quality Improvements to code quality

Comments

@pokey
Copy link
Member

pokey commented Jul 12, 2023

  • Note that it's exclusively used for untyped expansion. The only partial exception to that rule is that insertSnippet expands if untyped and not implicit (eg to distinguish "snip funk" vs "snip funk to this"). We could probably just have all of them require not implicit
  • My first thought was to just remove it altogether and let the action just run the action itself, but it is necessary to have this construct for two reasons:
    • For insertSnippet, we want to run the stage before converting to a destination, which has already happened by the time the action sees it
    • Generally, we want the stage to run on primitive targets, so that eg "funk wrap past air" / "funk wrap air past bat" perform expansion on the primitive targets rather than the final range. If we wait till the action gets the target, it's too late for that
  • I wonder if we should really just have actions expose a default scope type, and have it default to token. That would then also subsume the step where we run ContainingTokenIfUntypedEmptyStage. The only problem there is that that only runs on empty range, whereas the other ones still want to run for nonempty. Eg if you say "funk wrap this" and you have a nonempty selection, we still expand to containing statement, whereas if you say "take this" and you have a nonempty selection, we don't expand to token. I'd be tempted to unify these behaviours in exchange for a much simplified command pipeline and behaviour that's more consistent / easier to explain. I'd lean towards the token behaviour and only expand if empty.
  • I don't like how in Destinations as first-class objects #1605, we have made it easier to forget to pass around the final target stages
@pokey pokey changed the title Consider removing action.getFinalStages altogether Revisit action.getFinalStages Jul 12, 2023
@pokey pokey added the code quality Improvements to code quality label Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Improvements to code quality
Projects
None yet
Development

No branches or pull requests

1 participant