Skip to content

file indicator incorrectly indicating  #776

Description

@stefangroha

Describe the bug
I use Spacemacs and Mendeley and I am trying to directly access the pdfs of files through citar, however when I try to set

(setq citar-file-parser-functions
  '(citar-file-parser-default
    citar-file-parser-triplet))

as described in the documentation, I get the following error:

citar--check-configuration: ‘citar-file-parser-functions’ should be a list of functions: '(citar-file-parser-default citar-file-parser-triplet)

If I remove this snippet I do not see the link to the file when clicking on citar links, however when trying to insert links it does show me that a file is there.

To Reproduce
Steps to reproduce the behavior:
My citar configuration is (except for the above snippet) is

;; citar
(use-package citar
    :no-require
    :custom
        (org-cite-global-bibliography '("~/library.bib"))
        (org-cite-insert-processor 'citar)
        (org-cite-follow-processor 'citar)
        (org-cite-activate-processor 'citar)
        (citar-bibliography org-cite-global-bibliography)
    :bind
         (:map org-mode-map :package org ("C-c b" . #'org-cite-insert)))

(use-package citar-org-roam
        :after (citar org-roam)
        :config (citar-org-roam-mode))

(use-package citar-embark
       :after citar embark
       :no-require
       :config (citar-embark-mode))
  1. insert citation
  2. click citation
  3. error message

Expected behavior
A buffer should open with the choice for weblinks, notes and direct links.

Emacs version:
28.2

Activity

  1. bdarcus commented on Apr 12, 2023

    @bdarcus
    Contributor

    Something is weird. What you're trying to set is the default value?

  2. stefangroha commented on Apr 12, 2023

    @stefangroha
    Author

    Maybe I am confused by the documentation, I want to mainly set the triplet value, but I just copy and pasted this in case I want to use zotero as well.

  3. bdarcus commented on Apr 12, 2023

    @bdarcus
    Contributor

    You shouldn't have to set that; I never do, and just use the default.

    The way the parsing code works is it runs through the list of functions until it gets a result. So there's no harm in having both default.

    I mostly use Doom, but have a small experimental init over here, where it should all work correctly (see init-biblio.el in particular).

    https://gitlab.com/bdarcus/emacs-elp

  4. stefangroha commented on Apr 12, 2023

    @stefangroha
    Author

    Without setting the variable I can see that there is a file associated with the bibliography (when trying to insert), however when clicking the citation I only see the weblink and option to create a note and no option to open the pdf. As the file field in the bib-file is set in Mendeley format (:/path/to/pdf:pdf), I thought this part of the documentation would help.

  5. bdarcus commented on Apr 12, 2023

    @bdarcus
    Contributor

    Let's start from the beginning ...

    OK, I see a problem now :-)

    So the parser code and at-point functionality is probably confusing the issue.

    This is what I see:

    If I do M-x citar-open, and then filter the candidates to those that should have files associated with them, I see:

    image

    If I select the first, I see this:

    image

    I think that's what you're seeing also?

    So somehow a bug crept in somewhere.

    Two possibilities, which I guess aren't mutually exclusive:

    1. the indicators are wrong, incorrectly indicating there are files, when there are not
    2. the functions that list the files in the second step is wrong, incorrectly omitting them

    You are saying it's 2, but we'd want to confirm it is, and it's not 1, because it's different code.

  6. bdarcus commented on Apr 12, 2023

    @bdarcus
    Contributor

    Looking at the biblatex file for that entry, it has this as the value of file:

    /home/bruce/Zotero/storage/WZB4FH9E/1405ba54-7f4e-11ea-8013-1b6da0e4a2b7_story.html
    

    The reason that file is not showing up is because the file doesn't actually exist (long story)! So the problem there is 1 above.

    But now I need to figure out why the indicator is wrong.

  7. stefangroha commented on Apr 12, 2023

    @stefangroha
    Author

    That is exactly what I am seeing, however for me in the biblatex file I do see a link to a pdf:

    file = {:Users/smg/Library/Application Support/Mendeley Desktop/Downloaded/Heinz et al. - 2022 - Liver Colonization by Colorectal Cancer Metastases Requires YAP-Controlled Plasticity at the Micrometastatic Stage.pdf:pdf}
    

    which indeed exists at this location.

  8. bdarcus commented on Apr 12, 2023

    @bdarcus
    Contributor

    Can you confirm the file is actually there, though, with that precise path? The code won't list it if it can't find it.

    Not sure what OS you're on (Mac OS?), but this is what I see on Linux from the console:

    ❯ ls /home/bruce/Zotero/storage/WZB4FH9E/1405ba54-7f4e-11ea-8013-1b6da0e4a2b7_story.html
    ls: cannot access '/home/bruce/Zotero/storage/WZB4FH9E/1405ba54-7f4e-11ea-8013-1b6da0e4a2b7_story.html': No such file or directory
  9. changed the title [-]citar-file-parser-functions not working[/-] [+]file indicator incorrectly is incorrect[/+] on Apr 12, 2023
  10. stefangroha commented on Apr 12, 2023

    @stefangroha
    Author

    Ah, thank you so much, the file path from the Mendeley export did not have the initial "/' (before Users), which was of course needed. It does work now, if I change the bib file to the correct link.

  11. bdarcus commented on Apr 12, 2023

    @bdarcus
    Contributor

    I'm going to reopen this, then, as in that case, the UI should not indicate it as present.

  12. bdarcus commented on Apr 12, 2023

    @bdarcus
    Contributor

    @roshanshariff - before I dig into fixing this, anything I should know?

    Basically, right now, citar-has-files (well, really citar-file--has-file-field) does not check if a file exists, but the open commands do.

    So one can get an the indicator for an entry say it has a file, but it actually doesn't.

    Is that perhaps for performance reasons, or is this an actual bug/oversight?

    I can modify that function something like this:

      (when citar-file-variable
        (lambda (citekey)
          (and (citar-file--parse-file-field
                (or (citar-get-value citar-file-variable citekey) "") nil) t))))

    ... but that would also require changes in citar-file--parse-file-field.

  13. changed the title [-]file indicator incorrectly is incorrect[/-] [+]file indicator incorrectly indicating [/+] on Apr 13, 2023
  14. roshanshariff commented on Apr 13, 2023

    @roshanshariff
    Collaborator

    @bdarcus, this was an intentional change for performance reasons, so that showing the indicator doesn't require parsing the file fields, checking if files exist, and so on. Since this function is called for every bibliography item every time you call any citar command, that would be prohibitively slow.

    Instead, it just checks if each entry has a file field. I remember that we discussed this in the relevant PR, but can't find the link right now. I think we also agreed that it's a reasonable user experience, since the item is "supposed" to have an attached file, and if you get an error when you open it you can investigate why it's not there.

  15. bdarcus commented on Apr 13, 2023

    @bdarcus
    Contributor

    I thought that might be the case; sounds familiar.

    Except the file isn't listed in the related resources UI, so you don't get the error; just a discrepancy between the two UI views.

    So maybe we should list them even if in error?

    And do not the files in directories get checked?

  16. roshanshariff commented on Apr 13, 2023

    @roshanshariff
    Collaborator

    So maybe we should list them even if in error?

    Yes, I think all the files in the file field should be listed even if they don't exist. This is going to take some thinking, because currently the file field parser functions use the non-existence of the file to filter out bad parses (e.g. treating a triplet-style file field as a plain filename). But perhaps we can refine the parsing functions so we don't have to do this.

    And do not the files in directories get checked?

    Yes, for files that are found in the library path (i.e. not listed in the file field in a bibliography) the indicator does reflect whether the file actually exists.

  17. bdarcus commented on Apr 13, 2023

    @bdarcus
    Contributor

    Yes, for files that are found in the library path (i.e. not listed in the file field in a bibliography) the indicator does reflect whether the file actually exists.

    Is that faster than splitting a string and checking the path(s)?

    I guess the confusing part is also some files are checked differently than other files; that the indicator means different things depending.

  18. roshanshariff commented on Apr 13, 2023

    @roshanshariff
    Collaborator

    The difference is that, for library files, we can just scan the directory once and figure out which keys have files, because each filename starts with the key. Not having a file for a key is not an error; it just means you don't have an attached file. On the other hand, explicitly listing a non-existent file in the bibliography seems like a data error. Perhaps we could handle it by having a linting function that checks for non-existent files and offers to remove them, which you would run when needed?

    From a performance point of view, the files listed in file fields needn't be in any particular location, so we can't scan a directory beforehand. So we take the existence of a file field as a signal that the item has attached files.

  19. bdarcus commented on Apr 13, 2023

    @bdarcus
    Contributor

    On the other hand, explicitly listing a non-existent file in the bibliography seems like a data error. Perhaps we could handle it by having a linting function that checks for non-existent files and offers to remove them, which you would run when needed?

    Makes sense.

    I was also wondering about using faces to distinguish files that are confirmed to exist, and those that are confirmed to not exist, or unconfirmed?

    But maybe that gets too complicated.

  20. roshanshariff commented on Apr 13, 2023

    @roshanshariff
    Collaborator

    If I remember correctly, annotation and affixation functions are only called for visible candidates, not all candidates. So you could do (relatively) expensive work in those functions. But they can only change the visible representation of each candidate (like icons and other prefixes/suffixes, but not faces). They can't modify the actual candidate text like the searchable has:files tag.

    And you're right, doing all this adds complexity which we'd have to weigh against the usefulness of the feature.

  21. bdarcus commented on Apr 13, 2023

    @bdarcus
    Contributor

    Yes, that also makes sense. Annotations or affixations might be appropriate for that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions