Skip to content

Add instant and prefix-based completion (#36) #37

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

Merged
merged 1 commit into from
Mar 25, 2023

Conversation

lephe
Copy link
Contributor

@lephe lephe commented Oct 9, 2022

This PR implements a rough version of the features proposed in the linked issue.

  • The prefix feature is supported by adding a symbol_by_unique_prefix() function in mathsymbols.py. The function only searches for 2 matching prefixes, and is used both in can_replace() and the replacement function.
  • The instant conversion feature is supported by using a text change listener. Apparently that listener is designed for replaying changes (and is quite recent), so I might be hacking a bit. But the view listener only gives on_modified() and that doesn't allow me to disable conversion during undos; the commented view-listener version would just run instantly after undoing a conversion and redo it again.

Not sure if this is fine to merge yet (I'm a bit worried about the text change listener), but at least it's out there. 😄

@lephe
Copy link
Contributor Author

lephe commented Oct 10, 2022

I realize the way I implemented it is a bit too greedy. When a name is complete (eg \subset) I substitute it automatically, without allowing longer names to be typed (eg. \subseteq). In this situation I should wait for another character.

Note for performance: the original feature I saw enters a special mode when typing a backslash; the characters that follow are underlined and the substitution is only active during that time. If you click away then it stops and the backslash will no longer be substituted without an explicit command. This might be a better approach for the greedy completer.

@lephe
Copy link
Contributor Author

lephe commented Nov 28, 2022

Coming back to this now, I finally managed to make it behave as I want it to. It's starting to be quite a big change though. Since I made replacement logic more complex, I factored out can_convert() and replacement() which appeared to do the same work twice, into a single function.

In the end I think the most natural setting is to have "convert_instantly": true to get all the logic I just implemented, "accept_prefixes": false because it can be confusing that \forall gives you ∀ll, and "convert_on_space": false for extra saved keystrokes.

Just to be clear, instant conversion kicks in when:

  1. You type \name and, there is a symbol name and none other that starts with name, in which case it converts \name;
  2. You type \nameX, there is a symbol name, there are other symbols that starts with name (otherwise it would've been converted earlier) but none that starts with \nameX, in which case it converts \name and keeps the X.

The whole point is to get as close as possible to some "LaTeX effect" where there is zero control keystroke. If I close my eyes and type \delta \subseteq \pi(f), I will get δ ⊆ π(f) (δ triggered by case 1, ⊆ by case 2 with a space as X, π by case 2 with a parenthesis as X).

The prefix idea is so that I can close my eyes, type \all (\a \b: Type) and get ∀ (α β: Type), however it doesn't end up working super well. This is because \a and \b are not unambiguous. In addition, typing \forall with both prefixes and instant completion yields ∀ll, which is counter-intuitive. I will handle that locally by having aliases like a → alpha and b → beta. Prefix conversion is probably worth having outside of instant mode though, so I left it in.

This isn't finished (still need to tackle the sorting thing), but any feedback on the current code is welcome.

@lephe
Copy link
Contributor Author

lephe commented Dec 3, 2022

For the performance problem, I wanted to use an ordered map so we could have a single data structure to replace the dictionary, but I couldn't easily find one in the standard library (?!). I elected to keep both the dictionary and the sorted version, which I wrapped along with the inverse in a utility class. I'm open to suggestions on how to handle this.

I don't have any more planned changes on this branch for now, so another review would be welcome. I noticed an exception on "Convert Back", but it seems to occur on the current master as well, so I guess that's orthogonal.

Copy link
Owner

@mvoidex mvoidex left a comment

Choose a reason for hiding this comment

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

Thanks for comments in code; I need a bit more time to review unicodecomplete.py

@lephe
Copy link
Contributor Author

lephe commented Dec 8, 2022

All valid points, thanks! I'll remember to squash everything when we're done.

return True

# In instant mode, accept symbols when followed by an invalid character.
# For instance, when typing "x" in "\alphax", recognize that "\alpha"
Copy link
Owner

Choose a reason for hiding this comment

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

Looks a bit hacky, but maybe it's ok.
If we type 'x' after '\alpha' - it will convert '\alpha'
Will it convert if we paste 'x' after '\alpha' with Ctrl+V? Looks like it will
But it won't - if we paste 'xyz' after '\alpha', because prefix will be '\alphaxy' (without last 'z')
It is expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the intended behavior, though we could discuss how satisfying it is. Ideally you'd want to behave as if all characters were typed in sequence, but testing that doesn't seem too straightforward and I'm afraid of (1) performance issues, as pasting tens of thousands of characters is quite common, and (2) unduly substituting stuff that should've been left verbatim when pasting chunks of code. I think it's fair to have a typing-oriented input mode rather than a text-oriented input mode for instant mode since it is interactive by design.

@lephe
Copy link
Contributor Author

lephe commented Dec 14, 2022

Thanks for the review. Let me know when I can squash the branch. Also, I'm thinking the feature would be rather obscure without some proper documentation. Would a section in the README file be fine with you?

@mvoidex
Copy link
Owner

mvoidex commented Dec 16, 2022

Let me know when I can squash the branch

Looks good to me to squash

Would a section in the README file be fine with you?

It'll be nice to have readme section about new feature

@lephe
Copy link
Contributor Author

lephe commented Feb 24, 2023

Oops, not sure how and when this flew off my radar. Sorry! I added the README section and squashed the branch (no other changes today). I've been using this mode for a while now, had to disable some of the conversions (such as \x → ×) that triggered too often, but really happy with it otherwise. Thanks for bearing with me :)

@mvoidex mvoidex merged commit 3ccdfd7 into mvoidex:master Mar 25, 2023
@mvoidex
Copy link
Owner

mvoidex commented Mar 25, 2023

Thanks, merged!

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.

2 participants