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 bullet list style options #229

Merged
merged 1 commit into from Jul 12, 2022
Merged

Conversation

edwardloveall
Copy link
Contributor

@edwardloveall edwardloveall commented Jul 7, 2022

Hi! I know this repository practices Optimistic Merging but I've put this in draft-form for for a few reasons:

  1. It's not done. It still need to fill out documentation for new options and enums.
  2. I'm not sure if this is a feature you want to support and I didn't want to do 1 without knowing
  3. This is my second ever rust contribution and I'd love some feedback as I'm sure there are things that could be improved.

Original change description below:


This change adds a new option --list-style so users can choose which
character all bulleted lists use for their marker. The commonmark
spec
supports three options for bullet markers: -, *, + which is
also now the case here. Previously, when comrak (or cmark-gfm) rendered
markdown, it would pick - which is now the default.

This also adds the ability to optionally provide custom ComrakOptions
when testing. If the defaults are fine, you can pass None.

@gjtorikian
Copy link
Collaborator

Would this even need to be an option? I should think that the parser ought to accept any of those types at all times:

* wow
- wow?
+ wow!

src/parser/mod.rs Outdated Show resolved Hide resolved
src/parser/mod.rs Show resolved Hide resolved
@edwardloveall
Copy link
Contributor Author

edwardloveall commented Jul 7, 2022

@gjtorikian it does accept all those options, but it doesn't output all of them. Today it only outputs -. Unless I'm misunderstanding the question?

$ echo "+ foo" | comrak --to commonmark && echo "* foo" | comrak --to commonmark && echo "- foo" | comrak --to commonmark
- foo
- foo
- foo

Copy link
Owner

@kivikakk kivikakk left a comment

Choose a reason for hiding this comment

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

It's looking great! Once the docs are done (and seriously, don't spend much time on it, just straightforward one-liners are enough), this will be good to merge!

I'd love to give you some more feedback from a Rust perspective, but there's nothing to say from my point of view; looks clean!

src/main.rs Outdated Show resolved Hide resolved
@@ -1272,6 +1300,7 @@ fn exercise_full_api<'a>() {
width: 123456,
unsafe_: false,
escape: false,
list_style: ::ListStyleType::Dash,
Copy link
Owner

Choose a reason for hiding this comment

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

Note to self for next release: this will need a correspondingly large bump because we're modifying the public API.

(🤞 I remember.)

src/parser/mod.rs Outdated Show resolved Hide resolved
src/parser/mod.rs Show resolved Hide resolved
This change adds a new option `--list-style` so users can choose which
character all bulleted lists use for their marker. The [commonmark
spec] supports three options for bullet markers: `-`, `*`, `+` which is
also now the case here. Previously, when comrak (or cmark-gfm) rendered
markdown, it would pick `-` which is now the default.

This also adds the ability to optionally provide custom ComrakOptions
when testing. If the defaults are fine, you can pass `None`.

[commonmark spec]: https://spec.commonmark.org/0.30/#bullet-list-marker
@edwardloveall
Copy link
Contributor Author

Once the docs are done (and seriously, don't spend much time on it

Too late. I learned how to write rust docs that interlink! 😄

@edwardloveall edwardloveall marked this pull request as ready for review July 8, 2022 01:05
@gjtorikian
Copy link
Collaborator

but it doesn't output all of them.

Ignore me, please! I misread the PR. 😞

@kivikakk
Copy link
Owner

Sorry for the delay — have been dealing with some kind of respiratory illness 🙃 This is excellent. Thank you. ❤️

@kivikakk kivikakk merged commit e0de513 into kivikakk:main Jul 12, 2022
@kivikakk
Copy link
Owner

This is now part of 0.14.0: https://github.com/kivikakk/comrak/releases/tag/0.14.0!

@edwardloveall edwardloveall deleted the el-bullet-style branch July 13, 2022 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants