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

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

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

rowanc1
Copy link
Collaborator

@rowanc1 rowanc1 commented Apr 3, 2024

This code block now has indentation at the start. Previously it was trimmed.

```{code-block} c
:linenos: true
:lineno-start: 2
:emphasize-lines: 3
    for (int i = 0; i < 10; i++) {
        /* do something */
    }
```

Copy link
Collaborator

@fwkoch fwkoch left a comment

Choose a reason for hiding this comment

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

I think this change is fine: trim() on the directive content is problematic with leading spaces, using trimEnd() keeps leading spaces but makes the change slightly smaller since we are not entirely eliminating the trim.

My only suggestion is that the PR title and the changeset are misleading. This "trim end of line only" applies to all directives, not just code-block.

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...)

@rowanc1 rowanc1 changed the title 🔭 Trim end of line only in code-block directives 🔭 Trim end of line only in directives (important for code-blocks) Apr 3, 2024
@rowanc1 rowanc1 merged commit 6a57ab7 into main Apr 3, 2024
4 checks passed
@rowanc1 rowanc1 deleted the fix/code-block branch April 3, 2024 19:55
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.

2 participants