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.

Sign up for GitHub

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

Jump to bottom

🔭 Trim end of line only in directives (important for code-blocks) #1052

Merged
merged 2 commits into from
Apr 3, 2024
Merged

🔭 Trim end of line only in directives (important for code-blocks) #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7d0c82
🔭 Trim end of line only in code-block directives
rowanc1 Apr 3, 2024
da89ff6
Update .changeset/fuzzy-pots-think.md
rowanc1 Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/fuzzy-pots-think.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"markdown-it-myst": patch
"myst-parser": patch
---

Only trim end of line for myst-directives, not both the start and end of lines. This is important for keeping indentation in code blocks.
2 changes: 1 addition & 1 deletion packages/markdown-it-myst/src/directives.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function runDirectives(state: StateCore): boolean {
body.shift();
bodyOffset++;
}
const bodyString = body.join('\n').trim();
const bodyString = body.join('\n').trimEnd();
const directiveOpen = new state.Token('parsed_directive_open', '', 1);
directiveOpen.info = info;
directiveOpen.hidden = true;
Expand Down
28 changes: 28 additions & 0 deletions packages/myst-parser/tests/directives/code.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,34 @@ cases:
value: |-
# here is math
1+2
- title: code-block with indentation
markdown: |-
```{code-block} c
:linenos: true
:lineno-start: 2
:emphasize-lines: 3
for (int i = 0; i < 10; i++) {
/* do something */
}
```
mdast:
type: root
children:
- type: mystDirective
name: code-block
args: c
options:
emphasize-lines: 3
lineno-start: 2
linenos: true
value: " for (int i = 0; i < 10; i++) {\n /* do something */\n }"
children:
- type: code
lang: c
showLineNumbers: true
startingLineNumber: 2
emphasizeLines: [3]
value: " for (int i = 0; i < 10; i++) {\n /* do something */\n }"
- title: code-block with name language options parses
markdown: |-
```{code-block} python
Expand Down
6 changes: 4 additions & 2 deletions packages/myst-parser/tests/myst.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ function stripPositions(tree: GenericParent) {
return tree;
}

function removeTightAnnotations(tree: GenericParent) {
function fixMystDirectives(tree: GenericParent) {
selectAll('mystDirective', tree).forEach((node) => {
// Node markdown is trimmed
(node as any).value = (node as any).value?.trim();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not totally clear why we need this, without digging into it...? Is there, like, some test-case round-tripping that does not work any more?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are some of these defined in the myst-spec!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, ok - so the trim() originally came from respecting those tests, now we no longer totally align with them. (Well, there are already a few ways we don't align with them, this is just one more, not a big deal...)

// These are added on afterwards and we aren't taking them into account in myst spec
delete (node as any).tight;
});
Expand Down Expand Up @@ -115,7 +117,7 @@ describe('Testing myst --> mdast conversions', () => {
test.each(mystCases)('%s', (_, { myst, mdast }) => {
if (myst) {
const mdastString = yaml.dump(mdast);
const newAst = removeTightAnnotations(
const newAst = fixMystDirectives(
replaceMystCommentNodes(
stripPositions(
mystParse(myst, {
Expand Down
Loading