-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-29636: improve CLI of json.tool
#9765
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
Conversation
parser.add_argument('--sort-keys', action='store_true', default=False, | ||
help='sort the output of dictionaries alphabetically by key') | ||
options = parser.parse_args() | ||
|
||
infile = options.infile or sys.stdin | ||
outfile = options.outfile or sys.stdout | ||
sort_keys = options.sort_keys | ||
try: | ||
indent = int(options.indent) | ||
except ValueError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you use type in argparse you can remove this section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, then it will be impossible to pass tab character through to json dumps from the command-line, for example by using --indent=" "
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack!
…`json.tool` command-line interface.
Author of bpo-29636 and the associated PR #345 to add indentation options to
I initially started out my PR with only supporting Anyways, @wimglenn happy to have you involved, if you'd like to help with any of the open PRs and discussion on BPO. |
@wimglenn #345 was merged into 0325794 providing indentation control for json.tool. I also have #201 open providing an option for ensure_ascii. I'd be happy to close that PR in favor of this PR, if you are interesting in updating the scope to just be the ascii control and maintaining the PR until its merged. |
Hey Daniel, thanks for the ping. That's a little bizarre, the last discussion on issue29636 was
Then suddenly 4 days ago @methane asks "would you resolve the conflict?" and it's merged without further ado? What changed? Did these two core developers change their mind in some offline discussion? Why was it merged with no documentation, there should be a change in I mean I'm glad to have the feature in, even if I think the #345 was a little too complicated it's better than not having the functionality at all. I don't have much interest in reworking this PR for the --ensure-ascii, it's still labelled "awaiting core review" with not a peep from core in more than a year so I think I'll just be wasting my time and effort. |
I merged #345 because I want to reduce open PRs and issues. I thought about I should reject it or merge it. json-lines is not valid JSON, and output of the option is not a valid json-lines too. |
@methane "To reduce open PRs and issues" is a noble goal, but you don't request documentation for the change to go in alongside the change itself? And to disregard/ignore the comments from other core developers? I'll put the |
Yes, I noticed I missed it. It should be added.
Sometime, it is difficult to make a final agreement. We discuss in python-dev mailing list If the problem is important enough. I think this is important to reduce open PRs and issues. We can not discuss and make a decision in the mailing list for all feature requests. Time is a limited resource. Of course, I might be wrong and other core devs oppose it strongly. We can revert it then. |
OK. Well, it is good that we are burning down open PRs and issues at least. Poor @dhimmel was trying to move this since Feb 2017 (!) Here is the other part (created a new PR because this one has wrong bpo number now) --> #17472 Off-topic: that is a really cute dog @methane ... is a long haired dachshund? |
Added ability to pass through
indent
andensure_ascii
options injson.tool
command-line interface.Related issues are 27413 and 29636.
There is other PR existing: #345 - that one is stagnating because @serhiy-storchaka commented on the bpo thinking it should not be merged. Serhiy claimed it adds too many (potentially conflicting) options, and I would tend to agree. This alternate implementation is a more straight-forward pass-thru of the relevant options to
json.dumps
.Some motivation: How to use json.tool from the shell to validate and pretty-print language files without removing the unicode?
https://bugs.python.org/issue29636