emacsorphanage / helm-swoop

Efficiently hopping squeezed lines powered by Emacs helm interface
GNU General Public License v3.0
689 stars 55 forks source link

Fix breaking change in helm-display-function #124

Closed jguenther closed 6 years ago

jguenther commented 6 years ago

I believe this fixes #123

After manually declaring the helm-swoop-pattern variable as buffer-local, I was getting an error from helm-display-buffer():

Wrong number of arguments: (1 . 1), 2

Looks like the underlying problem is due to 97020d189f in helm. The helm-display-buffer function now calls the helm-display-function with an additional optional arg (RESUME).

This PR changes helm-swoop-split-window-function into a defcustom, and split out the old default lambda value into the function helm-swoop--split-window-default.

~I'm unsure if using defvar-local is the right thing to do for helm-swoop-pattern et al, but it seems to work for me.~

edit: this fix actually works without defvar-local, I'll remove that commit.

stsquad commented 6 years ago

OK once I updated helm-core this patch works for me in fixing #123. Does it need to detect which version of helm is installed to stay compatible with melpa-stable?

jguenther commented 6 years ago

@stsquad the new arg is &optional, should be OK on older helm versions.

jguenther commented 6 years ago

@ShingoFukuyama anything I can do to help get this merged?

xuchengpeng commented 6 years ago

@ShingoFukuyama Oh, lord, just merge this PR

ShingoFukuyama commented 6 years ago

Thank you @jguenther and everyone here for tackling the issue! And I'm sorry for this very late notice... I'll close this pull request due to #125