jimmejardine / qiqqa-open-source

The open-sourced version of the award-winning Qiqqa research management tool for Windows
GNU General Public License v3.0
380 stars 64 forks source link

file not found when using jabref-meta: fileDirectory #349

Open Ritsuka314 opened 3 years ago

Ritsuka314 commented 3 years ago

I'm running v82.0.7568.29227 (v82pre11).

When importing from a JabRef bibtex file I get a warning dialog saying The file you selected had xx entries, but xx of them ... the file could not be found on the disk in the location specified by the file entry.

The issue can be reproduced from the following bibtex file

% Encoding: UTF-8

@InProceedings{Barendregt1988,
  author         = {Henk P. Barendregt and Erik Barendsen},
  booktitle      = {Aspen{\ae}s Workshop on Implementation of Functional Languages, G{\"{o}}teborg. Programming Methodology Group, University of G{\"{o}}teborg and Chalmers University of Technology},
  title          = {Introduction to lambda calculus},
  year           = {1988},
  volume         = {85},
  endnotereftype = {Conference Proceedings},
  file           = {:Barendregt1988 - Introduction to Lambda Calculus.pdf:PDF},
  shorttitle     = {Introduction to lambda calculus},
}

@Comment{jabref-meta: databaseType:bibtex;}

@Comment{jabref-meta: fileDirectory:pdfs;}

placed in a folder with the following structure:

/
|- bib.bib
|- pdfs
   |- Barendregt1988 - Introduction to Lambda Calculus.pdf

Interestingly, Qiqqa seems to be ignoring completely the line @Comment{jabref-meta: fileDirectory:pdfs;}. If I move the pdf in the same directory as the bib file, even without removing the @Comment, and even if I put a non-existing folder in it, Qiqqa is able to find the pdf.

The same issue also happens with v83.0.7656.6401 and v82pre3. The issue does not happen with v79. EDITED: also not with v80

I tried to look at the source code and found the relevant places this and this. They look perfect to me, so I cannot tell anything's wrong with the code or with my bib file. I tried to compile the code to trace the execution, but unfortunately it seems my build tools are not properly set up.

Appreciates for making Qiqqa❤️ and any help!

Ritsuka314 commented 3 years ago

After realizing that the issue is introduced somewhere between v80 and v82pre3, I looked at the merged diff between 77469b and 190ff0.

The file Utilities/BibTex/Parsing/BibTexLexer.cs caught my eyes:

image

For the first two cases of the switch, shouldn't we skip the brackets by doing ++c before calling ParseUntilDelim, or otherwise the comment would begin with a bracket, so that the later check of whether the comment begins with jabref-meta: fileDirectory: here would always return false?

When I tried to change the bibtex line @Comment{jabref-meta: fileDirectory:DES;} to @Comment jabref-meta: fileDirectory:DES;\n, to ensure that we get into the default case in the screenshot above, Qiqqa is able to find the pdf file.

I'd really like to test the fix out, but I'm still unable to compile the code (never actually programmed in c#). I'm going to send a pull request so it's more clear what I meant. I realized we cannot do bibtex[c++] because it would unwantedly gobble a character in the default case.

GerHobbelt commented 3 years ago

(pullreq mentioned = #350)

👍 Thanks for reporting and the in-depth investigation + pullreq! Apologies for the late reply 🤒 .

BibTeX is indeed one of the fickle areas of the Qiqqa codebase; it's been a while since I rooted around in that part but I did have some run-ins with the current Qiqqa BibTeX parser myself -- particularly when handling slightly off-kilter BibTeX files 😓 -- and I've been wondering what the smart thing to do is...

Anyway, looks like you've found a bug in my patches, which were intended to make Qiqqa behave when being fed @comment and other @ tags, including incomplete ones, e.g. @delete and @delete{} -- invalid BibTeX like that can be entered by hand and should be coped with.

I missed the jabref @comment usage then. 😢

👍 Thanks for catching this one; will take a bit to integrate this (due to my limited time ATM) and will be included in the next release.

GerHobbelt commented 3 years ago

Related: #68; #21