Skip to content

feat: support line wrapping for Binary in literal syntax#1455

Open
sxrzh wants to merge 3 commits into
jbeder:masterfrom
sxrzh:fix1287-wrap-new
Open

feat: support line wrapping for Binary in literal syntax#1455
sxrzh wants to merge 3 commits into
jbeder:masterfrom
sxrzh:fix1287-wrap-new

Conversation

@sxrzh

@sxrzh sxrzh commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Fix #1287.
Support line wrapping for Binary in literal syntax.
The line wrapping length is 80 by default, which means each line (indentation included) doesn't exceed 80 characters in literal Binary. This length could be manipulated through out << YAML::Wrap(len), and Wrap(0) means disabling the wrapping.

If current indentation exceeds the wrapping length (not 0), Utils::WriteLiteralBinary will return false without any exception, just like other functions in emitterutils.cpp.

@SGSSGene SGSSGene left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for this PR! this finally makes larger binary data possible!

Comment thread src/emitterutils.cpp Outdated
Comment on lines +550 to +563
if(wrap) {
if(wrap <= indent) return false;
wrap -= indent;
std::size_t point = wrap;
for(std::size_t i = 0; i < encoded.size(); i++) {
if(i == point) {
wrapped += '\n';
point += wrap;
}
wrapped += encoded[i];
}
}
else
wrapped = encoded;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor: add spaces after if and for. Also added curly brackets for the else case.

Suggested change
if(wrap) {
if(wrap <= indent) return false;
wrap -= indent;
std::size_t point = wrap;
for(std::size_t i = 0; i < encoded.size(); i++) {
if(i == point) {
wrapped += '\n';
point += wrap;
}
wrapped += encoded[i];
}
}
else
wrapped = encoded;
if (wrap) {
if (wrap <= indent) return false;
wrap -= indent;
std::size_t point = wrap;
for (std::size_t i = 0; i < encoded.size(); i++) {
if (i == point) {
wrapped += '\n';
point += wrap;
}
wrapped += encoded[i];
}
}
else {
wrapped = encoded;
}

Comment thread src/emitterutils.cpp Outdated
}
else
wrapped = encoded;
WriteLiteralString(out, wrapped.data(), wrapped.size(), indent);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is an oversight from the other PR. If I see correctly WriteLiteralString is always true, but I think we should pass the return value if one day someone changes WriteLiteralString to return something else.
(The return true; in the next line should be removed)

Suggested change
WriteLiteralString(out, wrapped.data(), wrapped.size(), indent);
return WriteLiteralString(out, wrapped.data(), wrapped.size(), indent);

Comment thread src/emitterutils.cpp Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Even though it is not directly related. Could you return the value from WriteSingleQuotedString instead of returning always true?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure!

@sxrzh sxrzh requested a review from SGSSGene July 3, 2026 08:39
@sxrzh

sxrzh commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Done. Thanks for your hard work.

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.

YAML::Literal does not work for YAML::Binary

2 participants