Skip to content

Strip trailing whitespace in descriptions to avoid messy formatting#51

Open
LourensVeen wants to merge 2 commits into
developfrom
issue_38_funny_formatting_trailing_whitespace
Open

Strip trailing whitespace in descriptions to avoid messy formatting#51
LourensVeen wants to merge 2 commits into
developfrom
issue_38_funny_formatting_trailing_whitespace

Conversation

@LourensVeen
Copy link
Copy Markdown
Contributor

Addresses #38

@LourensVeen LourensVeen requested a review from maarten-ic May 23, 2026 18:18
@LourensVeen LourensVeen self-assigned this May 23, 2026
Copy link
Copy Markdown
Collaborator

@maarten-ic maarten-ic left a comment

Choose a reason for hiding this comment

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

One suggestion for improvement, but the changes look good to me!

Comment thread ymmsl/v0_2/component.py Outdated
Comment on lines +136 to +140
ynode = cast(yaml.ScalarNode, descr.yaml_node)
ynode.style = '|'
if not ynode.value.endswith('\n'):
ynode.value += '\n'
# remove trailing whitespace to ensure PyYAML actually uses block style
ynode.value = '\n'.join([
line.rstrip() for line in ynode.value.split('\n')]) + '\n'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perhaps make this a utility method instead of repeating in 4 places?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did that and it does look cleaner, but it's also more code. What do you think? Or are we bikeshedding too much already? 😄

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, I meant to create a method:

def set_block_style(ynode: yaml.ScalarNode) -> None:
    ynode.style = "|"
    # remove trailing whitespace to ensure PyYAML actually uses block style
    ynode.value = '\n'.join([
        line.rstrip() for line in ynode.value.split('\n')]) + '\n'

But fine to keep it like this.

Or are we bikeshedding too much already? 😄

Not really worth spending much more time on it indeed 🙂

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, but now I can't unsee it. And I've realised that this should be in yatiml.Node...

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