emacs-citar / citar

Emacs package to quickly find and act on bibliographic references, and edit org, markdown, and latex academic documents.
GNU General Public License v3.0
500 stars 54 forks source link

Issues with `citar-export-local-bib-file` #834

Open kslutsky opened 3 months ago

kslutsky commented 3 months ago

First, let me thank @bdarcus and the rest of the contributors for all the hard thought and work that went into citar.

Describe the bug I have encountered three issues while using the citar-export-local-bib-file function.

  1. The most significant one occurs within the call to citar--insert-bibtex. If the latter is called with a citekey which is not found in any of the bibtex-files, then it returns the first bibtex entry within the file instead of nil. The reason, I believe, is that while (bibtex-search-entry citekey) does return nil, subsequent calls to bibtex-beginning-of-entry and bibtex-end-of-entry are unconditional and point to the first bibtex entry within the file. This can be fixed by making these calls conditional on the returned value of bibtex-search-entry:

    (if (bibtex-search-entry citekey)
    (let ((beg (bibtex-beginning-of-entry))
          (end (bibtex-end-of-entry)))
      (buffer-substring-no-properties beg end))
    "")
  2. The doc string for citar-bibliography does say explicitly that it is expected to be a list of files, but the implementation of citar--bibliography-files allows (rather conveniently) for citar-bibliography to be a string provided there is only one bibliography file to be specified. The current implementation of citar-export-local-bib-file, however, does assume that citar-bibliography is a genuine list, since it calls (car citar-bibliography) to determine ext of the bibliography file. One way to fix this would to be define ext via

    (ext (file-name-extension (car (citar--bibliography-files 'global))))
  3. The last thing is that the current implementation of citar-export-local-bib-file searches for keys only among the global bibliography files and ignores the local ones. This, I believe, is because (citar--insert-bibtex citekey) is called within with-temp-file and therefore looses access to the local bibliography of the buffer from which citar-export-local-bib-file is called. I am not sure if this behavior is intentional or not. If it is, I think it would be appropriate to mention it in the doc string of citar-export-local-bib-file. But if local bibliography entries should be taken into account, then here is one way to accomplish this. We can let citar--insert-bibtex take an optional list of bibtex files to find the citekey in:

    (defun citar--insert-bibtex (citekey &optional bibtex-files-list)

    and the variable bibtex-files would then be bound as follows

    (bibtex-files (or bibtex-files-list (citar--bibliography-files)))

    Finally, the call to citar--insert-bibtex within citar-export-local-bib-file would supply the list of bibtex files.

With all the changes above, citar--insert-bibtex and citar-export-local-bib-file would look something like this:

(defun citar--insert-bibtex (citekey &optional bibtex-files-list)
  "Insert the bibtex entry for CITEKEY at point."
  (let* ((bibtex-files
          (or bibtex-files-list (citar--bibliography-files)))
         (entry
          (with-temp-buffer
            (bibtex-set-dialect)
            (dolist (bib-file bibtex-files)
              (insert-file-contents bib-file))
            (if (bibtex-search-entry citekey)
                (let ((beg (bibtex-beginning-of-entry))
                      (end (bibtex-end-of-entry)))
                  (buffer-substring-no-properties beg end))
              ""))))
    (unless (equal entry "")
      (insert entry "\n\n"))))

;;;###autoload
(defun citar-export-local-bib-file ()
  "Create a new bibliography file from citations in current buffer.

The file is titled \"local-bib\", given the same extension as
the first entry in `citar-bibliography', and created in the same
directory as current buffer."
  (interactive)
  (let* ((citekeys (citar--major-mode-function 'list-keys #'ignore))
         (ext (file-name-extension (car (citar--bibliography-files 'global))))
         (file (format "%slocal-bib.%s" (file-name-directory buffer-file-name) ext))
         (bibtex-files (citar--bibliography-files)))
    (with-temp-file file
      (dolist (citekey citekeys)
        (citar--insert-bibtex citekey bibtex-files)))))
bdarcus commented 3 months ago

Thanks @kslutsky!

@localauthor - I think you added this functionality, and I don't use it myself. Any input?

localauthor commented 3 months ago

I may have added this, but I’m almost as sure that I’ve never used it, which would explain why/how it got out of sync with later changes. In short, I’ve got nothing ;)

bdarcus commented 3 months ago

Thanks @localauthor.

@kslutsky - while I could implement this, I can't say when. Since you've been working on this, care to submit a PR? If yes, see the contributing doc.

kslutsky commented 3 months ago

Since you've been working on this, care to submit a PR?

Alas, not in the near future. My apologies. But I might be able to help in a few weeks, if the issue remains open.

Let me, however, ask a related implementation question, if I may. Functions that call citar--insert-bibtex are typically interested in retrieving the raw BibTeX entry with a given key. They do so from scratch by reading the bibliography files into a temporary buffer and delegating the search to bibtex-search-entry. Meanwhile, nearly all the necessary information is already available in the citar--entries hash table. I say nearly because citar--entries calls parsebib-parse with the default :display t field, which replaces parts of the TeX markup.

So, my question is: Is there a way to get a raw BibTeX entry out of the citar--entries hash table with all the TeX markup intact?

Thank you.

bdarcus commented 3 months ago

Is there a way to get a raw BibTeX entry out of the citar--entries hash table with all the TeX markup intact?

Not currently. We use parsebib-parse, which returns the data stripped of that markup and optimized for display.

It simplifies things on our end, and gets us input for CSL JSON as well.

But it has some limitations.

kslutsky commented 3 months ago

I ran into one more issue, which I thought I would like to report here. It affects multiple functions in the citar-citation-map, and has to do with the way they interact with citar-citation targets and citar-embark package.

Let me explain the problem by taking citar-copy-reference function as a specific example. This function accepts a list of citekeys, whose formatted references are to be inserted in the current buffer. When it receives its argument from citar--select-multiple or via an embark action on a citar-key target, all works well. But if the argument is from a citar-citation target with multiple keys, then citar-copy-reference fails to find the relevant bibliography entries.

I believe, the reason is that a citar-citation target produces a string obtained by concatenating individual keys with the " & " separator as done by the citar--stringify-keys function. Thus citar-copy-reference needs to unstringify the keys first, but its current implementation does not do this.

To give a concrete example, if citar-copy-reference is applied to the citar-citation target that corresponds to the LaTeX citation \cite{key1,key2}, then its argument becomes ("key1 & key2") instead of ("key1" "key2").

The problem can be fixed by unstringifying the argument first, e.g., by pre-processing it as follows:

(setq citekeys (mapcan #'citar--unstringify-keys citekeys))

The same issue, in fact, applies to functions citar-open, citar-open-links, citar-open-files, and citar-open-notes, each of which fails, in my case at least, to act on a citar-citation target with multiple keys.

bdarcus commented 3 months ago

I ran into one more issue, which I thought I would like to report here.

Any objection in me splitting this off into a separate issue?

kslutsky commented 3 months ago

Any objection in me splitting this off into a separate issue?

No objections at all.