Skip to content

around destination? #1799

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
josharian opened this issue Aug 16, 2023 · 14 comments
Open

around destination? #1799

josharian opened this issue Aug 16, 2023 · 14 comments

Comments

@josharian
Copy link
Collaborator

Very crude idea here, just jotting it down.

Problem

Wrapper snippets live in the top level Talon namespace. They compete with all other commands. This makes them hard to name, particularly as they become numerous.

Proposal

An "around" destination. Then instead of "if wrap air" it'd be "snip if around air", just as it is "snip range before/to/after air".

This could also be used as an alternate spoken form for paired delimiter wrapping. Instead of "curly wrap air" it could be "wrap curly around air".

This might also open the path to multiple positional holes in snippets, if the target had multiple ranges selected. But this gets complicated quickly.

@pokey
Copy link
Member

pokey commented Aug 17, 2023

Interesting idea. How to handle wrapping into different holes of a snippet? Today we use eg "if wrap air" vs "else wrap air"

@josharian
Copy link
Collaborator Author

The obvious analogue is "snip else around air". But maybe I'm missing something more subtle going on there.

@pokey
Copy link
Member

pokey commented Aug 17, 2023

I guess that could work, it's just that "snip " feels like it is the name of the snippet itself, which is the same snippet no matter which hole you're wrapping into. But I wouldn't say this is a showstopper; I still think this grammar is an interesting idea. Prob worth tossing on the to-discuss list

@josharian josharian added the to discuss Plan to discuss at meet-up label Aug 17, 2023
@josharian
Copy link
Collaborator Author

Even deeper into "ponies" land...

bring currently serves to provide a kind of ad hoc insertion snippet. If there was some way to textually represent a variable/hole, then it could also serve to provide ad hoc wrapper snippets.

I needed to wrap a dozen select statements today in coalesce( ... , "unknown").

I thought I was clever and created a custom wrapper snippet, but it was a lot of work to get it all hooked up correctly and iron out the bugs, and in the end it was clearly a waste of time. Some text in a split editor like coalesce(WHAT_GOES_HERE, "unknown") would have enabled a quick bring file cap around air past plex as needed.

@pokey
Copy link
Member

pokey commented Aug 18, 2023

Yeah ad hoc snippets were one of the eventual ideas behind building the code that we use for "snippet make" today

I also touch briefly on ad hoc snippets in #46

I think nowadays I'd be tempted to do it prefixed by a command like "with hole " to indicate the hole, eg "with hole air bring call air around bat"

@pokey
Copy link
Member

pokey commented Aug 24, 2023

Discussed briefly with @AndreasArvidsson and while this grammar seems reasonable, we're not clear what problem this new grammar would be solving. You mention the following:

Wrapper snippets live in the top level Talon namespace. They compete with all other commands. This makes them hard to name, particularly as they become numerous.

Not sure I follow the problem here. How do "<name> wrap <target>" vs "snip <name> around <target>" differ in this regard?

This might also open the path to multiple positional holes in snippets

Not sure I follow. If you want to inject multiple targets into holes of the snippet, then where does the snippet itself end up? Unless the targets are directly adjacent, it's not clear where to put the snippet, unlike wrapping a single target, where the snippet replaces that target

@pokey pokey removed the to discuss Plan to discuss at meet-up label Aug 24, 2023
@josharian
Copy link
Collaborator Author

josharian commented Aug 24, 2023

How do "<name> wrap <target>" vs "snip <name> around <target>" differ in this regard?

I thought about this more carefully. The fully ambiguous cases are a bit of a stretch. For example, if word wrap toggles word wrap (as it does in at least one place in community), and you had an insertion snippet called word (why?), then word wrap air could be:

  • "toggle word wrap" then "insert a"
  • "use insertion snippet word with target a"

I'm not entirely convinced that there aren't other less concocted examples involving homophones / near homophones instead of straight up ambiguity.

That said, I think what's really going on is that I mentally model talent commands as a giant decision tree, and I want everything snippet related to be under snip, rather than scattered around as a bunch of early roots. Which is not necessarily a good reason. :)

Unless the targets are directly adjacent, it's not clear where to put the snippet

Good point.

@josharian
Copy link
Collaborator Author

Another use case: the call action.

I find it hard to use because it requires selecting the target first ("take air call bat"). It could instead be "call bat around air".

@AndreasArvidsson
Copy link
Member

AndreasArvidsson commented Sep 15, 2023

Call with two targets is actually already supported in our api. You just need to add a spoken form for it. :)

@josharian
Copy link
Collaborator Author

Should we do that for everyone? (Do you have a spoken form for it already?)

@AndreasArvidsson
Copy link
Member

I personally don't use it, but we could do that.

@pokey
Copy link
Member

pokey commented Sep 16, 2023

We removed it a while ago to improve talon performance. Those performance issues should no longer be a problem, so I'd be happy to have it re-added.

@AndreasArvidsson
Copy link
Member

I will start on it a asap!

@AndreasArvidsson
Copy link
Member

pr: #1900

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

3 participants