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

[ENH] allow using of mkdoc admonitions #1711

Merged
merged 8 commits into from
Mar 15, 2024

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Feb 23, 2024

  • add a script to remove mkdoc admonitions
  • run it during pdf build
  • add minimal testing to help debugging if needed
  • add example admonition to highlight the bids examples repo

@Remi-Gau Remi-Gau added exclude-from-changelog This item will not feature in the automatically generated changelog user experience labels Feb 23, 2024
@@ -679,33 +681,38 @@ def process_macros(duplicated_src_dir_path):

duplicated_src_dir_path = "src_copy/src"

# Step 1: make a copy of the src directory in the current directory
# make a copy of the src directory in the current directory
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the step ordering... Because we will have to change it everytime steps are added.

Copy link

codecov bot commented Feb 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.93%. Comparing base (c9e4779) to head (f9e699b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1711   +/-   ##
=======================================
  Coverage   87.93%   87.93%           
=======================================
  Files          16       16           
  Lines        1351     1351           
=======================================
  Hits         1188     1188           
  Misses        163      163           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Remi-Gau
Copy link
Collaborator Author

@Remi-Gau
Copy link
Collaborator Author

@tsalo @sappelhoff @effigies
This is a draft but as far as I can tell this would allow using admonitions without breaking the PDF build.

Can you have a quick look and tell me if you see something I am missing?

@Remi-Gau
Copy link
Collaborator Author

the remark linting will be annoying though

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Great! Thanks a lot @Remi-Gau, I think this looks promising and can be deployed.

the remark linting will be annoying though

an easy fix would be to switch off that particular linting check, which I think would be fine.

@Remi-Gau
Copy link
Collaborator Author

an easy fix

OK so it seems that remark-lint-code-block-style is set to fenced by remark-preset-lint-markdown-style-guide

So need to find how to unset that.

@tsalo
Copy link
Member

tsalo commented Feb 23, 2024

This looks awesome!

@Remi-Gau Remi-Gau marked this pull request as ready for review February 23, 2024 16:57
@Remi-Gau Remi-Gau requested a review from ezemikulan as a code owner February 23, 2024 16:57
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that this is not run automatically in CI.

@Remi-Gau
Copy link
Collaborator Author

PDF looks fine to me.

Note that some links are not rendered in the pdf but this is not related to this PR.

For example the link at the top of the genetic section:

[`UK biobank`](https://github.com/bids-standard/bids-examples/tree/master/genetics_ukbb)

I think this is because of the back tick in the square brackets.

@Remi-Gau Remi-Gau changed the title [ENH][WIP] allow handling of mkdoc admonitions [ENH] allow using of mkdoc admonitions Feb 23, 2024
@Remi-Gau
Copy link
Collaborator Author

For contributors: should we preface our admonitions with some markdown comments the way we do it the macros so that users are not confused about what they are ?

@tsalo
Copy link
Member

tsalo commented Feb 23, 2024

I don't think that's necessary. Macros aren't exactly common, but admonitions are. We can have that info in a style guide if we want though.

@Remi-Gau
Copy link
Collaborator Author

I don't think that's necessary. Macros aren't exactly common, but admonitions are. We can have that info in a style guide if we want though.

OK so something to add in the contributing.md

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Feb 24, 2024

Actually before merging.

  • handle admonition that start with ??? and ???+
  • possibly simplify mkdoc macro that adds admonition below the filename templates

@sappelhoff sappelhoff requested review from effigies and ericearl March 11, 2024 15:24
Copy link
Collaborator

@ericearl ericearl left a comment

Choose a reason for hiding this comment

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

lgtm!

@Remi-Gau Remi-Gau merged commit 2a01f59 into bids-standard:master Mar 15, 2024
26 checks passed
@Remi-Gau Remi-Gau deleted the admonition branch March 15, 2024 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants