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

Custom ids #144

Merged
merged 9 commits into from
Nov 15, 2023
Merged

Custom ids #144

merged 9 commits into from
Nov 15, 2023

Conversation

Sky9x
Copy link
Contributor

@Sky9x Sky9x commented Oct 24, 2023

  • You can now set custom CSS ids for admonishment blocks with the id field.
  • You can now customize the default CSS id prefix (default is "admonition-").

Feel free to nitpick everything (doc wording etc)

- You can now set custom CSS ids for admonishment blocks with the `id` field.
- You can now customize the default CSS id prefix (default is `"admonition-"`).
src/resolve.rs Outdated Show resolved Hide resolved
@tommilligan
Copy link
Owner

Hi - just commenting briefly to say thanks for the contribution! I'm mid review, got interrupted by some Life, but hope to pick up in the next week. On board with the idea though 👍

Sky9x and others added 5 commits November 15, 2023 21:57
- You can now set custom CSS ids for admonishment blocks with the `id` field.
- You can now customize the default CSS id prefix (default is `"admonition-"`).
Copy link
Owner

@tommilligan tommilligan left a comment

Choose a reason for hiding this comment

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

This is excellent work, thank you very much!

I made changes while I was reviewing. I've rebased and pushed this to a new branch sky9-review (as I can't update Sky9x:main). You can see the review changes here: main...sky9-review

Mostly this was just moving code around to make types match up, after my comments. If you're happy with the changes I've made, please update this PR to match and we can get it merged. Also happy to chat more about any of the changes if you have further thoughts.

book/src/reference.md Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/render.rs Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
src/resolve.rs Show resolved Hide resolved
@Sky9x
Copy link
Contributor Author

Sky9x commented Nov 15, 2023

strange that you cant edit my branch as i have "allow edits by maintainers" enabled but 🤷

resolved merge conflicts with upstream/main, accepted all your changed, and added my final nitpicks

im done unless you have anything else otherwise its good to go

@tommilligan tommilligan merged commit c3207e4 into tommilligan:main Nov 15, 2023
5 checks passed
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

2 participants