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

Don't reset hippie-expand search string #1168

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bcc32
Copy link

@bcc32 bcc32 commented Jul 6, 2023

hippie-expand relies on the values of this-command and last-command in
order to determine what argument it should pass to try-expand
functions, in order to signal whether it's the "first" time they are
being called.

However, undo (even when called interactively) sets itself as
this-command, which breaks hippie-expand. This has the following
concrete effect: suppose there are two dabbrevs for "ab" in the
buffer, "abc" and "abcd" (occurring in that order, before point).
Also, suppose there is a yasnippet whose key is "ab", and yasnippet
comes before dabbrev in the hippie-expand-try-functions-list.

If a user presses "a b" and then M-/ (hippie-expand), repeatedly, what
happens on each M-/ is:

  1. The yasnippet with key "ab" is expanded
  2. The yasnippet is undone, and dabbrev is tried (and succeeds).
    Dabbrev inserts "abcd". However, yasnippet-hippie-try-expand also
    caused this-command to be set to undo.
  3. hippie-expand sees that this-command (hippie-expand) !=
    last-command (undo), and so the search string is set to "abcd"
    instead of "ab". There are no further occurrences of "abcd" in the
    buffer, and if the user was trying to expand "abc" (or any other
    string beginning with "ab" but not "abcd"), this fails.

This PR fixes the above bug by resetting this-command to the value
it had before undo was called. Since we do not care about chaining
consecutive undos in this context, this doesn't break undo, and it
fixes the above issue with hippie-expand.

bcc32 added 2 commits July 6, 2023 12:00
This variable is nil the first time the function is called, and t on
subsequent times, so first-time? is a misleading name.  Use `old` like
the other hippie-expand-try-functions do.
hippie-expand relies on the values of this-command and last-command in
order to determine what argument it should pass to try-expand
functions, in order to signal whether it's the "first" time they are
being called.

However, undo (even when called interactively) sets itself as
this-command, which breaks hippie-expand.  This has the following
concrete effect: suppose there are two dabbrevs for "ab" in the
buffer, "abc" and "abcd" (occurring in that order, before point).
Also, suppose there is a yasnippet whose key is "ab", and yasnippet
comes before dabbrev in the hippie-expand-try-functions-list.

If a user presses "a b" and then M-/ (hippie-expand), repeatedly, what
happens on each M-/ is:

1. The yasnippet with key "ab" is expanded
2. The yasnippet is undone, and dabbrev is tried (and succeeds).
   Dabbrev inserts "abcd".  However, yasnippet-hippie-try-expand also
   caused this-command to be set to undo.
3. hippie-expand sees that this-command (hippie-expand) !=
   last-command (undo), and so the search string is set to "abcd"
   instead of "ab".  There are no further occurrences of "abcd" in the
   buffer, and if the user was trying to expand "abc" (or any other
   string beginning with "ab" but not "abcd"), this fails.

This commit fixes the above bug by resetting this-command to the value
it had before undo was called.  Since we do not care about chaining
consecutive undos in this context, this doesn't break undo, and it
fixes the above issue with hippie-expand.
@bcc32 bcc32 force-pushed the dont-reset-hippie-expand-search-string branch from 8812db6 to d600590 Compare July 6, 2023 16:45
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.

1 participant