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

Delimiter pairs #1612

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Delimiter pairs #1612

wants to merge 15 commits into from

Conversation

AndreasArvidsson
Copy link
Collaborator

@AndreasArvidsson AndreasArvidsson commented Nov 23, 2024

  1. Adds a Talon list defining delimiter pairs
  2. Adds a voice command to insert delimiter pairs
  3. Adds a voice command to wrap selection with delimiter pairs
  4. Extends compound edit command to wrap with delimiter pairs

These are the spoken forms I personal use (except for padding). Do we want to use something else?

What do we do with all the existing symbol commands?
https://github.com/talonhub/community/blob/bb240ba1cf958bd3e2c7791e5a57d09d205adc81/plugin/symbols/symbols.talon#L10-L45

Fixes #546

Copy link
Collaborator

@nriley nriley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the additional ways to insert delimiters into a separate file, remove those that have spoken forms equivalent to the once being introduced and deprecate all the rest.

core/edit/delimiter_pair.talon-list Show resolved Hide resolved
core/edit/edit_command_actions.py Show resolved Hide resolved
core/edit/edit_command_actions.py Show resolved Hide resolved
core/edit/delimiter_pair.talon-list Outdated Show resolved Hide resolved
core/edit/delimiter_pair.talon-list Outdated Show resolved Hide resolved
@AndreasArvidsson
Copy link
Collaborator Author

Changes performed. Also migrated deprecated symbol commands to a separate file and called the deprecated command action.

core/edit/insert_between.py Outdated Show resolved Hide resolved
<user.delimiter_pair>: user.delimiter_pair_insert(delimiter_pair)

# Wrap selection with delimiter pairs
<user.delimiter_pair> that: user.delimiter_pair_wrap_selection(delimiter_pair)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one subtlety with the new wrap command versus the deprecated commands:
the cursor winds up within the delimiters when wrapping; previously, the cursor would wind up outside. I'm not entirely sure this change is undesirable. If the intent is to replace the deprecated commands, should we preserve the previous functionality?

Copy link
Member

@knausj85 knausj85 Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose there are three scenarios we could cover with the grammar.

  1. cursor between the delimiters. Done.
  2. Wrapping selection with delimiters. Done, but need to settle the cursor position.
  3. Patch in the delimiters, cursor after. I believe this was the original purpose of the "empty " commands, but they no longer behave as such. Should we bother with this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I prefer the cursor ending up inside but we can change it if people prefer. Should we get a few additional voices on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's see what @nriley and @phillco think :)

core/edit/edit.talon Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

Systematize handling of delimiters
4 participants