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

Add support for underline style headings #73

Merged
merged 11 commits into from
May 31, 2023
Merged

Conversation

wilbowma
Copy link
Contributor

Fixes #72

Comes with lots of options:
image

The defaults retain the old behaviour.

This is my first TypeScript in a while and my first Obsidian plugin so let me know if I did something stupid.

@dvcrn
Copy link
Owner

dvcrn commented May 30, 2023

wow that was quick! Thanks a lot for the PR, let me try to get this reviewed today

@wilbowma
Copy link
Contributor Author

Had an hour to spare and your code was easy to follow :)

@dvcrn
Copy link
Owner

dvcrn commented May 30, 2023

The format is currently breaking, could you run yarn format to apply the prettier presets to all files?

@wilbowma
Copy link
Contributor Author

Fixed

@dvcrn dvcrn self-requested a review May 30, 2023 03:36
main.ts Show resolved Hide resolved
Copy link
Owner

@dvcrn dvcrn left a comment

Choose a reason for hiding this comment

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

When changing underline string from === to something else, and 'replace heading style' is enabled, should this replace === to the new string?

My assumption without reading the code was that it will force the heading style to be whatever is set in the settings, but currently it doesn't change the underline

main.ts Outdated Show resolved Hide resolved
@dvcrn
Copy link
Owner

dvcrn commented May 30, 2023

Left a few small comments, besides that it looks good to me 👍

@wilbowma
Copy link
Contributor Author

Think I fixed them all.

@dvcrn
Copy link
Owner

dvcrn commented May 31, 2023

Tested and working great! I'll go ahead and merge this

There's still the comment regarding fileLines[i + 1].match(/^=+$/) !== null. I have a feeling this breaks when the underline is set to something else than =. Maybe not the original intent of this PR for someone to use ___ for example, but since it's possible I reckon people will do that

@dvcrn dvcrn merged commit ce50ed4 into dvcrn:master May 31, 2023
@dvcrn
Copy link
Owner

dvcrn commented May 31, 2023

@wilbowma
Copy link
Contributor Author

wilbowma commented Jun 1, 2023

If someone uses ___ as the underline, that's not a Heading 1 by the markdown spec, so it shouldn't be read as the heading.

It might be worth adding some validation to the settings to ensure only a sequence of =s is used.

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.

Feature request: support alternative Title\n ==== heading syntax
2 participants