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

Dont require trailing : to enforce semantic headings #48

Closed
wants to merge 6 commits into from
Closed

Dont require trailing : to enforce semantic headings #48

wants to merge 6 commits into from

Conversation

benbalter
Copy link
Contributor

This pull request removes the requirement that emphasis-based headings must have trailing punctuation in order to be flagged as invalid to more aggressively enforce semantic headings. I can't think of when this would cause a false positive, if on a line by itself.

It seems strange to me, that the rule's documentation states that # test: would be valid, even though that would conflict with no-heading-punctuation.

@benbalter
Copy link
Contributor Author

One thing to note, I just ran npm run build-rules. Not sure why the docs got so mangled. Halp?

@wooorm
Copy link
Member

wooorm commented Feb 28, 2016

Thanks! Don’t worry about the build, it’s not very contributor friendly. You can run npm test, that does all the building and stuff!

@wooorm
Copy link
Member

wooorm commented Feb 28, 2016

It’s a while ago since I worked on it; I think I added the : because they were in this example from the source of initial rules.

Personally, I use strong emphasis currently in my API sections, e.g., this section from hastscript

### `h(selector?[, properties][, children])`

DSL for creating virtual [HAST][] trees.

**Parameters**:

*   `selector` (`string`, optional)
    — Simple CSS selector, e.g., tag names (`foo`), IDs (`#bar`)
    and classes (`.baz`) are supported,
    defaults to a `div` element.

...

**Returns**: [`Node`][hast-node] — A HAST node.

(maybe it’s not very “semantic” but it looks good)

...but I see now that your proposed change allows my usage because I use two nodes.

Meaning, I like this! Thanks!

@wooorm wooorm added the 🦋 type/enhancement This is great to have label Feb 28, 2016
@benbalter
Copy link
Contributor Author

You can run npm test, that does all the building and stuff!

Had some trouble getting things to run locally (my fault, now resolved). Man, those tests are brutal (in the best way possible). Now passing. 😄

Personally, I use strong emphasis currently in my API sections, e.g., this section from hastscript

I think that's fine, and added an explicit test via 952780b to ensure that remains valid (since the next node is a list item, not a paragraph).

@wooorm
Copy link
Member

wooorm commented Feb 28, 2016

Green! Looks good, @benbalter!

Thanks for taking the time to do a PR, and by the way, thanks for choosing remark-lint, hope you have a pleasent flight 😉

Yes! The tests are quite heavy. Great to see you figured it out though, hope they helped!

I’ll merge this later today, but don’t wait in #360 for it being released, I’d like to combine it with some more stuff if something comes up this week!

@wooorm wooorm closed this in d54441e Feb 29, 2016
@wooorm wooorm added ⛵️ status/released 🗄 area/interface This affects the public interface 🧒 semver/minor This is backwards-compatible change labels Aug 15, 2019
@wooorm wooorm added the 💪 phase/solved Post is done label Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

Successfully merging this pull request may close these issues.

2 participants