Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unrobust reading of bad @Comment #1632

Closed
oscargus opened this issue Jul 27, 2016 · 13 comments
Closed

Unrobust reading of bad @Comment #1632

oscargus opened this issue Jul 27, 2016 · 13 comments
Labels
bug Confirmed bugs or reports that are very likely to be bugs

Comments

@oscargus
Copy link
Contributor

JabRef version

I got a BIB file from a comment to clean up and it couldn't be read. The "Error opening file" gave the informative message "null".

The reasons seems to be this line:

@Comment Any references

as the log message says (the line above is line 5):

11:28:12.979 [JabRef CachedThreadPool] ERROR net.sf.jabref.importer.OpenDatabaseAction - Error loading database C:\Users\Oscar\Documents\bibl.bib
java.io.IOException: Error in line 5: Expected { but received  
    at net.sf.jabref.importer.fileformat.BibtexParser.consume(BibtexParser.java:915) ~[bin/:?]
    at net.sf.jabref.importer.fileformat.BibtexParser.parseBracketedTextExactly(BibtexParser.java:856) ~[bin/:?]

Not sure what I expect to happen, but maybe that the error message "Error in line 5: Expected { but received " is shown. Not sure if it is worthwhile to support this somehow understandable interpretation of @comment though...

@oscargus oscargus added the bug Confirmed bugs or reports that are very likely to be bugs label Jul 27, 2016
@Siedlerchr
Copy link
Member

The @Comment is a legal way of commenting out sth. The line should be ignored by the parser.
I found some more details explaining this:
http://maverick.inria.fr/~Xavier.Decoret/resources/xdkbibtex/bibtex_summary.html#comment

@oscargus
Copy link
Contributor Author

Good link! If we trust it, I quote:

Actually, the rule is that everything from the @comment and to the end of line is ignored.

@matthiasgeiger
Copy link
Member

... I think the current implementation is "wrong" with respect to the information provided in the link as JabRef only accepts @comment { .... } as valid comments... right? @lenhard

@oscargus
Copy link
Contributor Author

The tricky/interesting thing here is probably that by writing @comment instead of an entry type, the next lines are also treated as comments (as they do not start with @ or are inside a non-Comment entry type). So it is quite important to keep those comment lines. :-)

@Siedlerchr
Copy link
Member

I tried to find some more information on this topic, but I can't find any more details, if it used as block or not.
Even in the original BibTeX manual it is just written that @Comment is allowed for compatibility reasons of Scribe

@lenhard
Copy link
Member

lenhard commented Jul 27, 2016

I had a quick look at the parser.

@matthiasgeiger: Currently, we accept @comment, but the casing does not matter (@cOmMeNt should also work with the current implementation).

@oscargus: The error occurs, because the parser expects something that starts with @ to be eventually followed by {. If it doesn't find an opening curly bracket, it throws this error. That is really independent of comments or entry types.

If I get this issue right we should be able to parse comments that are not followed by curly braces? I would really love some more official documentation on bibtex syntax (really an real official information you find would be very welcome), but I guess the best we have is a compiler.

Last but not least: Note that the current implementation drops any user comments from the database, i.e., anything within @comment that does not have a meta data flag or a custom entry type. A way to avoid this is to not use @comment, but just any text without @, which, in the current implementation should be kept.

@Siedlerchr
Copy link
Member

@lenhard the official BibTex source can be found here: https://www.ctan.org/tex-archive/biblio/bibtex/base
However, there is no gold-standard, but most parsers I gave a quick look accept at least @Comment{}

@lenhard
Copy link
Member

lenhard commented Jul 27, 2016

@Siedlerchr yes, unfortunately it is a bit scarce on comments.

So far I do not see a consensus here. Is there something that we should change about the parser? @JabRef/developers what do you say? If yes, what exactly is it and how would the desired behavior look like?

@tobiasdiez
Copy link
Member

As far as I know @Comment is a value-only entry type, exactly like @Preamble. Thus it expects an argument (which might be empty) between braces.
Since there is no defined bibtex standard, I would propose to use the official biblatex parser as an authoritative source. I don't have the time to check the code carefully, but https://github.com/ambs/Text-BibTeX/blob/5ec5ec913122f0d17246e8fa5c764eef6645944f/btparse/tests/data/comment.bib suggests that at least some form of parentheses should be used.

What we should do is to display a more informative error message.

@oscargus
Copy link
Contributor Author

I read up a bit and it seems like @comment in the (authoritative) BibTeX application works in two ways:

  • If it is followed by a brace, everything is commented out until the next matching brace (with "brace" being at least {}, "", and, based on Tobias link, ()). See a footnote in Tame the BeaST for this, where it is stated that the only practical use for it is to comment out large parts of a bib-file since it is enough to just remove the @ for a single entry.
  • If it is not followed by a brace, the rest of the line is the comment (and as a consequence, following lines may also be comments...). My colleagues file have been working for years before I loaded it in JabRef. Some internet documentation supports this, so people may use it.

@lenhard
Copy link
Member

lenhard commented Jul 29, 2016

Have a look at #1638

Comments marked by @Comment are now supported, regardless of whether they use brackets or not. I have added automated tests for this and tested it manually, but @oscargus please verify with the bib file of your colleague.

Essentially, it took a mere five lines of code (one try catch block) and a change in one method call to implement this. The fact that so few changes are needed shows that we are finally making progress in the extensibility of JabRef. All the quality refactoring finally starts to pay of. I'm happy right now :-)

@oscargus
Copy link
Contributor Author

@lenhard Loads well! Some (expected) info from the logger, but nothing else. It moved four entries for unknown reasons (from random places at the end to the beginning of the file), but unlikely that it has anything to do with this. No @Comment nearby.

@lenhard
Copy link
Member

lenhard commented Jul 29, 2016

Fixed by #1638

@lenhard lenhard closed this as completed Jul 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs
Projects
None yet
Development

No branches or pull requests

5 participants