Bold/Italic bug

I think I'm running up against another bold/italics bug. I did some quick searches and it looks like the other issues were considered resolved, sorry if I'm re-reporting on something already fixed that hasn't made it upstream yet.

Installed from pip current Python-Markdown version 3.0.1

The raw markdown line that breaks is:

This is text **bold *italic bold*** with more text

The output I'm getting is as follows:

<p>This is text <strong>bold *italic bold</strong>* with more text</p>

However, the following format does seem to work correctly.

This is text ***bold italic** italic* more text

The output is

<p>This is text <em><strong>bold italic</strong> italic</em> more text</p>

I can confirm I am seeing the same behavior in 3.0.1. However, this behavior does not exist in the latest code here on GitHub and will be available in the next release.

>>> import markdown
>>> markdown.version
'3.1.dev0'
>>> markdown.markdown('This is text ***bold italic** italic* more text')
'<p>This is text <em><strong>bold italic</strong> italic</em> more text</p>'

@waylan

@waylan, I don't think you tested is case properly. It wasn't This is text ***bold italic** italic* more text that failed, but This is text **bold *italic bold*** with more text.

But I will explain a little bit why this happens.

In the pattern This is text **bold *italic bold*** with more text, the parser Sees **bold *italic bold** and makes it bold *italic bold. This leaves two * in separate tags, and they cannot be resolved or you'd end up with some weird broken HTML.

The 3rd party pmdownx.betterem can handle this, but only when smart_enable is set to handle the star syntax (it only does "smart" for underscore by default).

>>> markdown.Markdown(extensions=["pymdownx.betterem"], extension_configs={"pymdownx.betterem": {"smart_enable": "all"}}).convert("This is text **bold *italic bold*** with more text")
'<p>This is text <strong>bold <em>italic bold</em></strong> with more text</p>'

Without smart enabled for *, it does what Python Markdown does by default:

>>> markdown.Markdown(extensions=["pymdownx.betterem"]).convert("This is text **bold *italic bold*** with more text")
'<p>This is text <strong>bold *italic bold</strong>* with more text</p>'

Based on how Python Markdown works, this behavior is as I would expect. One sure fire way to avoid such confusion is to use syntax like this:

This is text **bold _italic bold_** with more text

If not, something like pymdownx.betterem may be a way to handle this. Or, we'd have to do some similar logic over here.

If we find Python Markdown's behavior acceptable here, then we should just close this.

Oops, apparently I copied the wrong example when I tested that. And Babelmark clearly indicates we are in the minority here. This is a bug.

@facelessuser

I'm not sure how the parser works for the markdown as I've not been in the code but I'm currently writing a desktop browser/editor that uses markdown in the documents for a document store.

I added formatting to my editor as is typically done. It just makes the editor code more readable. I had my bold/italics formatting regex base assumptions that bold is always the inside two asterisks when working together with italics. I'm not sure if that makes sense for the markdown package or not but I'm parsing flawless formatting in my editor at the moment in terms of italics/bold and using an asterisk. I've yet to implement and support the underscore for formatting in the editor.

I'll take a look at the betterem options to resolve the issue since I'm already using the package anyways. Thank you for the feedback @facelessuser Either way, if the bug exists, we probably want to stomp on it. if it doesn't, I apologize for wasting your time.

Thank you to the whole team for the effort you put into this project.

@facelessuser

I had my bold/italics formatting regex base assumptions that bold is always the inside two asterisks when working together with italics.

That is not a safe assumption to make. That's why @facelessuser suggests usign _ and * together so you can control which is inside, and which is outside.

This is text **bold _italic bold_** with more text

Or do we want to consolidate some of the rules?

What did you have in mind?

We would have to insert between, is that okay?

Yes, that is what I would expect.

What did you have in mind?

I'm not sure if it will cause problems to reduce all the * related patterns to a single pattern and all _ to a single pattern (we could), but maybe at the very least combine the special cases ***text**text* and **text*text*** etc?

We've given the impression that all the standard patterns are on boundaries of 10s and that you can define your custom patterns in between, but we have nowhere to put these new patterns, and may overwrite a position that is being used by a custom extension.

I figure we could use the new Pattern format, and in a single pattern loop through our different regex for *. We could do this to include all the related patterns or just a couple.

One problem would be that some people still use the deprecated method of inserting relative to a name, so if we consolidated all patterns, we might break some extensions. If we just append this new pattern to an existing pattern, you'd at least not break parity.

You just reminded me that the deprecation path for the old add method should probably be taken to the next step in the 3.1 release (raise an error rather than a warning). That being the case, I don't think that is a concern.

That said, I understand the concern about breaking the "step by 10" model we have now. In 3.0 we significantly altered the way inline parsing can work, while in practice we made very few actual changes. If you are suggesting taking this to the next step, then that seems like a reasonable approach. I like the idea that all strong processors are combined into one, if that is reasonable to accomplish.

That said, I understand the concern about breaking the "step by 10" model we have now. In 3.0 we significantly altered the way inline parsing can work, while in practice we made very few actual changes. If you are suggesting taking this to the next step, then that seems like a reasonable approach. I like the idea that all strong processors are combined into one, if that is reasonable to accomplish.

We could take it to the next step. I was being more conservative, but if we want to go all in and combine them, that could be done quite easily. Initially, we'd just use the new format and loop through our regular expression patterns and output the appropriate element based on what pattern matches. If we wanted to in the future, we could even rewrite to functionally parse the patterns (if there was some advantage), but I see no need to completely rewrite everything. I think the current patterns are probably fine for now, we can just group them into one pattern step.

Okay, it seems like the best way to consolidate patterns, if we were to go that direction, would be to maybe take a more functional approach. Trigger off * or ** and then functionally parse any nested * and build up the strong or em tags with the appropriate nested strong and em tags. Additionally considering smart semantics if enabled. So if you were given something like **text *italic* text *italic* text**, we would just trigger off ** and then resolve any * until we hit ** at the end. We would return text italic text italic text so we didn't have to handle it in multiple passes.

I may give this a shot as it may make a lot more sense and probably clean things up as well.

I didn't notice this before, but it looks like we don't properly apply smart logic always to underscores: test___smart_logic__. See https://johnmacfarlane.net/babelmark2/?text=test___smart_logic__.

# ***strongem*** or ***em*strong**
EM_STRONG_RE = r'(\*|_)\1{2}(.+?)\1(.*?)\1{2}'

# ***strong**em*
STRONG_EM_RE = r'(\*|_)\1{2}(.+?)\1{2}(.*?)\1'

To fix this, we'd need even more patterns, extras for underscore that employ smart logic.

We may need to do this in a better way...or just add more patterns that might conflict with plugins.

I didn't notice this before, but it looks like we don't properly apply smart logic always to underscores: test___smart_logic__.

Interesting edge case. For the record, we only apply "smart logic" to single underscores, not double or triple. Presumably the thinking is that single underscores are common enough in plain text that we should special case them, but double and triple underscores are not common enough and we always treat them the same as asterisks. Of course, the specific edge case above is handled by a doubt/triple regex, so it doesn't follow the "smart" underscore logic. And in the end, I'm not too worried about it as it only behaves that way because there are double and triple underscores with it. The point is that we only use the "smart underscores" when we are only dealing with all single underscore text.

I can't say for certain if that was a conscious decision of an accident that it turned out that way. I suppose a way to determine if it was intended behavior would be to find a test for it. If no such test exists, then it was an unforeseen side effect. Either way, I'm okay with it staying the way it is.

I've actually worked out logistics and have implemented a prototype over in the BetterEm extension: https://github.com/facelessuser/pymdown-extensions/pull/516/files#diff-81791fadc501f2d0d876f760472c81b4R98.

Essentially, a similar inline processor will be used here: 1 for asterisks and 1 for underscores. All of the logic will be consolidated into a single processors for each.

Refactor em strong to consolidate code and fix issue #792

This issue was fixed by #805

Fix unit test regressions for Markdown rendering.

Navigation Menu

Repository navigation

Bold/Italic bug #792

9 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Bold/Italic bug #792

Description

Activity

waylan commented on Feb 27, 2019

facelessuser commented on Feb 27, 2019

facelessuser commented on Feb 27, 2019

facelessuser commented on Feb 27, 2019

waylan commented on Feb 27, 2019

Dave-ts commented on Feb 27, 2019

waylan commented on Feb 27, 2019

9 remaining items

waylan commented on Mar 4, 2019

waylan commented on Mar 4, 2019

facelessuser commented on Mar 4, 2019

facelessuser commented on Mar 4, 2019

waylan commented on Mar 4, 2019

facelessuser commented on Mar 4, 2019

facelessuser commented on Mar 9, 2019

facelessuser commented on Mar 9, 2019

waylan commented on Mar 9, 2019

facelessuser commented on Mar 12, 2019

facelessuser commented on Sep 4, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions