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

Make #+pandoc-emphasis-pre work as expected (#8059) #8360

Merged
merged 4 commits into from Oct 10, 2022
Merged

Conversation

adql
Copy link
Contributor

@adql adql commented Oct 8, 2022

The main issue with the original implementation is that it kept the assumption that no emphasis markers may occur directly after most non-whitespace chars, e.g. in the middle of the word (in Pandoc terms: following native Str). But with the possibility to modify which chars are allowed around emphasized text, Pandoc should remain indifferent in that respect.

These commits allow it and modify some tests accordingly. This is both:

  1. in accordance with Org-mode's behavior when customizing org-emphasis-regexp-components.
  2. reasonable in many languages1 (but rare in English).

Resolves #8059.

Footnotes

  1. which is the case in Hebrew and German, both of which I speak.

Since #+pandoc-emphasis-pre and #+pandoc-emphasis-post have somewhat
different implementations, it is wise to test them
separately (e.g. issue jgm#8059 affects only the pre char setting).

The addition of tests for an alphanumeric pre/post char demonstrates
the aforementioned issue.
So far, orgStateLastPreCharPos wasn't updated appropriately after each
parsing to native Str (by the parser str). In addition to solving
this, the guard notAfterString in emphasisStart is removed to allow
emphasis after Str at the first place.
With the introduction of the settings for allowed pre/post chars
around emphasized text (and the resolve to jgm#8059), such markup-chars
may appear in the middle of the word according to the user's choice.
@adql
Copy link
Contributor Author

adql commented Oct 8, 2022

By the way, I originally used *> in updatePositions', but hlint suggested to replace it with $>. This is very strange because it's wrong. Replacing with >> eliminated the suggestion. Any explanation to that?

@jgm jgm requested a review from tarleb October 8, 2022 14:30
@tarleb
Copy link
Collaborator

tarleb commented Oct 9, 2022

Thank you, this is excellent.

hlint sometimes suggests functions that'd need a separate import; this would work though: updatePositions str' = str' <$ update....

The function T.last is partial and throws an error when passed "". While we know that this can never happen, we have the general policy of avoiding partial functions if possible: could you change the code to use T.unsnoc to replace T.last?

@adql
Copy link
Contributor Author

adql commented Oct 9, 2022

this would work though: updatePositions str' = str' <$ update....

Yes, I see how this expression is useful. Makes it clear that we're here for the side effect.

@tarleb tarleb merged commit 31aa660 into jgm:master Oct 10, 2022
@tarleb
Copy link
Collaborator

tarleb commented Oct 10, 2022

Thank you!

@adql
Copy link
Contributor Author

adql commented Oct 10, 2022

My pleasure :)

@adql adql deleted the issue8059 branch October 10, 2022 18:02
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.

Org-mode reader: #+pandoc-emphasis-pre doesn't work as expected
2 participants