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

Flow collections always wrap at >80 characters, regardless of lineWidth option #507

Closed
TheBITLINK opened this issue Dec 3, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@TheBITLINK
Copy link

Describe the bug
When using the flow collection style, collections always seem to wrap at around 80 characters, regardless of the lineWidth option:

blockCollection:
  aKey: hello
  anotherKey: world
  yetAnotherKey: "!!!"
listOfFlowCollection:
  - {
      aKey: hello,
      anotherKey: world,
      yetAnotherKey: "!!!",
      oneMoreKey: "!!!!!!!!"
    }
  - {
      aKey: hello,
      anotherKey: world,
      yetAnotherKey: "!!!",
      oneMoreKey: "!!!!!!!!"
    }
flowColTwo:
  - { aKey: hello, bKey: world, cKey: "!!!" }
  - { aKey: hello, bKey: world, cKey: "!!!" }
  - { aKey: hello, bKey: world, cKey: "!!!", dKey: "!" }

To Reproduce

  function serialize() {
    const doc = new YAML.Document({
      blockCollection: {
        aKey: 'hello',
        anotherKey: 'world',
        yetAnotherKey: '!!!'
      },
      listOfFlowCollection: [
        { aKey: 'hello', anotherKey: 'world', yetAnotherKey: '!!!', oneMoreKey: '!!!!!!!!'  },
        { aKey: 'hello', anotherKey: 'world', yetAnotherKey: '!!!', oneMoreKey: '!!!!!!!!'  },
      ],
      flowColTwo: [
        { aKey: 'hello', bKey: 'world', cKey: '!!!'  },
        { aKey: 'hello', bKey: 'world', cKey: '!!!'  },
        { aKey: 'hello', bKey: 'world', cKey: '!!!',  dKey: '!' },
      ]
    })
    YAML.visit(doc, {
      Pair(_, pair: any) {
        if (pair.key?.value === 'listOfFlowCollection' || pair.key?.value === 'flowColTwo')  {
          for (const i of pair.value.items) {
            i.flow = true
          }
        }
      }
    })
    return doc.toString({ lineWidth: 120 })
  }

Expected behaviour
Flow collections respect the lineWidth option and wrap at 120 characters instead of 80 (or not wrap at all if lineWidth is 0)

blockCollection:
  aKey: hello
  anotherKey: world
  yetAnotherKey: "!!!"
listOfFlowCollection:
  - { aKey: hello, anotherKey: world, yetAnotherKey: "!!!", oneMoreKey: "!!!!!!!!" }
  - { aKey: hello, anotherKey: world, yetAnotherKey: "!!!", oneMoreKey: "!!!!!!!!" }
flowColTwo:
  - { aKey: hello, bKey: world, cKey: "!!!" }
  - { aKey: hello, bKey: world, cKey: "!!!" }
  - { aKey: hello, bKey: world, cKey: "!!!", dKey: "!" }

Versions (please complete the following information):

  • Environment: Chrome 119.0.6045.159
  • yaml: 2.3.4

Additional context
The same issue manifests when using collectionStyle: 'flow' in the toString() options, so it's not specific to mixed style.

@TheBITLINK TheBITLINK added the bug Something isn't working label Dec 3, 2023
@eemeli
Copy link
Owner

eemeli commented Dec 4, 2023

lineWidth isn't the option controlling this, but maybe it should be?

This is effectively a duplicate of #288, and there's an open issue #421 about improving the ergonomics of the solution.

@TheBITLINK
Copy link
Author

TheBITLINK commented Dec 4, 2023

Oh, cool, didn't know there was an internal value controlling that, only saw lineWidth in the documentation so I thought that would control this (it would make sense given the description of the property in the docs).

Not sure if lineWidth should control this as well, semantically it would make a lot of sense, but maybe there are people who want to control string length and flow width separately and there's always the weird edge cases on code already in production.

At the very least though, they should both be available in the options for toString() and the docs clarified to explain the differences.

@ben519
Copy link
Sponsor

ben519 commented Dec 6, 2023

Funny enough, I just stumbled my way here too. Fully expected doc.toString({ lineWidth: 120 }) to set the line width after reading the docs.

EDIT: My mistake.. The word wrapping was working as I expected. However, my editor was still wrapping text which game me the impression that lineWidth: 9999 wasn't working properly. My bad!

@eemeli
Copy link
Owner

eemeli commented Dec 6, 2023

A PR making lineWidth be used for flow collections instead of the current undocumented approach would be welcome.

no92 added a commit to no92/yaml that referenced this issue Feb 12, 2024
no92 added a commit to no92/yaml that referenced this issue Feb 12, 2024
no92 added a commit to no92/yaml that referenced this issue Feb 12, 2024
no92 added a commit to no92/yaml that referenced this issue Feb 12, 2024
no92 added a commit to no92/yaml that referenced this issue Feb 13, 2024
eemeli pushed a commit that referenced this issue Feb 15, 2024
@eemeli
Copy link
Owner

eemeli commented Feb 24, 2024

Fixed by #522.

@eemeli eemeli closed this as completed Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants