Ensure lineWidth=0 is respected for block literal vs folded

[muff1nman]

No description provided.

[eemeli]

Is this missing a description accidentally or on purpose? I can't immediately see what issue this is solving, and some discussion would be rather welcome.

[muff1nman]

I believe the title adequately describes the issue at hand but let me put the comparison from the test inline here so we can discuss:

Previous behavior with lineWidth=0

lorem: >
  Lorem ipsum dolor sit amet, consectetur adipiscing elit. In laoreet massa eros, dignissim aliquam nunc elementum sit amet. Mauris pulvinar nunc eget ante sodales viverra. Vivamus quis convallis sapien, ut auctor magna. Cras volutpat erat eu lacus luctus facilisis. Aenean sapien leo, auctor sed tincidunt at, scelerisque a urna. Nunc ullamcorper, libero non mollis aliquet, nulla diam lobortis neque, ac rutrum dui nibh iaculis lectus. Aenean lobortis interdum arcu eget sollicitudin.
  
  Duis quam enim, ultricies a enim non, tincidunt lobortis ipsum. Mauris condimentum ultrices eros rutrum euismod. Fusce et mi eget quam dapibus blandit. Maecenas sodales tempor euismod. Phasellus vulputate purus felis, eleifend ullamcorper tortor semper sit amet. Sed nunc quam, iaculis et neque sit amet, consequat egestas lectus. Aenean dapibus lorem sed accumsan porttitor. Curabitur eu magna congue, mattis urna ac, lacinia eros. In in iaculis justo, nec faucibus enim. Fusce id viverra purus, nec ultricies mi. Aliquam eu risus risus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Suspendisse potenti.

Desired behavior (and should match lineWidth > actual content i.e. 1000)

lorem: |
  Lorem ipsum dolor sit amet, consectetur adipiscing elit. In laoreet massa eros, dignissim aliquam nunc elementum sit amet. Mauris pulvinar nunc eget ante sodales viverra. Vivamus quis convallis sapien, ut auctor magna. Cras volutpat erat eu lacus luctus facilisis. Aenean sapien leo, auctor sed tincidunt at, scelerisque a urna. Nunc ullamcorper, libero non mollis aliquet, nulla diam lobortis neque, ac rutrum dui nibh iaculis lectus. Aenean lobortis interdum arcu eget sollicitudin.
  
  Duis quam enim, ultricies a enim non, tincidunt lobortis ipsum. Mauris condimentum ultrices eros rutrum euismod. Fusce et mi eget quam dapibus blandit. Maecenas sodales tempor euismod. Phasellus vulputate purus felis, eleifend ullamcorper tortor semper sit amet. Sed nunc quam, iaculis et neque sit amet, consequat egestas lectus. Aenean dapibus lorem sed accumsan porttitor. Curabitur eu magna congue, mattis urna ac, lacinia eros. In in iaculis justo, nec faucibus enim. Fusce id viverra purus, nec ultricies mi. Aliquam eu risus risus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Suspendisse potenti.

Not how the content is nearly identical except the block literal is wrong in the first leading to missing newlines when actually parsed.

[eemeli]

So the folding is producing incorrect output in some circumstances? Could you show an example with JS code that I can execute somewhere that shows this invalid case producing broken output without the fixes you're looking to apply here?

[muff1nman]

Okay after rereading some sections of the yaml spec, I misspoke when I said "wrong". Rather the behavior here is just not inline with the desired behavior of what should happen when lineWidth is 0 (or negative). As seen in the following, when lineWidth is zero, currently you still get a folded block, rather than the desired literal block:

var YAML = require("yaml");
var src = {
    lorem: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. In laoreet massa eros, dignissim aliquam nunc elementum sit amet. Mauris pulvinar nunc eget ante sodales viverra. Vivamus quis convallis sapien, ut auctor magna. \nCras volutpat erat eu lacus luctus facilisis. Aenean sapien leo, auctor sed tincidunt at, scelerisque a urna. Nunc ullamcorper, libero non mollis aliquet, nulla diam lobortis neque, ac rutrum dui nibh iaculis lectus. Aenean lobortis interdum arcu eget sollicitudin.\n\nDuis quam enim, ultricies a enim non, tincidunt lobortis ipsum. Mauris condimentum ultrices eros rutrum euismod. Fusce et mi eget quam dapibus blandit. Maecenas sodales tempor euismod. Phasellus vulputate purus felis, eleifend ullamcorper tortor semper sit amet. Sed nunc quam, iaculis et neque sit amet, consequat egestas lectus. Aenean dapibus lorem sed accumsan porttitor. Curabitur eu magna congue, mattis urna ac, lacinia eros. In in iaculis justo, nec faucibus enim. Fusce id viverra purus, nec ultricies mi. Aliquam eu risus risus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Suspendisse potenti. \n'
};
YAML.scalarOptions.str.fold.lineWidth = 0;
var asYaml = YAML.stringify(src);
console.log(asYaml);

leads to:

lorem: >
  Lorem ipsum dolor sit amet, consectetur adipiscing elit. In laoreet massa eros, dignissim aliquam nunc elementum sit amet. Mauris pulvinar nunc eget ante sodales viverra. Vivamus quis convallis sapien, ut auctor magna. 

  Cras volutpat erat eu lacus luctus facilisis. Aenean sapien leo, auctor sed tincidunt at, scelerisque a urna. Nunc ullamcorper, libero non mollis aliquet, nulla diam lobortis neque, ac rutrum dui nibh iaculis lectus. Aenean lobortis interdum arcu eget sollicitudin.


  Duis quam enim, ultricies a enim non, tincidunt lobortis ipsum. Mauris condimentum ultrices eros rutrum euismod. Fusce et mi eget quam dapibus blandit. Maecenas sodales tempor euismod. Phasellus vulputate purus felis, eleifend ullamcorper tortor semper sit amet. Sed nunc quam, iaculis et neque sit amet, consequat egestas lectus. Aenean dapibus lorem sed accumsan porttitor. Curabitur eu magna congue, mattis urna ac, lacinia eros. In in iaculis justo, nec faucibus enim. Fusce id viverra purus, nec ultricies mi. Aliquam eu risus risus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Suspendisse potenti. 

[muff1nman]

I've edited the title and will edit the commit message to match

[eemeli]

Ah, now I get it. Yeah, okay, it does make sense to default to literal rather than folded block scalars when lineWidth has been set to 0.

But there shouldn't be a need to involve the folding code into this logic. Why not just add a condition here to the type === Type.BLOCK_LITERAL check? Would make sense to first extract lineWidth to a local variable, as it's then referred to more than once.

yaml/src/stringify/stringifyString.ts

Lines 159 to 164 in a51a079

	const literal =
	type === Type.BLOCK_FOLDED
	? false
	: type === Type.BLOCK_LITERAL
	? true
	: !lineLengthOverLimit(value, strOptions.fold.lineWidth - indent.length)

[muff1nman]

Sure I can see how the folding code should be separate, but I was trying to keep the lineWidth 0/negative logic in one place. I also thought that the lineLengthOverLimit function was a better spot to nest this logic as it is in line with the functions name/purpose and the double ternary already seemed complicated enough.

[muff1nman]

I'd be fine with reverting the foldFlowLines changes and replicating the same logic to the lineLengthOverLimit function. Does that sound reasonable?

[eemeli]

Yeah, that sounds like a good plan. Also note that I finally got this code ported to TypeScript just now; hopefully that won't make it difficult for you to apply these changes to the .ts version of the file.

[eemeli]

Thank you!