Skip to content

Conversation

@jrandall
Copy link

@jrandall jrandall commented Nov 4, 2021

Address half of #146 by adding support to generate YAML with comments. Parsing YAML with comments into the internal form (and making round-tripping possible) would be the next step, but we had an immediate need on the generation side so we went ahead and implemented that, and hope that it might be useful to others as well.

Because we don't have a parser yet, the tests for comment generation are somewhat verbose (involving literal expected values). Once the parser is implemented, these tests can be replaced by round-trip tests as are used for much of the other functionality.


#[derive(Clone, PartialEq, PartialOrd, Debug, Eq, Ord, Hash)]
pub struct Comments {
/// A before comment is a vector of strinsg representing the text from full

Choose a reason for hiding this comment

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

Suggested change
/// A before comment is a vector of strinsg representing the text from full
/// A before comment is a vector of strings representing the text from full

/// Alias, not fully supported yet.
Alias(usize),
/// Yaml node with block or line comments
CommentedYaml(self::CommentedYaml),

Choose a reason for hiding this comment

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

this is a breaking change, so I guess, it'll require bumping minor version (0.4.5 to 0.5.0), maybe? But I'm not the maintainer :)

Copy link

@andrewgazelka andrewgazelka Jan 17, 2022

Choose a reason for hiding this comment

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

Since this is a breaking change, it might also potentially be worth adding non_exhaustic to the enum, so in the future, changes like this won't be breaking.

#[derive(Clone, PartialEq, PartialOrd, Debug, Eq, Ord, Hash)]
pub struct CommentedYaml(pub Box<Yaml>, pub Comments);

#[derive(Clone, PartialEq, PartialOrd, Debug, Eq, Ord, Hash)]
Copy link

@andrewgazelka andrewgazelka Jan 17, 2022

Choose a reason for hiding this comment

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

Suggested change
#[derive(Clone, PartialEq, PartialOrd, Debug, Eq, Ord, Hash)]
#[derive(Clone, PartialEq, PartialOrd, Debug, Eq, Ord, Hash, Default)]

It would be nice to have a default implementation given there are so many fields.

@Nepo26
Copy link

Nepo26 commented May 12, 2025

@jrandall Did you look into it?

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.

4 participants