-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
[YAML] Add support to parse and dump comments #22516
Comments
I am not convinced that we should add this feature. The parser is already complex enough. So 👎 from me. |
Definitely not something that we want to implement. |
I understand your point, but still, this could be useful feature in some cases. At the moment we are storing configuration settings in YAML files, where i would like to keep the comments added above the key-value pairs. I would like to have these comments added when we are writing out the bean, so no parser changes needed. I would like to have something like an extension point where i can add my custom property scanner that checks if the field has an annotation like this: private String field = "value";` And the dump should look like this: field: value` Or please let me know if it is already implemented. I think this comment annotation would be used frequently by other users also. |
Why not just make it optionally extendable for those who need it? Like Twig filters, that anyone can add and don't have to be in the core. |
Even "just" adding an extension point increases the complexity of the parser. Feel free to give it a try, but I doubt this will be possible without too much unneeded complexity. |
I don't know parser nor its performance in detail, so I don't think my attempt would be useful or effective :) But adding extension point != increase complexity. I often add such point to decrease complexity and lower responsbility. |
Well, I don't see how that could be done in this specific case without adding too much complexity. So yes, the only thing I can offer is that someone gives it a try and proves us wrong here. |
I understand the complexity involved but I still think a lot of people would greatly benefit from this feature. Comments aren't just a "nice to have" in YAML files. Given that people use YAML specifically because its human-readable, I think you should treat the human-focused elements (comments) with the same priority as everything else. I maintain BLT, which has some automation tools to help people manage large sets of YAML files. Right now we have to force users to choose between taking advantage of that all of that helpful tooling or keeping their comments. It's a pretty terrible choice. |
Also if anyone is looking to handle comments without the help of Symfony, here are some other tools (I can't comment on their quality): |
@danepowell Here comes my feedback to the tools you mentioned:
I also checked if https://github.com/mustangostang/spyc can write comments back - not at this time. |
Since the decision to not support this is three years old now, maybe reality has changed enough to reconsider this:
|
"As Symfony’s YAML flavor is basically unparseable with non-Symfony parsers" -> That's new to me. |
Various parsers won’t accept e.g. the
|
But that's valid YAML AFAIK. But if you're not using this (I won't use it myself), the YAML parser/dumper IS YAML compliant and should be parseable in other languages (I'm doing that in Go without any problems). I don't really get the goal of your comment to be honest. |
I feel we are applying different definitions for what we consider compatible but it probably doesn’t matter terribly much. The goal of my original comment was to reopen the discussion on should Symfony’s YAML parser have a comment preserving mode and my argument for why it should have one is because it would enable a whole class of tooling to exist that likely won’t exist if the parser does not support it. |
Let's say there would be such comment preserving mode, what would be the expected result for this yaml document: # comment 1
root: # comment 2
foo: 1
# comment 3
bar: 2 # comment 4
baz: 3 How would you represent this structure after parsing? |
@gharlan The parsed YML would need to be represented as object like DOM. $yml->getValue(); // ['root' => ['foo' => 1, 'bar' => 2, 'baz' => 3]]
$yml->root->bar->getValue(); // 2
$yml->root->bar->getComment(); // 'comment 3'
$yml->root->bar->getInlineComment(); // 'comment 4' |
One more closely tied task is preserving original formatting of Yaml file. That's really needed for code generators to merge generated Yaml with existing one. |
I've been thinking about this. The following steps would be automated and called from php with exec:
|
I totally agree. We are not talking about a custom feature request, this is a valid native "type". So refusing to even have basic functionality built in, is a bit weird. In other words, using automation tools, this becomes unusable if you don't want to unnecessarily annoy your users. It's a shame that something as big as symfony can't offer basic functionality while on the other hand has stuff like custom tags that aren't necessary for providing basic functionality. |
Obviously nobody cared enough about this to come up with a PR proving that adding this would be possible without making the parser even more complex than it is right now. |
Parsing into an AST is what is necessary to be able to have comments preserved. This is a totally different kind of parsing than parsing the data themselves. This is not a feature we can "simply" add. It involves a full rewrite of the component. Also, I would not consider "parse into an AST" the basic functionality either. |
AST parsing might also have a performance penalty. This may impact projects that use this component for loading configuration run-time. |
Indeed. Producing an AST first and then resolving the data from the AST will indeed likely be slower (and consume more memory) than directly parsing the data. |
As is stated in the official docs, the YAML component is not able to parse and dump comments. I need to parse and dump comments in a helper command I'm doing to add Doctrine mappings automatically for new bundles into the
config.yml
file for Symfony Standard distribution (so I need to keep the comments).The text was updated successfully, but these errors were encountered: