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

Org reader: Fix Post standing for Pre in setEmphasisPreChar #8134

Merged
merged 1 commit into from
Jun 19, 2022

Conversation

adql
Copy link
Contributor

@adql adql commented Jun 19, 2022

Small typing mistake which causes e.g. )/emph/) and (/noemph/) on the default pre-chars.

@adql
Copy link
Contributor Author

adql commented Jun 19, 2022

I discovered this by chance while trying to work on #8059. It appeared in this commit about 4 years ago, go figure how this hasn't disturbed anyone so far (couldn't find a relevant issue).

@tarleb
Copy link
Collaborator

tarleb commented Jun 19, 2022

Thank you, good catch! If I understand correctly, then the bug was triggered only when the new "pre" setting could not be parsed.

@tarleb tarleb merged commit 5fe154d into jgm:master Jun 19, 2022
@adql
Copy link
Contributor Author

adql commented Jun 19, 2022

As I understand, it actually affected all parsing when no pandoc-emphasis-pre is set, i.e. the default behavior (since setEmphasisPreChar pulls the default from defaultOrgParserState when no setting is given, and until now it accidentally pulled the Post).

Kind of surprising that it went unnoticed, but then the only affected characters in the default setting are the directional ones ( and {. Maybe it's not very common to emphasize directly after an opening parenthesis.

@adql adql deleted the org-emphasis branch June 19, 2022 18:26
@tarleb
Copy link
Collaborator

tarleb commented Jun 19, 2022

The function setEmphasisPreChar is called when the org-file contains a #+pandoc-emphasis-pre line. Pandoc treats the rest of that line as a Haskell expression that defines a string, i.e., it should be surrounded by double quotes. If that succeeds, then the pre chars are updated. If, however, the line does not contain a valid Haskell string, then setEmphasisPreChar is called with Nothing as the first argument. Only then does the fromMaybe evaluate the fallback value (its first argument).

The idea behind that mechanism is that a sole #+pandoc-emphasis-pre, without any arguments, will reset to the default behavior. That mechanism, as you found, was broken. The value was set to the allowed post chars instead of reset to its initial value.

@adql
Copy link
Contributor Author

adql commented Jun 20, 2022

That's interesting. Where is the default behavior actually defined then?

I should mention that #+pandoc-emphasis-pre actually hardly works at all. I tested several cases and it seems like #8059 is broader than at first seemed. Most characters that I tried to add failed. The test in the suite only tests for [, which happens to pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants