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

Completion Tweaks #757

Merged
merged 3 commits into from
Feb 12, 2025
Merged

Completion Tweaks #757

merged 3 commits into from
Feb 12, 2025

Conversation

andymandias
Copy link
Collaborator

@andymandias andymandias commented Feb 8, 2025

This PR has three ~related changes to completion (which can be split further if needed):

  • Fixes a bug where text completion will not replace the previous completion properly when the specified suffix contains a space (i.e. during typical usage). This bug was introduced as part of a previous minor bug fix, where whitespace in a message would be compressed (e.g. multiple spaces replaced with a single space) when using text/emoji completion.
  • When cycling through text completions, the last name would require two tab presses to loop back to the first completion from the last completion. This fixes that behavior by cycling back to the original prompt before looping back to the first completion (with one tab between each).
  • Select the first command/emoji automatically, when using the command/emoji picker. This was suggested by @englut, and I like how it works; I think this is more in line with how other messengers work, and will probably be more intuitive to users. The only drawback I can see is that it will prevent sending "uncompleted emoji" without adding a space afterward. Maybe we could enable Ctrl-Enter and Command-Enter to bypass the command/emoji picker and send the message? And/or, we could allow tab to cycle to "no command/emoji" selected as part of its loop.

@englut
Copy link
Contributor

englut commented Feb 8, 2025

This works great! Thank you for addressing the feedback so quickly <3

The only drawback I can see is that it will prevent sending "uncompleted emoji" without adding a space afterward.

I think that actually makes sense behaviourally. If you intended to type :smile (and not an emoji) and adding a space after would be a good way to end the picker. But.. I guess if you don't want a space that might make sense. maybe letting the user close out the picker if they hit esc?

@andymandias andymandias marked this pull request as draft February 8, 2025 19:34
  - Fix bug where text completion may not replace the previous completion properly.
  - Select the first command/emoji automatically, when using the command/emoji picker.
  - When cycling text completion, cycle back to the original prompt before looping through the list of completions.
@casperstorm
Copy link
Member

This make sense to me!

@andymandias andymandias marked this pull request as ready for review February 10, 2025 04:11
@andymandias
Copy link
Collaborator Author

Having thought it over I think being able to tab into a no-command/emoji-picked state is probably not good UX. At some point it might be nice to also allow ctrl/cmd-enter to submit a send, but for now I think being able to dismiss the picker with escape is enough.

@andymandias andymandias merged commit 771f29c into main Feb 12, 2025
1 check passed
@andymandias andymandias deleted the completion-tweaks branch February 12, 2025 05:26
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.

3 participants