Skip to content
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

YAMLGenerator: multiline string will always be quoted when line has trailing space #366

Open
njank opened this issue Dec 22, 2022 · 12 comments
Labels
2.18 Fix or feature targeted at 2.18 release has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue pr-needed Feature request for which PR likely needed (no active development but idea is workable) yaml Issue related to YAML format backend

Comments

@njank
Copy link
Contributor

njank commented Dec 22, 2022

I am generating a yml file with the below settings:

ObjectMapper mapper = new ObjectMapper(new YAMLFactory()
	.enable(YAMLGenerator.Feature.LITERAL_BLOCK_STYLE)
	.enable(YAMLGenerator.Feature.MINIMIZE_QUOTES)
);

This will write multiline strings as following:

    - replaceInFiles:
        replacement: test
                     123

However if i put a space between "test" and the following line break it will quote the string as if MINIMIZE_QUOTES was disabled:

    - replaceInFiles:
        replacement: "test \n\n123\n"
@cowtowncoder
Copy link
Member

The first thing is that you would also need to include actual code -- what String value are you trying to write?
Description does not give enough details to know what is going on.

Due to complexity of YAML rules for handling white-space, it may be tricky to determine whether behavior makes sense or not tho.
My guess is that quoting would be considered necessary to retain part of whitespace included in String value to write (if any).

@cowtowncoder
Copy link
Member

Also this is wrong repo, moving.

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-dataformat-xml Dec 22, 2022
@cowtowncoder cowtowncoder added the yaml Issue related to YAML format backend label Dec 22, 2022
@nylend95
Copy link

nylend95 commented Sep 11, 2024

I have the same issue.
Mapper setup:

    private static final ObjectMapper MAPPER = JsonMapper.builder(new YAMLFactory()
            .enable(YAMLGenerator.Feature.MINIMIZE_QUOTES)
            .enable(YAMLGenerator.Feature.INDENT_ARRAYS_WITH_INDICATOR)
            .enable(YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS)
            .disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER))
            .enable(JsonReadFeature.ALLOW_SINGLE_QUOTES)
            .build().findAndRegisterModules();

Example YAML:

someKey:
  otherKey.properties: |-
    # comment with trailing whitespace  
    property1=my-value

This will be converted to

someKey:
  otherKey.properties: "# comment with trailing whitespace \nproperty1=my-value"

See example Maven project:
jackson-yaml.zip

Build and run

  1. Download zip
  2. Unzip
  3. Run mvn clean package
  4. Run java -jar target/jackson-yaml-issue-1-jar-with-dependencies.jar

@cowtowncoder
Copy link
Member

@nylend95 And this is with 2.17.2?

@cowtowncoder cowtowncoder added the 2.18 Fix or feature targeted at 2.18 release label Sep 11, 2024
@nylend95
Copy link

Yes! Forgot to include that in the description, but as you can see in my example, I have configured 2.17.2 in the pom.xml.

@cowtowncoder cowtowncoder added the pr-needed Feature request for which PR likely needed (no active development but idea is workable) label Sep 12, 2024
@cowtowncoder
Copy link
Member

On issue itself, just to make sure I understand what is expected: it is to keep "use literal block style" usage, even if "minimize quotes" is desired?

.enable(YAMLGenerator.Feature.LITERAL_BLOCK_STYLE)
	.enable(YAMLGenerator.Feature.MINIMIZE_QUOTES)

@nylend95 I don't see the first setting in your example tho?

I am not sure if above is doable, might be? (or whether it's easy to do if so).
I don't have much time to focus on this either, so contributions (PR) would be the best way to move forward (I do take time to review contributions).

But one thing that is often helpful is to contribute PR for failing test (under yaml/src/test/java/...../failing/) even without fix: having easily runnable reproduction is often surprisingly helpful wrt actual fixing. Plus ensures fix actually works.

@nylend95
Copy link

Ah, yes! Our use cases is to keep block style where suitable, and no quotes on single lines if possible. I have yet to dig into the code to see how/if these settings are triggering different behavior.

With both of these features enabled, I still see the same behavior as before.

JsonMapper.builder(new YAMLFactory()
                .enable(YAMLGenerator.Feature.LITERAL_BLOCK_STYLE)
                .enable(YAMLGenerator.Feature.MINIMIZE_QUOTES)
                .enable(YAMLGenerator.Feature.INDENT_ARRAYS_WITH_INDICATOR)
                .enable(YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS)
                .disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER))
                .enable(JsonReadFeature.ALLOW_SINGLE_QUOTES)
                .build().findAndRegisterModules();

The block style string is converted to a single line when at least one of the lines end with a whitespace. I don't know if it should be considered a bug or not, but I find it a bit weird behavior that a change in a trailing space on a single line should trigger that the field will be converted to a single line.

If you/FasterXML decides that this behavior is wrong, I can get a PR up for the failing test and then we can take it from there.

@cowtowncoder
Copy link
Member

From description this does appear to be a flaw. One thing to note is that the low-level encoding is by SnakeYAML so it may be down to question of interaction between Jackson YAML module and SnakeYAML generator.
But hopefully it's just a simple logic flaw (missing handling of some combination of settings, most likely) that can be corrected.

And yes, we could use help here with failing test!

@nylend95
Copy link

I can see if I can reproduce this with just SnakeYAML. If I can, I guess it would be better to open an issue towards SnakeYAML instead?

@cowtowncoder
Copy link
Member

Yes, if it is something reproducible with SnakeYAML. If it turns out it is due to way Jackson uses SnakeYAML then needs to be fixed here of course.

njank added a commit to njank/jackson-dataformats-text that referenced this issue Oct 3, 2024
@njank
Copy link
Contributor Author

njank commented Oct 3, 2024

But one thing that is often helpful is to contribute PR for failing test (under yaml/src/test/java/...../failing/) even without fix: having easily runnable reproduction is often surprisingly helpful wrt actual fixing. Plus ensures fix actually works.

I have created PR #501 based on the successful test function SimpleGenerationTest.testLiteralBlockStyle.

njank added a commit to njank/jackson-dataformats-text that referenced this issue Oct 4, 2024
@cowtowncoder cowtowncoder added the has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue label Oct 4, 2024
@cowtowncoder
Copy link
Member

Had a quick look at code, and style Jackson module passes to SnakeYAML is STYLE_LITERAL for both cases, and it is SnakeYAML that then overrides output mode.
I suspect this is because use of block style would lose information (trailing space at the last row). But at any case its SnakeYAML that decides, not Jackson module, so not sure what we could do here.

Leaving open if anyone has any ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.18 Fix or feature targeted at 2.18 release has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue pr-needed Feature request for which PR likely needed (no active development but idea is workable) yaml Issue related to YAML format backend
Projects
None yet
Development

No branches or pull requests

3 participants