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.

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

Support destination pseudo-scopes #1631

Open
Tracked by #1616
pokey opened this issue Jul 13, 2023 · 3 comments
Open
Tracked by #1616

Support destination pseudo-scopes #1631

pokey opened this issue Jul 13, 2023 · 3 comments

Comments

@pokey
Copy link
Member

pokey commented Jul 13, 2023

The problem

Consider the following code

const aaa = 0;

In this case, there is no type. However, we want the following to work:

  • "change type" from within the statement should insert a : after the aaa and leave your cursor after the :
  • "bring bat to type air" should insert a : after the aaa, followed by "bat"

The solution

Query language update

  • In our query language, support @myScopeType.destination

  • Could be used as follows:

    (
      (lexical_declaration
         (variable_declarator
            name: (_) @type.destination.endOf
            !type
         )
      )
      (#leading-insertion-delimiter! @type.destination ": ")
    )

How to handle these queries

The naive approach is to create a stub target that only has a getDestination method and throws on everything else. Unfortunately the problem is that this would break the following:

function aaa(): void {
    const bbb = 0;
}

If you say "copy type bat", you'll get an error, because it will get trapped in the pseudo-scope. Maybe that's fine because it could be confusing to have "copy type bat" and "change type bat" refer to different scopes (the latter is legitimately a destination pseudo-scope that allows adding a type to bbb)

@pokey pokey added the to discuss Plan to discuss at meet-up label Jul 13, 2023
@pokey
Copy link
Member Author

pokey commented Aug 13, 2023

update from meet-up, taking "type" as our running example:

  • "drink", "pour", and "change" all do the same thing when you have no type
  • "drink" and "pour" don't work when you have a type (unless you're in a language which suppers multiple types)
  • If you say "take type" from within something that has no type but could (ie has a destination pseudo-scope), it is an error. This is useful when combined with Filter / forgiving adverbs #1015
  • for something that can have more than one (eg "name" in scm), then "drink" and "pour" work
  • Add (#prefix-insertion-delimiter! ": ") predicate
  • Add (#postfix-insertion-delimiter! ": ") predicate
  • Rename (#insertion-delimiter!) to #infix-insertion-delimiter?
  • if no infix, then we don't support "drink" and "pour" when there is already 1

@pokey pokey removed the to discuss Plan to discuss at meet-up label Aug 13, 2023
@pokey
Copy link
Member Author

pokey commented Aug 13, 2023

Note that the interaction with #1015 is a bit nontrivial because for eg "pour soft type" or "bring air to soft type", we actually want "soft" to apply to the conversion to a destination rather than to the modifier after it. Not sure how to distinguish that from wanting it to apply to the modifier after it 🤔

@pokey
Copy link
Member Author

pokey commented Aug 14, 2023

There's also an interesting interaction with the pattern we're proposing in #1742. For the case of "name" in the .scm language, which we're using to refer to @foo captures, then we get the captures as multiple targets when you're not directly in one of them. For example:

(aaa) @bbb @ccc

In this case, if you say "name air", you'll get bbb and ccc. The problem is that if you now have something like the following:

(aaa) @bbb @ccc
(ddd) @eee

If you say "bring name drum to air", you'll get

(aaa) @eee @eee
(ddd) @eee

Which is obviously not desirable. In this case, it seems like the destination "to name air" should possibly be the range bbb @ccc, rather than as separate targets. Or we could just have that be the target "name air" as well.

Maybe the answer here is that for #1742 we should have them be one target that is the union of their ranges if they're adjacent? They can still use "every name" if they want them separately

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant