Error: Wrong type argument: stringp when creating *sly-completion* buffer

[BooAA]

Hi, thanks for the work.

When using sly-symbol-completion-mode, there is error "(wrong-type-argument stringp #<buffer sly-completions>)" when creating the completion buffer. Screenshot:

2022-09-15.23-47-25.mov

Version:
Sly: Master, 805c29672c8a1f6c68286ab379359f9ab9ad9dc2
Emacs: Master, 4cf9c92e27d20da9453f9abe89d84bee5d776329

I think the error is from sly--completion-transient-mode-display-guard-p, backtrace listed below:

Debugger entered--Lisp error: (wrong-type-argument stringp #<buffer *sly-completions*>)
  sly--completion-transient-mode-display-guard-p(#<buffer *sly-completions*> ((display-buffer--maybe-same-window display-buffer-reuse-window display-buffer--maybe-pop-up-frame-or-window display-buffer-below-selected) (window-height . t) nil))
  #f(compiled-function (conditions) #<bytecode -0x620ed9aa9162099>)((sly--completion-transient-mode-display-guard-p))
  buffer-match-p(sly--completion-transient-mode-display-guard-p #<buffer *sly-completions*> ((display-buffer--maybe-same-window display-buffer-reuse-window display-buffer--maybe-pop-up-frame-or-window display-buffer-below-selected) (window-height . t) nil))
  display-buffer-assq-regexp(#<buffer *sly-completions*> ((sly--completion-transient-mode-display-guard-p (sly--completion-transient-mode-teardown-before-displaying))) ((display-buffer--maybe-same-window display-buffer-reuse-window display-buffer--maybe-pop-up-frame-or-window display-buffer-below-selected) (window-height . t) nil))
  display-buffer(#<buffer *sly-completions*> ((display-buffer--maybe-same-window display-buffer-reuse-window display-buffer--maybe-pop-up-frame-or-window display-buffer-below-selected) (window-height . t) nil))
  temp-buffer-window-show(#<buffer *sly-completions*> ((display-buffer--maybe-same-window display-buffer-reuse-window display-buffer--maybe-pop-up-frame-or-window display-buffer-below-selected) (window-height . t) nil))
  sly--completion-pop-up-completions-buffer("def" (#("decf" 0 2 ... 2 3 ... 3 4 ...) #("defun" 0 3 ... 3 5 ...) #("deref" 0 2 ... 2 4 ... 4 5 ...) #("defvar" 0 3 ... 3 6 ...) #("defsetf" 0 3 ... 3 7 ...) #("deftype" 0 3 ... 3 7 ...) #("defclass" 0 3 ... 3 8 ...) #("defmacro" 0 3 ... 3 8 ...) #("uiop:defun*" 0 5 ... 5 8 ... 8 11 ...) #("sb-c:xdefun" 0 6 ... 6 9 ... 9 11 ...) #("sb-c:id-leaf" 0 6 ... 6 7 ... 7 9 ... 9 10 ... 10 11 ... 11 12 ...) #("defmethod" 0 3 ... 3 9 ...) #("delete-if" 0 2 ... 2 8 ... 8 9 ...) #("defstruct" 0 3 ... 3 9 ...) #("defglobal" 0 3 ... 3 9 ...) #("asdf:perform" 0 2 ... 2 3 ... 3 6 ... 6 7 ... 7 8 ... 8 9 ... ...) #("delete-file" 0 2 ... 2 7 ... 7 8 ... 8 11 ...) #("sb-c:defknown" 0 5 ... 5 8 ... 8 13 ...) #("defgeneric" 0 3 ... 3 10 ...) #("defpackage" 0 3 ... 3 10 ...) ...))
  sly--completion-in-region-function(#<marker at 286 in *sly-mrepl for sbcl<5>*> 289 #f(compiled-function (string pred action) #<bytecode 0x6377807fa8846dc>) nil)
  #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_15>(#f(compiled-function (&rest args) #<bytecode -0x1d7af936d74db536>) #<marker at 286 in *sly-mrepl for sbcl<5>*> 289 #f(compiled-function (string pred action) #<bytecode 0x6377807fa8846dc>) nil)
  apply(#<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_15> #f(compiled-function (&rest args) #<bytecode -0x1d7af936d74db536>) (#<marker at 286 in *sly-mrepl for sbcl<5>*> 289 #f(compiled-function (string pred action) #<bytecode 0x6377807fa8846dc>) nil))
  #f(advice #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_15> :around #f(compiled-function (&rest args) #<bytecode -0x1d7af936d74db536>) ((name . sly--setup-completion)))(#<marker at 286 in *sly-mrepl for sbcl<5>*> 289 #f(compiled-function (string pred action) #<bytecode 0x6377807fa8846dc>) nil)
  completion-in-region(#<marker at 286 in *sly-mrepl for sbcl<5>*> 289 #f(compiled-function (string pred action) #<bytecode 0x6377807fa8846dc>) nil)
  completion-at-point()
  sly-mrepl-indent-and-complete-symbol(nil)
  funcall-interactively(sly-mrepl-indent-and-complete-symbol nil)
  command-execute(sly-mrepl-indent-and-complete-symbol)

Thanks.

[joaotavora]

Can't reproduce on an Emacs master build from about 1,5 months ago. Can you confirm this is only in a very recent Emacs 29?

[BooAA]

Can you confirm this is only in a very recent Emacs 29?

Sorry for the late reply, as there are many broken builds during July and Aug in the Emacs master branch. I have tested builds in 2022-09-02 (commit: 0ec831b91cde2a0e1b65f99c1190975c6e6959f9) and 2022-08-15 (commit: d5e74d9cd7fd657f13ef7ac72cf84c3bc3a03ba9). Both of them exist the problem I mentioned above.

I will update more information if I can find some earlier builds that works.

[joaotavora]

Can't reproduce, just tried with the latest commit as of today:

commit 8892abbaf92ebd2e8f968fe86270fba08bf1d07a (HEAD -> master, origin/master, origin/HEAD)
Author: Stefan Kangas <stefankangas@gmail.com>
Date:   Fri Sep 23 11:50:28 2022 +0200

My test:

cd ~/Source/Emacs/sly
~/Source/Emacs/emacs/src/emacs -Q -L . -l sly
C-u M-x sly <path-to-my-sbcl>
;; type (def
C-M-i to invoke completion

Everything works as expected

[joaotavora]

[BooAA]

I think I found the issue. How if you config (setq tab-always-indent 'complete) to use tabs for both indentation and completion? I found using C-M-i can trigger completion normally, but using tabs will result in the issue mentioned above.

2022-09-24.00-38-17.mov

[joaotavora]

Well it triggers an issue, but not the issue mentioned above. Furthermore, I can't even get a backtrace.

I think the best way is to git bisect this. It works fine in Emacs 28.2. A good bisection test is

emacs -Q -L . -l sly --eval '(setq tab-always-indent (quote complete))' -f sly

followed by a quick manual test (like you have in the gif)

Unfortunately, I don't have time to do this bisection right now. Maybe you do?

[BooAA]

Sorry for the late reply.

Unfortunately, I don't have time to do this bisection right now. Maybe you do?

I'm very willing to do that but might not recently, as I'm busy onboarding my first job this 2 weeks. I will do the test when I'm free.

[1player]

Same issue here on Emacs master, and I too have (setq tab-always-indent 'complete). I'll try a bisect when I find a moment to do so.

[joaotavora]

This is foremost a regression in Emacs master. It would be good to figure out first when it started failing before thinking about fixes to Sly.

[joaotavora]

What does "following Postel" mean? Anyway, i don't think it's starting a fight to observe that a piece of Elisp worked in a version of Emacs and doesn't work in a later version. That's usually the definition of a "regression", unless that piece of Elisp was using some API "illegally". But I've seen no evidence of that.

…

On Wed, Nov 9, 2022, 15:26 Jon Irving ***@***.***> wrote: I mean, sure, but there's no harm in following Postel with something like this. I had a quick look at the git blame for subr.el and there's a *lot* of new work there, I didn't see a relevant commit from earlier than April this year in buffer-match-p. Since buffer-match-p explicitly accepts buffer-or-name, converting it to a buffer, and the doc string specifies: CONDITION is either: [ ... ] - a predicate function that takes a buffer object and ARG as arguments, and returns non-nil if the buffer matches, I think "regression" might be a little strong. Perhaps "clarification" or "tightening up of design" in this case. But I am not looking for a fight :) Thanks for a great package, and good luck tracking down the regression and talking someone into fixing it. — Reply to this email directly, view it on GitHub <#534 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6PQ4IWC32NTF7GGTQCWDWHO7DDANCNFSM6AAAAAAQNRDFWA> . You are receiving this because you commented.Message ID: ***@***.***>

[joaotavora]

I don't think it was intentional. Such things are mentioned in a special section of the NEWS file. I haven't checked, but I doubt it's there.

…

On Wed, Nov 9, 2022, 17:56 Jon Irving ***@***.***> wrote: I meant Jon Postel, as in the "robustness principle": Be conservative in what you do, be liberal in what you accept from others. I was not being disingenuous when I said I didn't want to start a fight. I meant me, I wasn't accusing you :) I get into trouble because I disagree with people and use terse language. Since it is a major version of emacs (i.e., 29 vs 28) I would say it's a breaking change rather than a regression. I guess it hinges on whether it was intentional. 🤷 — Reply to this email directly, view it on GitHub <#534 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6PQ4H2U4EMIFVF4MJCSTWHPQWPANCNFSM6AAAAAAQNRDFWA> . You are receiving this because you commented.Message ID: ***@***.***>

[joaotavora]

Finally found time to do the bisection:

❯ git bisect bad
59ecf25fc86081c9df05b84194c36414c225c265 is the first bad commit
commit 59ecf25fc86081c9df05b84194c36414c225c265
Author: Philip Kaludercic <philipk@posteo.net>
Date:   Thu Mar 10 10:59:52 2022 +0100

    * window.el (display-buffer-assq-regexp): Use buffer-match

 lisp/window.el | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

The bisection test I used, for reference:

make -j
src/emacs -Q -nw -L ~/Source/Emacs/sly -l sly --eval '(setq tab-always-indent (quote complete))' -f sly
;; type (def, following by TAB

[joaotavora]

Reported to Emacs https://debbugs.gnu.org/cgi/bugreport.cgi?bug=62417. Fix proposed there.

[sebasmonia]

Just yesterday I ran into this in a Sly w/Emacs compiled from master (I am setting up a fresh dev environment).
In the linked post, Eli suggests "The users can easily make local changes". Does he mean applying your patch, or is there some other (smaller) way to get this working?

[joaotavora]

No, the best way is just wait for Eli to finally understand what is going on and solve this in a perfectly equivalent way. Give them time :-)

[joaotavora]

"The users can easily make local changes" is completely misguided. Most SLY users aren't even awayre SLY is using display-buffer-alist temporarily. This is already fixed in Emacs 29. Eli is just going to tweak it, and that's fine.

[sebasmonia]

sounds good, thanks for replying! the timing of seeing this reported and being patched just as yesterday I ran into it.

Best workaround I guess is using C-M-i to invoke completion, rather than C-i/TAB.