[ISSUE-12929] Handle BibTeX reserved characters in comments. #13566
Conversation
|
@koppor I've implemented this according to your suggestion: |
|
Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of |
| @@ -1079,6 +1079,7 @@ void parsePreambleAndEntryWithoutNewLine() throws IOException { | |||
| } | |||
|
|
|||
| @Test | |||
| @Disabled | |||
There was a problem hiding this comment.
It tests definition of preamble in a comment, here's tested line:
\% Encoding: US-ASCII@preamble{some text and \latex}
According to 'Check if line starts with comment indicator string: %, #, --, *, // - and then treat the whole line as comment' this test should not parse preamble but treat it as commented out definition.
There was a problem hiding this comment.
Then modify the test or remove it? Or add a reason in Disabled("disabled becuase of...)
There was a problem hiding this comment.
@Siedlerchr I didn't remove it because I wanted to start discussion. This test contradicts proposed solution - as I said, I'm not familiar with the BibTeX standart - someone wrote this test as a valid test case and I got puzzled. I think it is better to remove it since it is not a valid test case or leave it but assert that preable won't be parsed but treated as commented out line.
There was a problem hiding this comment.
Please give us time to.think.
We need to inspect existing bib files, setup a CI pipeline to test this behavior. Somewhere wr have started this.
You fan also propose something....
|
@trag-bot didn't find any issues in the code! ✅✨ |
|
@koppor Not in line with BibTeX -> I'm not familiar with BibTeX, could you elaborate on this so I culd better understand the issue I'm trying to fix? The solution with checking for column 1 was better. -> Will have a look and refactor No tests included for all cases. -> What are the missing test cases? I'm not familiar with BibTeX and description of this bug mentions only single case. If you could hint me with the missing cases It will help me to better understand the problem. Tests seem to contain multiple assert statements -> Do you mean checking various comments in sinlge test or invocation of 'assertEquals' method? |
|
@ankamde its a good third issue, thus we assume some familiarity with BibTeX. Google for "tame the beast"/BibTeX (you will find a decent description of it). And then search for the biblatex manual. (On the road, sorry for the short answer; wanted to give you pointers early) |
|
We also have https://docs.jabref.org/cite/bibtex-and-biblatex. |
| @Test | ||
| void parseWithBibLaTeXReservedCharacterInComments() throws IOException { | ||
| String bibtexEntry = """ | ||
| % Type of BibLaTeX entries : @article |
There was a problem hiding this comment.
All types of comments checked in a single test. Needs to be split.
Maybe parameterized test?
| ParserResult result = parser.parse(Reader.of(bibtexEntry)); | ||
| Collection<BibEntry> entries = result.getDatabase().getEntries(); | ||
| BibEntry entry = entries.iterator().next(); | ||
|
|
There was a problem hiding this comment.
See testing at DevDocs that one can also do a comparison of rich objects. Should be done here.
|
Thanks for the hints. I unasigned myself from this task - need to learn more about biblatex and I don't want to block this issue. Perhaps someone will provide correct solution faster? I will leave this PR as open for a while. |
|
Idea:
A .tex file could look like \documentclass{article}
\begin{document}
\nocite{*}
\bibliographystyle{plain}
\bibliography{references}
\end{document} |
Closes #12929
Steps to test
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)