search

AuthMe / ConfigMe

A simple configuration management library for any Java project!
MIT License
37 stars 14 forks source link

Add multi-line string support in configuration files #407

Closed gamerover98 closed 7 months ago

gamerover98 commented 8 months ago

Fix #405

The pull request introduces support for multi-line strings in configuration files. This is done by changing the scalar style in SnakeYamlNodeBuilderImpl.java if a value contains a newline character \n. A corresponding test was added in StringPropertyTest.java to verify the new functionality. A test file, multiple_lines.yml, was also added for test cases.

single-line-string: "A single row"
multi-line-string: |
  First row
  Second row
  Third row
ljacqu commented 8 months ago

Changing the scalar style based on its contents is a great solution—it's simple, consistent and works out well. Thanks!

ljacqu commented 7 months ago

I looked into the carriage return thing more—from what I understand, the YAML spec is fine with having \r in string blocks. For some reason, SnakeYAML refuses to use the block style when there is a \r. Specifically, in org.yaml.snakeyaml.emitter.Emitter#analyzeScalar it picks up \r as a "special character" (there's a boolean specialCharacters), because of which allowBlock is set to false in the analysis's result. I'm not sure why, and it might be a bug (or at least a potential "feature request" on SnakeYAML side to allow this).

I'm personally not interested in contacting SnakeYAML as carriage returns probably don't have much business being in config strings in the first place. I'll still do a few tests to see if carriage returns can easily sneak in from Windows files, but if I can rule that out, I would argue it's the developer's responsibility to manage this. There are more cases than just carriage returns where SnakeYAML decides the block style is not applicable, which I think is fine (in fact, sometimes I think string blocks are literally not possible per the spec for some contents).

ljacqu commented 7 months ago

I have the new changes on a branch here (I don't want to push to your master), so if there are no objections I'll merge this and then append my commit right after. My commit is at https://github.com/AuthMe/ConfigMe/commit/28b38e98228ada4350ee7ad334067f80168e98e0

ljacqu commented 7 months ago

I accidentally pushed to your master—sorry! I added one final test that ensures CRLF files are actually parsed as just LF. Thus, even in Windows environments, we should never end up with \r in any strings unless someone explicitly sets them. This is great news! :tada: Thanks again for initiating all of this :blush: