emacs-helm / helm

Emacs incremental completion and selection narrowing framework
https://emacs-helm.github.io/helm/
GNU General Public License v3.0
3.36k stars 389 forks source link

Wrong type argument: arrayp, nil (when calling sql-connect with (helm-mode 1) turned on) #1553

Closed ahungry closed 8 years ago

ahungry commented 8 years ago

To reproduce:

Set global helm-mode via:

(helm-mode 1)

Attempt to connect to an SQL server via sql-connect command (You'll probably receive an unrelated sql-connect/comint error about setting a nil, and this sql-connect command never works until the second time it is run during an emacs session, but thats another thing entirely...)

You'll then receive an error: Wrong type argument: arrayp, nil

This has to do with the default candidate list being sent in by sql-connect, which is '(nil), which according to the documentation for completing-read, is a valid list of default values. Changing the sql-connect function to pass in a nil here instead of '(nil) fixes the issue, but this is a newly introduced bug on the helm side (it was not present a month or so ago).

ahungry commented 8 years ago

This minimal elisp can produce it outside of sql-connect, I think it may have to do with the history push portion of completing-read (it only occurs with helm active though, so maybe helm is not returning the proper value for a completing-read to the history part?)

(let ((history '())) (dotimes (x 2) (completing-read "Test: " '(foo bar) nil t nil 'history '(nil))))

The dotimes is required, as this error only occurs when the history list already has a value in it (if history is '() it works ok, if history is '(foo) or '(bar) it does not).

thierryvolpiatto commented 8 years ago

Matthew Carter notifications@github.com writes:

This minimal elisp can produce it outside of sql-connect, I think it may have to do with the history push portion of completing-read (it only occurs with helm active though, so maybe helm is not returning the proper value for a completing-read to the history part?)

(let ((history '())) (dotimes (x 2) (completing-read "Test: " '(foo bar) nil t nil 'history '(nil))))

The dotimes is required, as this error only occurs when the history list already has a value in it (if history is '() it works ok, if history is '(nil) or '(whatever) it does not.

Thanks for the recipe, should be fixed now.

Thierry

ahungry commented 8 years ago

Awesome, thanks!

ahungry commented 8 years ago

Works great with the fix, so closing this out