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

The json linter has broken wrapping behaviour #38007

Open
imayhaveborkedit opened this issue Feb 14, 2020 · 2 comments
Open

The json linter has broken wrapping behaviour #38007

imayhaveborkedit opened this issue Feb 14, 2020 · 2 comments
Labels
<Bug> This needs to be fixed [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Code: Tooling Tooling that is not part of the main game but is part of the repo. [JSON] Changes (can be) made in JSON (S2 - Confirmed) Bug that's been confirmed to exist

Comments

@imayhaveborkedit
Copy link

imayhaveborkedit commented Feb 14, 2020

When formatting collections, the linter properly wraps the first item at >120 characters. However, for the subsequent items, it wraps them based on the total length including the indent at >118 characters. The implication of this is that the wrapping changes based on depth and is totally inconsistent.

Steps To Reproduce

Given this rather extreme example formatted with the web linter:

Click to expand
[
  {
    "item1": [
      [ "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "12" ],
      [
        "1234567890",
        "1234567890",
        "1234567890",
        "1234567890",
        "1234567890",
        "1234567890",
        "1234567890",
        "1234567890",
        "12"
      ],
      [ "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "12345678" ]
    ],
    "item2": [
      [
        [
          [
            [
              [
                [
                  [
                    [
                      [
                        [
                          [
                            [
                              [
                                [ "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "12" ],
                                [
                                  "1234567890",
                                  "1234567890",
                                  "1234567890",
                                  "1234567890",
                                  "1234567890",
                                  "1234567890",
                                  "1234567890",
                                  "1234567890",
                                  "12"
                                ],
                                [ "1234567890", "1234567890", "1234567890", "1234567890", "1234567890", "1234567890" ]
                              ]
                            ]
                          ]
                        ]
                      ]
                    ]
                  ]
                ]
              ]
            ]
          ]
        ]
      ]
    ]
  }
]
Note how the first array is exactly 120 from bracket, the second item is the exact same as the first but wrapped, and the third is exactly 118 characters from the start of the line to end bracket.

Expected behavior

The linter should consistently wrap every item at >120.

Screenshots

N/A

Versions and configuration

N/A

Additional context

N/A

@snipercup
Copy link
Contributor

I also reproduced this issue. It shows up in the "conditional_names" key and the "snippet_category" key.

@kevingranade
Copy link
Member

Ok I see what's happening, but fixing it is going to be a pain.
When writing out a collection with wrapping, we write the opening separator ( [ or { ), a newline, and then the indent. Then the format function is called on the first element, which measures itself starting at the cursor, which is at the end of the indention.
Then each subsequent collection entry writes itself out, starting with the separator ( , ) and the newline, then the indention.

I think I need to add a bit of state, "need indent" much like "need separator" that will tell an element to write itself out starting with indention, and remove indention writing from start_array() and start_object(). To fix up the separator bit, I think we can get away with just checking the "need separator" flag and adjusting our line wrap arithmetic.

@int-ua int-ua added [C++] Changes (can be) made in C++. Previously named `Code` Code: Tooling Tooling that is not part of the main game but is part of the repo. [JSON] Changes (can be) made in JSON (S2 - Confirmed) Bug that's been confirmed to exist labels Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Code: Tooling Tooling that is not part of the main game but is part of the repo. [JSON] Changes (can be) made in JSON (S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

No branches or pull requests

5 participants