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

Repairing setext heading support, certain uses of backticks after GitHub API format changes #145

Merged
merged 10 commits into from Oct 1, 2023

Conversation

coreysciuto-toast
Copy link
Contributor

@coreysciuto-toast coreysciuto-toast commented Sep 22, 2023

This must be applied on top of #144 (Thanks @jkburges!) but when I tried that PR locally, I realized the setext headings (https://github.github.com/gfm/#setext-heading) are broken.

What I'm getting is

*Title 2*
=========

renders as * [](#title-2) when it should be something like * [<em>Title 2</em>](#title-2) prior to Github's reformatting.

I later realized that some usages for backticks (which generate as the <code> tag) get put on a newline, breaking the parsing.

@coreysciuto-toast coreysciuto-toast marked this pull request as ready for review September 23, 2023 00:32
@coreysciuto-toast
Copy link
Contributor Author

I think this is working now. It includes #144 out of necessity.

@coreysciuto-toast coreysciuto-toast changed the title Adding some tests for setext heading support Repairing setext heading support, certain uses of backticks after GitHub API format changes Sep 23, 2023
@coreysciuto-toast
Copy link
Contributor Author

@ekalinin I think I took this about as far as I can - all of the remote tests fail. I copied one of the non-english README's over into a local test to show that it still can pass, but the curl/wget command that pulls by URL now returns a single-line payload that begins with a ton of script, etc before it gets to the body, which, again, is on one line now.

@ekalinin
Copy link
Owner

Thanks for the patch!
I've merged #144, could you please fix conflicts?

@coreysciuto-toast
Copy link
Contributor Author

Hey @ekalinin I've fixed the conflicts - hopefully the number of tests failing is similar to in pull/144. Should actually be one fewer I believe.

@coreysciuto-toast
Copy link
Contributor Author

coreysciuto-toast commented Sep 29, 2023

@ekalinin so yeah, the remote tests are all still failing but all the local ones are passing, including the new ones. The remote ones might be some effort to get them working again.

@ekalinin ekalinin merged commit 656b340 into ekalinin:master Oct 1, 2023
1 check failed
@coreysciuto-toast coreysciuto-toast deleted the setext-header-tests branch October 2, 2023 13:31
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.

None yet

3 participants