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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amarshall
Copy link

@amarshall amarshall commented Jan 26, 2021

Now it will retain pretty-printing, just removing any indentation. See also #1465.

@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
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

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

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.

8 participants