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

Upgrade anchor-markdown-header to fix emoji-titled headers #232

Merged
merged 2 commits into from
Sep 22, 2022

Conversation

kevin-david
Copy link
Collaborator

@kevin-david kevin-david commented Aug 6, 2022

Add test for emoji-titled headers

This provides an example of the problem described in #191.

### Add launch.json to enable test debugging

For codespaces/VS Code. This was very helpful in diagnosing #191.

Based on approach taken in https://stackoverflow.com/a/39607924, though the port stuff wasn't necessary anymore.

Upgrade anchor-markdown-header to fix emoji-titled headers

Actually fixes #191

@kevin-david kevin-david changed the title Add [failing] test for emoji-titled headers (Draft) Add [failing] test for emoji-titled headers Aug 7, 2022
@kevin-david kevin-david marked this pull request as draft August 7, 2022 14:07
@kevin-david kevin-david changed the title (Draft) Add [failing] test for emoji-titled headers Add [failing] test for emoji-titled headers Aug 7, 2022
@kevin-david kevin-david force-pushed the kevin-david/debug branch 2 times, most recently from 345ac5f to 418cfcc Compare August 8, 2022 18:25
@thlorenz
Copy link
Owner

Hi great work @kevin-david on reproducing this in a test.
I'd love if you can take a stab at fixing this as well as I currently don't have bandwidth to address it.
Thanks.

@kevin-david
Copy link
Collaborator Author

kevin-david commented Sep 19, 2022

Hey @thlorenz this should be fixed by my previous change in thlorenz/anchor-markdown-header#45 - I'm actually using my private fork at the moment, but I would love to clean that up!

We just need a new release of https://www.npmjs.com/package/anchor-markdown-header with that change so I can consume it here, is that something you can do? Happy to update this PR to non-draft mode once that's done (with this test now passing)

@kevin-david kevin-david marked this pull request as ready for review September 21, 2022 17:58
@kevin-david
Copy link
Collaborator Author

@thlorenz ready to go now with https://github.com/thlorenz/anchor-markdown-header's 0.6.0 release!

@kevin-david kevin-david changed the title Add [failing] test for emoji-titled headers Upgrade anchor-markdown-header to fix emoji-titled headers Sep 21, 2022
@kevin-david kevin-david changed the title Upgrade anchor-markdown-header to fix emoji-titled headers Upgrade anchor-markdown-header to fix emoji-titled headers Sep 21, 2022
Copy link
Owner

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

Aside from the vscode config file LGTM, so please just remove and we can merge.
I'm making you contrib so you can do this yourself once you fix that nit.

.vscode/launch.json Outdated Show resolved Hide resolved
@kevin-david kevin-david merged commit bb82960 into thlorenz:master Sep 22, 2022
@kevin-david kevin-david deleted the kevin-david/debug branch September 22, 2022 15:51
@thlorenz
Copy link
Owner

Thanks, is this ready to be published to npm?

@kevin-david
Copy link
Collaborator Author

I think so, I've been testing this for a while with my fork and haven't seen any issues!

@thlorenz
Copy link
Owner

OK went out as v2.2.1. Thanks for your work!

@thlorenz
Copy link
Owner

Just for the record, I did publish but forgot to check in the version change or tag it.
So unfortunately there is no record of it in the github repo now (I don't have a local copy of it anymore either).

So we just need to be aware the next time we publish a version as npm version patch would again create v2.1.1 which we actually need to skip now.

Sorry about that.

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.

TOC headers with emoji create broken relative-links
2 participants