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

fix(table): tables with a preceding footnote are now rendered correctly #930

Closed
wants to merge 1 commit into from
Closed

fix(table): tables with a preceding footnote are now rendered correctly #930

wants to merge 1 commit into from

Conversation

shawnfunke
Copy link

fixes #929

@MarketingPip
Copy link

Hey @shawnfunke - I am not a contributor of this project etc. But I would suggest providing some tests to ensure this provides the proper fix.

In the meantime hopefully @tivie or @SyntaxRules can look at merging this PR.

pinging them both in hope they see this.

@shawnfunke
Copy link
Author

Hi @MarketingPip,

The tests are located in test/functional/makehtml/cases/features/tables/..., the
provided Markdown file is rendered and compared with the corresponding HTML
file. I can't think of any specific cases that aren't already tested through other
existing tests definitions, but I'm open to suggestions.

@MarketingPip
Copy link

@shawnfunke I will be honest I completely missed seeing your tests!

No ☕ when looking earlier!

Hopefully the repo maintainers see this - as it seems it has been requested. Tho it looks like they need some more support to close some existing issues (as they are getting out of hand)

ps; I don't know if this is up your alley but being on the regex side of things, (assuming it use's a regex match) or last match of character. But maybe you want to add a fix for this into another PR if willing.

Issues with link format / regex · Issue #951 · showdownjs/showdown

@shawnfunke
Copy link
Author

@MarketingPip, if this pull request gets merged I don't mind looking into
your issue further and proving a fix for it. But i'll wait for now since I don't
want to put the work in, while not knowing if it actually will get merged.

@MarketingPip
Copy link

@shawnfunke more than 100% understandable. Considering the maintainers have dismissed some previous pull requests for fixes that could have been applied previously. (and allowing someone else to end up re-writing those functions AGAIN) instead of applying a PR that could have already fixed it.

I don't want to spam the owners of this. But I tagged them in previous issues etc in hope it grabs some attention + possibly hopefully allow some other maintainers etc to close some previous issues & more etc. Before this project get's more cluttered than it already is!

Other than that, sorry to be a bothersome & jump in on your PR.

Tho I do hope this does help get a move on the maintainers for merging your PR. 👍

@shawnfunke
Copy link
Author

@MarketingPip, don't worry you're good, I don't mind at all. It'd be great
if third-party contributions would be accepted again.

@shawnfunke shawnfunke closed this by deleting the head repository Jan 3, 2023
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