bibtex-actions-read returns (nil nil) when selecting item

[emacs-citar]

bibtex-actions-read returns (nil nil) when selecting item

[publicimageltd]

Deleting the URL doesn't change anything.

Adding the dummy parameter "fixes" it, simply because I then use the eval'd version instead of the byte compiled one.

Byte compiling yields no warning or whatever.

Maybe it's something with cl-defun? It looks like an Emacs bug to me. The hard way to check would be to rewrite the package without cl-defun and test it. I have no time for this the next days, however, and I am not suggesting you should do it.

But you should be able to reproduce the error:

1. open emacs
2. set the .bib file above as bibliography file for bibtex-completion
3. bibtex-actions-refresh
4. select "jörissen"
5. nil nil
6. go to the source
7. re-eval bibtex-actions-read
8. select "jörissen" again
9. it works
10. reload the compiled version (load-library "bibtex-actions")
11. and back to 2.

I will try to reproduce the error with a clean version (-q) and report the results. Might take a while, though.

[publicimageltd]

Declaring the variable fixes the issue. And it makes sense, since it is an issue with byte compilation, and that's the only thing changed by that declaration. I made a PR.

[minad]

@bdarcus Indeed, there is no warning in this case. The problem is that you used when-let here, which is not a good idea. when-let automatically uses the lexically bound variables since it expands to let+when. I suggest you only use when-let in case it is necessary to check the value.

[bdarcus]

So my guess is that the problem has to do with compilation.

But no errors upon such compilation; right?

In the past, I sometimes have run into errors like that which comes down to the way completing-read-multiple parses candidates (it gets confused by trailing whitespace, and punctuation can throw it off, which is why the ampersand is the delimiter).

[publicimageltd]

But no errors upon such compilation; right?

No, the other way around: Error only with the compiled version, no error if I evaluate the defun directly.

[bdarcus]

Sorry; I meant ... there are no compile errors, or even warnings?

[bdarcus]

... but adding the dummy parameter fixes it anyway? EDIT: and "certain bibliography items" trigger it? What happens if you remove the url from the first one you posted? I don't think it will make any difference, but ...

[bdarcus]

It looks like an Emacs bug to me.

Sure sounds like it.

I will try to reproduce the error with a clean version (-q) and report the results. Might take a while, though.

Thank you!

[publicimageltd]

It's the comma!

I was able to reproduce the error with a clean setup. I finally found the error: It's the "," in the title field. It makes completion-read-multiple think that I have entered multiple items, whereas in reality, I have not. Stupid little Emacs. I have no idea why the error goes away once I eval the defun directly (this behavior too was reproducible).

Here's what I did for debugging:

;; start with emacs -q -l 'filenameofthatsnippet.el'
(require 'package)
(package-initialize)
(require 'use-package)
(require 'quelpa-use-package)
(setq visible-bell nil
      ring-bell-function 'ignore)

(use-package bibtex-completion
  :config
  (setq bibtex-completion-bibliography   '("~/bibfile_causing_nil.bib"))
  (setq bibtex-completion-notes-path "~/Dokumente/Hefte/zettelkasten"))

(use-package bibtex-actions
  :bind
  (("C-x l" . bibtex-actions-open-notes)))

And I added some debug vars to bibtex-actions-read:

  (when-let ((crm-separator "\\s-*&\\s-*")
	      (candidates (setq bibtex-actions--debug-candidates (bibtex-actions--get-candidates rebuild-cache)))
              (chosen
               (setq bibtex-actions--debug-chosen
                     (completing-read-multiple ......

Using this setup, I could provoke the error and look at the variables. If I choose the first title with the comma, bibtex-actions--debug-chosen is a list of two strings. This causes the cl-loop clause to produce (nil nil). If I choose the second title, it is a list with only one string -- the expected behavior.

[bdarcus]

Yeah, why I mentioned the CRM issue above and asked about the URL field, which has an &.

When you realize the CRM selections are one string, you can see how it can be brittle in certain circumstances.

But I don't see why the comma is a problem, given the crm-separator is the ampersand?

Indeed, @oantolin suggested the ampersand precisely to avoid that problem with these data (because you have to escape it in tex).

Is there something subtle wrong with that regexp, or with how I'm setting the local variable? E.g. is & not in fact being used as the crm-separator??

[publicimageltd]

The default value of crm-separator is "[ ]*,[ ]*". So it looks like the when-let doesn't override it. But I just caught the value of crm-separator in a global debug variable, like with the other let-bound values above, and it returned the modfied string with the ampersand. So it is overriden in this scope.

I can't explain it, but I have the feeling that somewhere in the depths of completions-read-multiple, something relies on dynamic scoping, or for some other reasons accesses the default instead of the overriden value. Which is why it works when I eval it manually in the file.

[publicimageltd]

Well, at least the source file for completing-read-multiple has no lexical scoping declaration in its top.

[bdarcus]

@oantolin @minad - do you have any insight into what @publicimageltd is hypothesizing just above here? Is this a crm bug? Something wrong with this code??

[publicimageltd]

Debugging in real time: If I set the variable crm-separator explicitly to the ampersand regexp usingdefvar, it works. So the problem is indeed that completing-read-multiple accesses the dynamically bound value for crm-separator, whereas bibtex-actions-read uses lexical-binding: t.

Problem being, of course, that using defvar, the global default is changed. But there must be a way to set a variably dynamically within a file with lexical binding enabled... or? And I vaguely remember having read that in Emacs 28, all files have be changed to use lexical binding.

[minad]

@bdarcus @publicimageltd Yes this can be. Are you requiring 'crm? Then you should get the defvar which makes the variable dynamically bound. Otherwise you can write a defvar declaration yourself, (defvar crm-separator). My recommendation is to use package-lint in combination with flycheck or flymake to catch many of such issues early on. In this case it would probably warn that crm-separator is lexically bound but unused.

[bdarcus]

I originally did that, but didn't understand why.

But when it went through the review at MELPA, they recommended removing it.

I'll add it back then.

So am I adding that specific crm-separator as a defvar at the top, and removing the let-bound one on the function, or am I only setting the defvar to nil (this is what I originally did).

[bdarcus]

Any chance you want to submit a PR @publicimageltd?

[bdarcus]

On this, @minad @publicimageltd:

My recommendation is to use package-lint in combination with flycheck or flymake to catch many of such issues early on. In this case it would probably warn that crm-separator is lexically bound but unused.

1. I use doom emacs for elisp coding, which has all this setup, and does flag these sorts of warnings and errors for me. No such warnings or errors ATM.
2. I have the elisp-check github action setup, which does the same checks, and flags unused lexical variables on PRs. I have a policy, for myself as well, of never merging a PR with even a warning.
3. I haven't seen this bug, and @publicimageltd only sees is when he uses the byte-compiled package.

These three facts don't add up for me. Do they for you two?

[bdarcus]

@publicimageltd - #23 is the original PR I merged that I thought fixed the problem, and later removed.

[publicimageltd]

3. I haven't seen this bug, and @publicimageltd only sees is when he uses the byte-compiled package.

Which Emacs version do you use? If it's a greater (how do you say? bigger?) version than 27.2, have a look at the source file for crm, and if it has a lexical binding declaration on the first line. As I said, I have read somewhere on emacs devel that all files have been converted to lexical binding lately for some more recent version of Emacs.

[bdarcus]

Which Emacs version do you use?

I've mostly been using 28, but reverted to 27.2 recently because of an apparent bug that was bothering me.