-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
Strip whitespace in latexsub :exit-function #206
Conversation
According to the docstring for completion-extra-properties, the "STRING" passed to the :exit-function should be the bare text to which the field was completed, but helm-mode adds an extra space at the end of "STRING". Since none of our latexsubs include whitespace, we can safely strip whitespace in the :exit-function and work around this helm bug. I will file an upstring bug report against helm, but I think it's worth fixing quickly here since it's only a simple 3-line change. Closes #204.
For anyone curious on history, this is the same bug as emacs-helm/helm#2360. I'm not at all sure how I had helm working before given that Thierry only fixed that issue for insertion but not for what |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
string-clean-whitespace does more than is needed and is provided by subr-x instead of just subr, so was causing tests to fail on older emacs versions.
Thanks @tpapp and @FelipeLema! Merged as 2dfc869. |
Sorry, I had a chance to test this only now! It mostly works in the sense that |
Looks like helm-mode isn't calling the |
According to the docstring for
completion-extra-properties
, thestring
passed to the:exit-function
should be the bare text to which the field was completed, but helm-mode adds an extra space at the end ofstring
. Since none of our latexsubs include whitespace, we can safely strip whitespace in the:exit-function
and work around this helm bug.I will file an upstring bug report against helm, but I think it's worth fixing quickly here since it's only a simple 3-line change.
Closes #204.
@tpapp, @giordano, or @FelipeLema could one of you review please? This fixes an extremely annoying bug for anyone using helm. @giordano, could you please double-check that this fixes the issue for you if you get a chance?