Skip to content

Fix --indent 0 implicitly enabling compact-output #2256

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

Closed
wants to merge 1 commit into from

Conversation

amarshall
Copy link
Contributor

@amarshall amarshall commented Jan 26, 2021

Now it will retain pretty-printing, just removing any indentation. Fixes #1465 and fixes #2498.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.509% when pulling bedc1ff on amarshall:fix-indent-zero into 80052e5 on stedolan:master.

@amarshall
Copy link
Contributor Author

@itchyny I would argue this is a bug fix, not a feature request. The manpage doesn’t document the prior behavior, and --indent 0 behaves completely differently from any other value. That said, I understand someone, somewhere might be relying on the prior functionality. Ultimately it’s the decision of maintainers, I don’t mind either way.

@itchyny
Copy link
Contributor

itchyny commented Jul 31, 2023

Okay, rebase this PR, please. Also tests are needed.

@nicowilliams
Copy link
Contributor

Hmm, how do I approve the actions to run on this PR?

@wader
Copy link
Member

wader commented Jan 16, 2024

@nicowilliams i'm guessing it needs a new push to run

@PatMyron
Copy link

Now it will retain pretty-printing, just remove any indentation.
@wader
Copy link
Member

wader commented Jan 25, 2024

Seems to break --indent 1

$ ./jq --indent 0 -n '[{a:1}]'
[
{
"a": 1
}
]
$ ./jq --indent 1 -n '[{a:1}]'
[{"a":1}]
$ ./jq --indent 2 -n '[{a:1}]'
[
  {
    "a": 1
  }
]

Maybe this is also an indication we should have some tests for indent/compact flags?

@gbrlmarn
Copy link
Contributor

Hello, I've created a pr #3202 with tests regarding this pull request.

@itchyny
Copy link
Contributor

itchyny commented Jan 23, 2025

Seems to break --indent 1

I don't reproduce the issue. Seems working.

BTW (n) == 0 check is not necessary anymore because (0) << 8 is 0.

@wader
Copy link
Member

wader commented Jan 23, 2025

@itchyny hmm strange but good i guess :) maybe we should merge the tests from #3202 into this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--indent 0 omits more than indentations --indent 0 removes newlines
8 participants