Repository: sciunto-org/python-bibtexparser
Pull request #416 (2023-11-04 02:29:20)

Fix blocks not starting on new lines (#411)
This PR modifies the `@` marker regex so that it doesn't necessarily appear at a new line. It also passes all the tests.

zepinglee: > A more significant problem is on [line 175](https://github.com/sciunto-org/python-bibtexparser/pull/416/files#diff-9928b8e2ef0ad92b3a5aa9b829f3fb50032950bc14afd79a085b47df6df2005dL175). Currently, we discover if a new block starts with a previous one still open, which typically highlights an error in the parsed file. In such a case, we abort processing the still open block, in deliberate deviation from "strict bibtex parsing". As we only considered `@....` on the start of a new line, the risk of flagging a "false positive" was negligible, but at the same time the user experience increased drastically.

I don't think the `_move_to_comma_or_closing_curly_bracket()` method where line 175 is located matches the original BibTeX's behavior. BibTeX only accepts strictly brace-balanced string in a literal field token (in terms of the `bibtex.web`) even when the outer delimiters are quotes. The braces or quotation marks in a field token are not treated as escaped characters. In addition, an `@` character doesn't break the field.

https://github.com/sciunto-org/python-bibtexparser/blob/770eb001349dcad48149bc101f07cfc88282d8b3/bibtexparser/splitter.py#L131-L189

In the example of `@entry{key, title="all blocks must start with @entry{"}`, the second quotation mark is in brace level 1 and it doesn't terminate the field token. The final right brace make the string balanced and BibTeX look for another quotation mark to end the field. This is verified by running BibTeX and the raised error `Illegal end of database file`. Yet `@entry{key, title="all blocks must start with @entry{"}"}` works fine with BibTeX and the `title` field is `all blocks must start with @entry{"}`.

I'm planning to rewrite this method and make another PR.
MiWeiss: Thanks for your PR update and for your long comment.

#### First things first:
Your argument convinced me that a plain `@entry{` would be very rare, and I thus see no reason not to merge this PR anymore. We may change on this feature in the future, based on received feedback, but for now I guess merging this PR is a clear improvement of things!

#### Regarding the larger argument you made:
 This library is deliberately not 100% consistent with bibtex, but it attempts to be somewhat more fault-tolerant: We aim to be able to handle cases when there are small mistakes in the bibtex file gently, without messing up the parsing of subsequent blocks, or even prevent parsing at all. This is a request we received numerous times reagarding bibtexparser v1. Take the following example:

```
@string{myKey = {myValue))

@string{anotherString = {anotherValue}}
```

While the intention of the writer here is pretty clear, bibtex would not be happy: Note the `))` on the first line which should be `}}`. Technically, the braces are not balanced after the first line, thus not allowing for a new string on the third line. Previously, in `bibtexparser v2`, we thus aborted the parsing of a block if a line started with a "block start", e.g. a `@entry{`. This allowed a "fresh" start with that block. After merging this PR, we now do this even if the "block start" appears anywhere, not just on a new line. 

Thus, I am very happy to review any improvement you make to the parsing (and I am sure there are plenty to be made ;-)), but please keep in mind that we take a deliberate trade-off between sticking to the pure bibtex rules, and user friendly parsing of imperfect files. If in doubt, before investing too much time, feel free to ask for mine (and anyones) comment in a seperate issue before investing too much time.

#### Most importantly
 Thanks again for your contribution, and I am looking forward to your next PRs.




Patch:
diff --git a/bibtexparser/splitter.py b/bibtexparser/splitter.py
index 72ce58e..ac19f77 100644
--- a/bibtexparser/splitter.py
+++ b/bibtexparser/splitter.py
@@ -254,7 +254,7 @@ class Splitter:
             The library with the added blocks.
         """
         self._markiter = re.finditer(
-            r"(?<!\\)[\{\}\",=\n]|(?<=\n)@[\w]*( |\t)*(?={)", self.bibstr, re.MULTILINE
+            r"(?<!\\)[\{\}\",=\n]|@[\w]*( |\t)*(?={)", self.bibstr, re.MULTILINE
         )
 
         if library is None:
@@ -395,7 +395,7 @@ class Splitter:
             entry_type=entry_type,
             key=key,
             fields=fields,
-            raw=self.bibstr[m.start() : end_index + 1],
+            raw=self.bibstr[m.start() : end_index],
         )
 
         # If there were duplicate field keys, we return a DuplicateFieldKeyBlock wrapping


Test patch:
diff --git a/tests/splitter_tests/test_splitter_basic.py b/tests/splitter_tests/test_splitter_basic.py
index c65a601..a969ef8 100644
--- a/tests/splitter_tests/test_splitter_basic.py
+++ b/tests/splitter_tests/test_splitter_basic.py
@@ -319,3 +319,79 @@ def test_handles_duplicate_strings():
     assert isinstance(
         lib.failed_blocks[0].ignore_error_block, bibtexparser.model.String
     )
+
+
+blocks_not_starting_on_new_lines = """\
+@article{KUEH2023S98,
+title = {Myocardial Characterisation Using Delayed Dual-Energy Cardiac Computed Tomography},
+author = {S.-H. Kueh and J. Benatar and R. Stewart}
+}@INPROCEEDINGS{9837531,
+  author={Hassan, Mona Bakri and Saeed, Rashid A. and Khalifa, Othman and Ali, Elmustafa Sayed and Mokhtar, Rania A. and Hashim, Aisha A.},
+  title={Green Machine Learning for Green Cloud Energy Efficiency},
+  doi={10.1109/MI-STA54861.2022.9837531}}@ARTICLE{9372936,
+  author={Hu, Ning and Tian, Zhihong and Du, Xiaojiang and Guizani, Nadra and Zhu, Zhihan},
+  title={Deep-Green: A Dispersed Energy-Efficiency Computing Paradigm for Green Industrial IoT},
+  doi={10.1109/TGCN.2021.3064683}}@ARTICLE{5445167,
+  author={Kumar, Karthik and Lu, Yung-Hsiang},
+  title={Cloud Computing for Mobile Users: Can Offloading Computation Save Energy?},
+  doi={10.1109/MC.2010.98}}
+"""
+
+
+@pytest.mark.parametrize(
+    "expected",
+    [
+        {
+            "key": "KUEH2023S98",
+            "type": "article",
+            "raw": "@article{KUEH2023S98,\n"
+            "title = {Myocardial Characterisation Using Delayed Dual-Energy Cardiac Computed Tomography},\n"
+            "author = {S.-H. Kueh and J. Benatar and R. Stewart}\n}",
+            "start_line": 0,
+        },
+        {
+            "key": "9837531",
+            "type": "inproceedings",
+            "raw": "@INPROCEEDINGS{9837531,\n"
+            "  author={Hassan, Mona Bakri and Saeed, Rashid A. and Khalifa, Othman and Ali, Elmustafa Sayed and Mokhtar, Rania A. and Hashim, Aisha A.},\n"
+            "  title={Green Machine Learning for Green Cloud Energy Efficiency},\n"
+            "  doi={10.1109/MI-STA54861.2022.9837531}}",
+            "start_line": 3,
+        },
+        {
+            "key": "9372936",
+            "type": "article",
+            "raw": "@ARTICLE{9372936,\n"
+            "  author={Hu, Ning and Tian, Zhihong and Du, Xiaojiang and Guizani, Nadra and Zhu, Zhihan},\n"
+            "  title={Deep-Green: A Dispersed Energy-Efficiency Computing Paradigm for Green Industrial IoT},\n"
+            "  doi={10.1109/TGCN.2021.3064683}}",
+            "start_line": 6,
+        },
+        {
+            "key": "5445167",
+            "type": "article",
+            "raw": "@ARTICLE{5445167,\n"
+            "  author={Kumar, Karthik and Lu, Yung-Hsiang},\n"
+            "  title={Cloud Computing for Mobile Users: Can Offloading Computation Save Energy?},\n"
+            "  doi={10.1109/MC.2010.98}}",
+            "start_line": 9,
+        },
+    ],
+)
+def test_blocks_not_starting_on_new_lines(expected: Dict[str, Any]):
+    """Test the new blocks that are not on new lines.
+
+    Discussed at https://github.com/sciunto-org/python-bibtexparser/issues/411.
+    """
+    import bibtexparser
+
+    lib = bibtexparser.parse_string(blocks_not_starting_on_new_lines)
+    assert len(lib.entries) == 4
+    entry_by_key = lib.entries_dict
+    assert len(entry_by_key) == 4
+
+    entry = entry_by_key[expected["key"]]
+    assert entry.key == expected["key"]
+    assert entry.entry_type == expected["type"]
+    assert entry.raw == expected["raw"]
+    assert entry.start_line == expected["start_line"]