Skip to content

Conversation

facelessuser
Copy link
Collaborator

The table extension should have escaped pipes in order to adhere to the
PHP Markdown extra implementation. In order to properly add escaped
pipes, a bug relating to persisting appended escapes also had to be
patched. Reference: #526

The table extension should have escaped pipes in order to adhere to the
PHP Markdown extra implementation.  In order to properly add escaped
pipes, a bug relating to persisting appended escapes also had to be
patched. Reference: #526
@facelessuser
Copy link
Collaborator Author

I'm going to need to adjust border striping to account for escaped chars and the header test. Let me rebase and finish it proper.


def extendMarkdown(self, md, md_globals):
""" Add an instance of TableProcessor to BlockParser. """
if '|' not in md.ESCAPED_CHARS:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me wonder if ESCAPED_CHARS should be a set. Then it will always only contain one of each item and extensions don't need to check before appending. Just an idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea. That would be even better :).

@waylan
Copy link
Member

waylan commented Jan 18, 2017

@facelessuser there is no need to close a PR. Just add a comment that it is not ready. You can just rebase locally and then do a 'force push' to the same remote branch and GitHub will update the PR properly.

@waylan
Copy link
Member

waylan commented Jan 18, 2017

Oh, I just noticed you committed to your master branch. If you use a separate unique branch for a PR, then rebasing etc, becomes easier.

@facelessuser
Copy link
Collaborator Author

Oh, I just noticed you committed to your master branch. If you use a separate unique branch for a PR, then rebasing etc, becomes easier.

Yeah, I need to get out of that habit.

@facelessuser
Copy link
Collaborator Author

I have it working. I'll get the pull in this evening.

I'll also incorporate changing the escaped list to a set...are we worried about breakage on the change to a set (append vs add)? If you aren't, I have no problem. I don't know who actually uses the escaped char list. I know I do, but I'll prepare for it before the next Python Markdown release.

@facelessuser
Copy link
Collaborator Author

If we care about preventing breakage on the migration from list to set (at least in the 2.6 series), we could just derive from set and add the append/extend options as that is probably all anyone would should ever use. If we don't care about breakage in this area, then it doesn't really matter. We could also just stick to a list for 2.6.

For 3.0 though, I think a set should be used.

I'll add the set if I don't hear otherwise tonight when I create a new pull.

@waylan
Copy link
Member

waylan commented Jan 18, 2017

are we worried about breakage on the change to a set (append vs add)?

I forgot about that. Let's leave it a list for now. I agree, a set makes sense for 3.0.

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