-
-
Notifications
You must be signed in to change notification settings - Fork 74
YAML comments support #410
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
base: trunk
Are you sure you want to change the base?
Conversation
|
Thanks for this contribution! I would love to have full support for reading comments back from configuration files, but I think this could be a decent stopgap until that work is complete. |
|
I wasn't aware that comments are also supposed to be read back. I might change the way the comments are read back for map keys a bit, but the general concept is there. Edit: made the changes |
|
Any update on this? |
|
no |
Sorry, should've asked differently: "What's stopping this from getting merged?" |
|
time |
This comment was marked as spam.
This comment was marked as spam.
format/yaml/src/main/java/org/spongepowered/configurate/yaml/YamlConstructor.java
Outdated
Show resolved
Hide resolved
| @Override | ||
| protected Object constructObjectNoCheck(final Node yamlNode) { | ||
| //noinspection DataFlowIssue guarenteed NonNull by getSingleData, which load(Reader) uses | ||
| final CommentedConfigurationNode node = CommentedConfigurationNode.root(this.options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really not a fan of the fact that by creating a new root node here, we effectively construct two nodes per node when loading in a file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're then mainly talking about the mapping part right? When looking at the differences between a root node and a .node node it looks like only the path and parent differs.
We could add a method that works essentially like attachIfNecessary / parentEnsureAttached, but allows you to set the parent & path as well.
This comment was marked as spam.
This comment was marked as spam.
format/yaml/src/main/java/org/spongepowered/configurate/yaml/YamlConfigurationLoader.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
|
Is it currently possible to disable a new line being created before every comment? When defining comments using the @ Comment annotation, the generated file has an empty line before the comment itself. For example: @ConfigSerializable
public class MainConfig {
@Comment("Fruit")
String string = "Apple";
@Comment("Number")
int integer = 25
}will end up looking like this:
# Fruit
string: Apple
# Number
integer: 25Other than that, this PR appears to be fully functional for my use case. |
This adds support for YAML comments.
Let me know if you want to see changes.