Strip trailing whitespace in descriptions to avoid messy formatting#51
Conversation
maarten-ic
left a comment
maarten-ic
left a comment
There was a problem hiding this comment.
One suggestion for improvement, but the changes look good to me!
| 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' |
There was a problem hiding this comment.
Perhaps make this a utility method instead of repeating in 4 places?
There was a problem hiding this comment.
Did that and it does look cleaner, but it's also more code. What do you think? Or are we bikeshedding too much already? 😄
There was a problem hiding this comment.
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 🙂
There was a problem hiding this comment.
Hmm, but now I can't unsee it. And I've realised that this should be in yatiml.Node...
There was a problem hiding this comment.
Okay, we need a new release and making a new yatiml release for one utility function is a bit silly. So I'm going to merge it like this, and I'll add it to yatiml later and update here, maybe when I finally convert yatiml to lower-case types.
2 participants
Addresses #38