markdown escaping: Fix injection of escapes#1822
Conversation
c437630 ("Fix markdown character escaping bug (issue py-pdf#1236)") fixed the escaping of multiple markdown markers, but it dropped dividing by two the number of escaped characters being injected: - txt_frag[: -((num_escape_chars + 1) // 2)] + for _ in range(escape_run - 1): This restores dividing by two the number of escapes, and restores the test results accordingly, making it the behavior closer to https://spec.commonmark.org/dingus/ Fixes: py-pdf#1215
|
Thanks for opening this PR @sthibaul , and sorry it has taken me so long to respond. What I’m trying to evaluate is that this PR introduces new behavior for backslashes that are not escaping markdown markers. For example, I understand this moves us closer to CommonMark-style escaping, but it also changes existing behavior for users of I am currently leaning towards merging it. @Lucas-C or @CoLa5 if you have any input I would appreciate. |
Consequently, here I am assuming the current FPDF-specified markdown syntax as being defined by:
Coming to the issue: Starting with the simple escape case:
The case defines clearly that a leading backslash character The arising questions are:
For me, both questions lead to same answer, escape the escape character to print a backslash:
Currently (before this pull request), the escaping of the markdown escape character (case 2) is wrong because the escaping of the escape character is printed as:
making it impossible to show a single backslash in front a markdown style marker. So, this pull request seems to fix this issue, so I would accept it. All the versions between c437630 and now had a bug introduced by the referenced commit which is fixed by this pull request. The only remaining question is: I would suggest to follow Python style, accept them and print them as being escaped, accompanied by a >>> pdf.multi_cell(
... w=pdf.epw,
... text="\\ example \\**text \\\\__case__",
... dry_run=True,
... output="LINES",
... )
["\\\\ example \\**text \\\\__case__"]but I will open another issue for this! |
c437630 ("Fix markdown character escaping bug (issue #1236)")
fixed the escaping of multiple markdown markers, but it dropped dividing by two the number of escaped characters being injected:
This restores dividing by two the number of escapes, and restores the test results accordingly, making it the behavior closer to https://spec.commonmark.org/dingus/
Fixes: #1215
A unit test is covering the code added / modified by this PR
In case of a new feature, docstrings have been added, with also some documentation in the
docs/folderA mention of the change is present in
CHANGELOG.mdThis PR is ready to be merged
By submitting this pull request, I confirm that my contribution is made under the terms of the GNU LGPL 3.0 license.