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

[Migration tooling] Admonitions #106

Closed
bmorelli25 opened this issue Dec 17, 2024 · 5 comments
Closed

[Migration tooling] Admonitions #106

bmorelli25 opened this issue Dec 17, 2024 · 5 comments
Labels
authoring Relates to our markdown parser

Comments

@bmorelli25
Copy link
Member

Summary

  • Supported admonitions in Asciidoc: NOTE, TIP, IMPORTANT, CAUTION, WARNING
  • Supported admonitions in V3: attention, caution, note, tip

Some context in #3.

To do

Either map existing admonitions to new admonitions, or refine the list of supported admonitions.

Conversion output

```{admonition} '#{title}'
:class: important

#{content}
```
@bmorelli25 bmorelli25 added the authoring Relates to our markdown parser label Dec 17, 2024
@Mpdreamz
Copy link
Member

We explicitly don't/won't support MYST syntax which directly influences HTML.

So we never use :class: to set the admonition type or influence any other directive for that matter.

We also explicitly don't support the open {admonition} and require the type to be specified as part of the directive name

```{note}
```

```{caution}
```

```{tip}
```

```{attention}
```

Are the supported admonitions, adding support for others is relatively straightforward if needed.

@bmorelli25
Copy link
Member Author

Currently, admonitions are collapsed by default. We need to change the default behavior to be non-collapsible and open by default.

@Mpdreamz
Copy link
Member

Eek, that's a regression I introduced yesterday. Now the absence of :open: is parsed correctly again @bmorelli25

@bmorelli25
Copy link
Member Author

Supported admonitions should be Note, Warning, Important, and Tip. We currently support note, caution, tip, and attention. Note and Tip are okay as-is. Caution (yellow) should be renamed to Warning. Attention (red) should be renamed to Important.

Mpdreamz pushed a commit that referenced this issue Jan 5, 2025
### Summary

Per
#106 (comment),
supported admonition types should be `Note`, `Warning`, `Important`, and
`Tip`. This list was determined after analyzing supported admonitions in
our current Asciidoc system and cross-referencing with our style guide.

This PR removes support for `Caution` and `Attention` and replaces them
with support for `Warning` and `Important`. It also adds a missing
`AdmonitionUnsupportedTest`.

I've updated #3 to reflect these supported admonition types as that
issue is referenced in the error logs.

### Results

Tested locally with:
```
dotnet publish "src/docs-builder/docs-builder.csproj" -c Release -o .artifacts/publish \
    --self-contained true /p:PublishTrimmed=true /p:PublishSingleFile=false /p:PublishAot=true -a arm64

./.artifacts/publish/docs-builder serve
```
<img width="625" alt="Screenshot 2025-01-03 at 10 25 37 PM"
src="https://github.com/user-attachments/assets/52fd3419-194f-4903-b368-724d6389e28c"
/>
<img width="632" alt="Screenshot 2025-01-03 at 10 25 32 PM"
src="https://github.com/user-attachments/assets/7419fff1-6d4f-4177-9c8e-7a836a9591ed"
/>
@bmorelli25
Copy link
Member Author

All bugs are fixed and admonitions look good in migration script 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authoring Relates to our markdown parser
Projects
None yet
Development

No branches or pull requests

2 participants