In my last article, I documented how I was working hard to get to the end of the unprioritized items in my issues list and on to the prioritized parts of the list. This article details the work that I am doing to make that push happen.
Now a week or so removed from the New Year’s Holiday break, it was refreshing to know that I was still able to resolve a healthy collection of items from the issues list. Sure, it was not the same volume as during the holiday, but it was still a healthy volume of issues to resolve.
And I definitely felt that I was getting closer to the end of the initial phase of the PyMarkdown project. I was pretty sure that I would not be able to resolve every item from the unprioritized section of the issues list this week, but I was confident that it was going to happen in the week after. I just had to maintain a good velicity of resolving issues, and I would get there soon!
What Is the Audience for This Article?¶
While detailed more eloquently in this article, my goal for this technical article is to focus on the reasoning behind my solutions, rather that the solutions themselves. For a full record of the solutions presented in this article, please go to this project’s GitHub repository and consult the commits between 11 Jan 2021 and 17 Jan 2021.
Doing Some Simple Cleanup¶
As always, I try and ease into the project work for the week with something simple. This
week was no different. And while this change might seem to be of no consequence to others,
to me it was a question of readability and searchability. The task? To replace newline
characters in the source code with
ParserHelper.newline_character and to replace the
colon character (
:) separating the extra data field for the Markdown tokens with
While it might not be obvious to people that are not dealing with the source code, the presence of those two characters in an unescaped and unreferenced form had caused me a lot of frustration. I guess if I had to pin down a cause why that happened, it was because a newline character and a single colon character are easy to miss when scanning through source code. I wanted something that I could easily find, and not something that I would easily miss, as had happened numerous times during debugging sessions. For me, the cost of this change was easily outweighed by the benefit for readability.
It was not a change that fixed an issue or enabled a scenario tests, but I was able to notice the difference almost instantly. It just helped me see the code better, and that was its goal!
Upgrading To Python 3.8¶
Having started a year ago, Python 3.7 was the stable release of Python at the time the project started. At that time, Python 3.8 had just been released on 14 October 2019 1, a couple of weeks before I started working on the code. With the first commit of the source code on 22 Nov 2019, it just seemed like a safer bet to stay with version 3.7 until the bugs were worked out of the then brand new Python release.
Just over a year later, with Python 3.9 released on 15 Oct 20201, it felt like a
good time to upgrade one minor version with the same reasoning in mind. However, there was
also another reason: performance. Having started to explore the performance of the project
on sample Markdown pages, I found that the project’s parser was taking a long time to
parse a simple Markdown file. Using
I knew that the number one problem that I had with performance was the way I used log
statements. Without going too far into my research2, in order solve the
performance issue while keeping the extra functionality that helped me debug more
efficiently, I would soon need to write my own logging wrapper. To do this properly, my
research on logging
indicated that I would need to use the
stacklevel argument to allow the wrapper to
function while logging the location where the wrapper’s log statement was called from.
The catch? It was introduced in Python 3.8.
With a good reason to update and a good, stable version of Python 3.8 to update to, I proceeded with the upgrade with relatively few issues. The main issue that I hit was that I needed to ensure that I uninstalled Python 3.7 in the project, install Python 3.8 on my system (including all environment variables), and then install Python 3.8 in the project. Once that was done, the only other issue that I had was with the Black Python formatter. In that case, I needed to examine the graph for that package and make sure that I installed the correct version of the dependent library in the project.
After that small headache, which took minutes to solve, everything was working fine and continues to work fine.
More Fun With Link Reference Definitions¶
The first thing to mention about the next task is that while the commit was performed earlier than the 3.8 Upgrade commit, chronologically this task came after the upgrade task. The reason that this is relevant is that the project uses the default settings for Black, and either those defaults or the algorithm implementing the line folding changed after the upgrade was completed. Why is this relevant? While the commit itself looks like it has a lot of changes, many of those changes occurred in folding the lines according to upgraded settings. And as I was focused on the Link Reference Definitions, I did not notice those formatting changes until after I had made a number of changes. It was just easier to commit them together at that point than to pull them apart.
Other than that noise, there were three new scenario tests introduced, testing
Link Reference Definition elements broken up across container block boundaries. The
first test added, test function
test_link_reference_definitions_extra_01, was created
with a single Unordered List element character, followed by a valid Link Reference
Definition spread over two lines, with the second line not being indented:
- [foo]: /url
The second test, function
test_link_reference_definitions_extra_02, used the same
format, but used a Block Quote element prefix instead of an Unordered List prefix.
Finally, to provide a touchstone, I added function
test_link_reference_definitions_extra_02 that has both lines of the Link Reference
Definition preceded by the Block Quote element prefix. While it was a duplicate
test, I felt it was a good reminder of how a test with both lines worked, and thus
it was a good reference test.
Now, according to the specification, the List element continues if the next line starts with enough whitespace to maintain the indent or if it is a continuation of a Paragraph within the list. As the Link Reference Definition is not a Paragraph when parsed the first time, the second line terminates the list, and causes the Link Reference Definition to be requeued and parsed as a normal Paragraph on the second parse through. Or so I thought.
Github Flavored Markdown vs CommonMark¶
While both specification are usually in sync with each other, sometimes the GFM Specification and the reference CommonMark implementation CommonMark.Js called from Babelmark 2 differ in small implementation details. I had experimented with the Block Quote scenario test for three or so hours before I submitted a question to the CommonMark forums asking if I had misunderstood something in the specification.
The answer that came back was a supportive answer, but at the same time, an honest answer. The approach that CommonMark’s reference parser had taken was to parse the lines as the start of an Unordered List followed by a Paragraph block. Only after that Paragraph block had been parsed, with the paragraph continuation kicking in, does the parser look for a Link Reference Definition at the start of that Paragraph.
Is this 100% according to the GFM specification? No. But does it make sense for the CommonMark team to do this? I would argue yes. Getting Link Reference Definitions correct in the PyMarkdown parser has continued to be a pain to this day. Based on my limited experience, while Link Reference Definitions can be spread over multiple lines, there are very few cases where that is done in “real life”. From a correctness viewpoint, if I had to guess on the percentages, I believe I would estimate that their approach correctly parses 99.5% of the Link Reference Definition elements, with only some “weird” multiline Link Reference Definition scenarios not being parsed.
But that left me with a decision. What was the correct thing to do for the PyMarkdown parser?
Which To Choose?¶
After thinking about this overnight, I decided that the best approach for the project was
to align with the CommonMark reference implementation, while also discussing the
the GFM Specification with the CommonMark team in the forums. By making that choice,
I had confidence that I would have something to compare against for correctness that was
both concrete and
measurable. It either would parse properly against
commonmark.js 0.29.2 and be
considered correct or it would
not and be considered a parsing failure. As for any differences, I could clearly note
them in documentation somewhere, and talk about them on the forums with the owners of
CommonMark and the GFM specification, trying to improve both. It was not a perfect answer,
things rarely are perfect.
With that decision in hand, I marked all three new tests as skipped before starting to
work on the Block Quote functions. Based on what I was seeing in the test failures,
everything looked fine in the HTML output, except that the output was missing an entry
in the Block Quote for the parsed line. Taking a wild guess, I determined that I needed
to introduce something in the
handle_block_quote_block function to ensure that the number
of lines in the Block Quote element were correct.
Surprisingly, that was the fix that was needed. No changes in the Markdown transformer
were needed, and no changes in the consistency check was needed. Not sure what I had
going on that evening, I decided to mark the List Block version of the scenario test,
test_link_reference_definitions_extra_01, as disabled. Cleaning up
the code and committing it to the repository. It was a good place to stop while I
figured out what was going on in the evening.
Getting Back To It¶
With the plans for that evening falling through, I found that I had a couple of hours
free that evening. Not wanting to let them go to waste, I decided to see if I could tackle
test_link_reference_definitions_extra_01 function that I elected not to get working
in the previous section. To ensure I was moving in the correct direction, I added
extra variations of the test that included one and two spaces before the second half
of the Link Reference Definition element, as well as one with each half of the Link
Reference Definition in its own List Item.
As I have had numerous problems with List Blocks in the past, I expected to expend a
lot of effort to clean these cases up, but only a little bit of effort was required.
Specifically, the only change that was needed with in the
__check_for_list_closures function. Like the previous section and the
Link Reference Definition element that spanned Block Quote element levels, the CommonMark
reference implementation was treating the Link Reference Definition text as a Paragraph
before detecting the Link Reference Definition element itself. To replicate this
behavior, I needed to modify the
__check_for_list_closures function to keep the
‘paragraph’ open if it was parsing a Link Reference Definition.
Making those modification, I was able to get the main function,
test_link_reference_definitions_extra_01, working, as well as the sibling functions
test_link_reference_definitions_extra_01b. This meant that a Link Reference
Definition split over a List Block and the end of that block with various amounts of
indentation was working properly. However, function
test_link_reference_definitions_extra_01c, where I split the Link Reference Definition
over two List Items was not working at all. With my time used up in the evening, I
marked it as skipped, cleaned it up, committed it, and went to sleep for the night.
And More Link Reference Definitions¶
As I was looking at the content of the existing scenario tests, I noticed that I did not have a couple of tests that had simple multi-line Raw HTML elements and multi-line Code Span elements in links. Basically, I wanted to take example 644 for Raw HTML elements:
foo <!-- this is a comment - with hyphen -->
and example 345 for Code Spans:
`` foo bar baz ``
and place them within both Inline Links and Reference Links. I was hoping that this was a simple test, but I was not sure.
Adding all four tests, (two for Inline Links and two for Reference Links), I was pleasantly surprised that all four tests passed without any changes. While I am aware that I am getting closer and closer to the initial release of the project, I still find that I expect things to fail as a default setting. As I am usually an optimistic person, my only explanation for that failure viewpoint is one of writing too many automation tests in my career. When I note something down in the issues list, I believe that I feel that most of those items are going to be things that I forgot to cover, not things that I wish to ensure are covered.
Regardless, I need to figure that out and work on it a bit. I do have a lot of confidence in the PyMarkdown project and its accuracy, and I need to project that more.
Fixing Disabled Tests¶
After a good night’s sleep and a good day’s worth of work under my belt, I settled down
in the evening to work on the next issue: enabling test functions
test_block_quotes_extra_02ad. The good news was
that the HTML transformer and the Markdown transformer were both working properly.
The bad news was that the consistency checks were failing for all these tests.
It took me a bit to get going that evening, but when I did, it was obvious to me that the
problem was that the consistency checks were not recognizing the active Block Quote
element. Following along in the
verify_line_and_column_numbers method, it became
obvious that the code:
if container_block_stack and container_block_stack[-1].is_block_quote_start:
was not evaluating to
true for the four test functions that I was evaluating. But
how to fix them?
It took me a while to realize that the reason that the condition was not evaluating to
True was that the Block Quote token was not always the last token on that list.
When I read the variable named
container_block_stack, in my head I was parsing it as
“the stack for container Block Quotes”, not “the stack for container blocks”. Once
I figured that out, the answer became obvious. I created a new function
find_last_block_quote_on_stack that went back in the stack until it found to last
Block Quote token and returned it. From there, I replaced any occurrence of
last_block_quote_token. Therefore, the code
from above became:
last_block_quote_token = find_last_block_quote_on_stack( container_block_stack ) if last_block_quote_token:
I ran the tests, and after clearing up a couple of typing mistakes, the tests all worked properly, and they were now passing!
A Quick Fix… I Hope!¶
Looking at the end of the uncategorized section of the issues list, there was one item that I felt confident that I could quickly deal with:
- 634, but forcing an html block
To start working on this item, I made six copies of test function
The first copy was in a List Block element, the next three copies were in various forms
of a Block Quote element, the fifth copy was within a SetExt Heading element, and the
last copy was within an Atx Heading element. The hard part of each of these tests was
that I needed to make sure I was generating an HTML Block token and not a Raw HTML
token. That took extra care but did not slow me down that much.
Like a handful of other issues like this that I have fixed, the answer to this
one leapt out at me as soon as I looked through the log files. When the next line
was examined to figure out if the Block Quote element should be continued, the
check_for_lazy_handling was allowing it to continue. The only issue here was that
it was an HTML block, a leaf block type that does not have any continuation logic in
the specification. Having noticed that, it was easy to change the following code:
if ( parser_state.token_stack[-1].is_code_block or (is_leaf_block_start) ):
if ( parser_state.token_stack[-1].is_code_block or parser_state.token_stack[-1].is_html_block or (is_leaf_block_start) ):
thereby fixing the issue. Running the scenario tests again, the tests were indeed fixed without needed any other changes.
That Week’s Big Thing¶
Wrapping up the work for that week, I wanted to make another dent in the issues list, so I decided to at least get the tests set up for the following item:
- links with & and \ with inner link to mine - see __collect_text_from_blocks
The base concept of this item was simple: create a group of tests to verify how inline elements were represented when placed within a Link element. To make sure that I was doing a good scenario test, I made the choice to use a Reference Link element. By doing this, I would be testing the link label normalization code and the representation code at the same time.
test_reference_links_extra_03x, I created a Link element with a link
label that contained a backslash in the link label:
[bar\\foo]: /uri [bar\\foo]
From there, I then created a copy of that test that encapsulated that link label within another link label:
[xx[bar\\foo]yy](/uri) [bar\\foo]: /uri1
and finally, I created a copy of that test, changing the Link element to an Image element:
![xx[bar\\foo]yy](/uri) [bar\\foo]: /uri1
Then, after a bit of thinking, I decided there was only one combination I was missing, so I added that:
[xx![bar\\foo]yy](/uri) [bar\\foo]: /uri1
That being done, I then repeated that process with
© for character
entity references, code spans, emphasis, autolinks, raw HTML, and hard line breaks.
By the time I was done, I had added 40 scenario tests to cover all these cases.
Starting to execute the scenario tests, all the tests that just dealt with Link elements were passing without any changes. The Image elements, they were a different story. The failures seemed to stare back at me, standing in the way of me writing the article for that week. It was just time to start debugging and figuring things out.
After a bit of debugging, I seemed to notice that the test failures seemed to be in three
separate groups of issues. The first group of issues was that the
__collect_text_from_blocks function used to grab the existing tokens and render them
as text was not complete. But that was not the entire issue, but I felt that there
was too much “noise” in the way for me to see the issue clearly. Resolving to reduce
the noise in the issue, I started working on the main part of the issue. In the cases
where the inline element was on its own inside the parent
Link element, the Code Span element, the Raw HTML element, and the Autolink element
were not representing their elements properly. A bit of exploration and debugging took
care of that. With that noise out of the way, I was able to see the other part of that
issue better, and added the condition:
if not is_inside_of_link:
at the start of each of those handlers. The noise that I had experience was simply that in cases where a possible Link element was within another Link element’s link label, each of the changed elements just needed to emit nothing to the collection. Running the tests again, it verified that observations, but I also saw something else.
Only after getting rid of those issues was I able to see that the
function was not handling Emphasis start and end elements properly, failing an assert
near the end of that function. That fix was easy, adding four lines to the
elif inline_blocks[ind + 1].is_inline_emphasis: pass elif inline_blocks[ind + 1].is_inline_emphasis_end: pass
With both of those issues dealt with, the couple of failures that remained were easy ones.
Looking at the HTML output, there were a number of
/a character sequences in the HTML
output. Being the signature for replacement references, I quickly change of the code for
the Text element from:
image_alt_text += inline_blocks[ind + 1].token_text
image_alt_text += ParserHelper.resolve_references_from_text( inline_blocks[ind + 1].token_text )
Beware Of Special Cases¶
Running the tests again, all the tests were passing except for those that dealt with the
& sequence. All the tests dealing with the
© sequence were working fine,
so I had to think quickly to figure out what the problem might be. Because I am used to
looking at processed HTML code, I initially did not see any problem with a
in the HTML output. It looked right.
Then it hit me. The problem was not with the HTML output, it was with the processing of
the Markdown input. In the cases that I had problems with, the desired HTML output had
&amp; which did not look right until I thought about it. Whereas the
character sequence is interpreted as a named charactery entity and replaced with the
© symbol, the sequence
& was not being interpreted in the same way. The
desired HTML output was correct! The initial
& from the sequence was being replaced
with the sequence
& to ensure it was displayed properly, removing any chance of
it being interpreted as a named character entity.
Making a quick decision, I looked at the
InlineHelper class and noticed the
append_text function used to ensure that such strings were properly interpreted.
Taking a quick look at the imports for
LinkHelper, I thought there
was a chance of a circular reference occurring. Given that observation, I decided to
make a copy of the
append_text function in the
LinkHelper class to get around the
possibility of the circular reference.
Finally, after a couple of hours of work, all 40 tests were passing. It was a bit of a trek to get there, but it was worth it!
As I was starting to write the article, I also started to look into what it would take
to remove the duplicate of the
append_text function in the
LinkHelper class. I had
introduced the clone into the
LinkHelper class to avoid any issues with referencing
InlineHelper class from the
LinkHelper class. It was as I was starting my
research into this task that I discovered something that I had previously missed. The
LinkHelper class was already referencing the
After a bit of “how did I miss that?”, I replaced the call to
InlineHelper.append_text and everything worked fine. Removing the instance of the
append_text function from the
LinkHelper class, I ran the complete suite of scenario
tests again, and everything worked fine.
What Was My Experience So Far?¶
Looking back at the work I did during the week, it was hard not to get jazzed about the progress. I started off by doing a simple fix that made the source code more readable, enhancing my ability to read the source code. Then I upgraded the base Python version to 3.8, knowing that it would allow me to write the wrapper I wanted around Python’s logging functions. Add to that the coverage and testing I was able to add and verify for Link elements, Image elements, and Link Reference Definitions, and it was really good work!
At some point I noticed the number of scenario tests that I execute with each change. At approximately 2000 tests, I am confident that I am hitting a very large degree of all scenarios for Markdown transformation, not just the “Golden” scenarios, and that was also a boost to my confidence. While I can expect things that I note as an issue to not work, I also need to make sure I appreciate what is working. Having a solid set of tests like that is what allows me to refactor with confidence that I am not negatively impact the code.
Refactor? Sigh. One of the things I know I am looking forward to is looking at the refactor tasks in the prioritized sections and getting a couple of them underway. The fact that they are present in those sections is a good reminder to me that I can always learn how to do things better, and how to make the project more maintainable. And that is always something I can look forward to!
What is Next?¶
With a solid amount of work done this week, I am hoping to be able to clear out the uncategorized section of the issues list in the next week. Will I make it? Stay tuned!
So what do you think? Did I miss something? Is any part unclear? Leave your comments below.