Skip to content

bpo-27413: add --no-ensure-ascii argument to json.tool #201

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.

Sign up for GitHub

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 13 commits into from
Closed

bpo-27413: add --no-ensure-ascii argument to json.tool #201

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a47f396
Add indent support for json.tool
dhimmel Feb 20, 2017
fb5d984
Add support for disabling ensure_ascii in json.tool
dhimmel Feb 20, 2017
4fb3bb3
Add test for no_escape arg of json.tool
dhimmel Feb 20, 2017
29d01fe
bpo-27413: --no-ensure-ascii argument in json.tool
dhimmel Feb 23, 2017
a3e2b23
Add non-ascii characters to TestTool.data
dhimmel Feb 23, 2017
97152e9
Add --no-indent argument
dhimmel Feb 23, 2017
d699070
Escape non-ascii characters in json.tool tests
dhimmel Feb 23, 2017
0f38c18
Switch --no-indent to use store_const
dhimmel Feb 26, 2017
66a173a
Improve argparse help messages
dhimmel Feb 26, 2017
dd25cf3
Add unicode character in the U+0080-U+00FF range
dhimmel Feb 26, 2017
3413f4f
Format unicode output for JSON
dhimmel Feb 26, 2017
52bafb0
Test indent
dhimmel Feb 26, 2017
1306518
Move unicode tests outside of self.data
dhimmel Feb 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions Lib/json/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@
import sys


def parse_indent(indent):
"""Parse the argparse indent argument."""
if indent == 'None':
return None
if indent == r'\t':
return '\t'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like such special casing.
I think json.tool should be minimal to expose json.dumps as CLI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@methane can you think of any other way to expose setting indent? The issue is that indent can take an int, string, or None. Command line arguments are not ideal for type flexibility. Furthermore, the most common string is a tab, which is difficult to pass via the command line without such special casing.

Another option would be to pass options.indent through ast.literal_eval. Therefore, users could specify any python value, but the command line usage would be more cumbersome.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this either. You can use external commands like unexpand for converting spaces to tabs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-referencing bpo-29636.

@serhiy-storchaka so do you think it's best to simplify so you must either pass --indent an int or None. Anything that cannot be converted to an int besides None would throw an error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pass an integer with --indent. Add --no-indent for None.

try:
return int(indent)
except ValueError:
return indent


def main():
prog = 'python -m json.tool'
description = ('A simple command line interface for json module '
Expand All @@ -25,24 +37,36 @@ def main():
help='a JSON file to be validated or pretty-printed')
parser.add_argument('outfile', nargs='?', type=argparse.FileType('w'),
help='write the output of infile to outfile')
parser.add_argument('--no_escape', action='store_true', default=False,
help='Do not set ensure_ascii to escape non-ASCII characters')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option name should be --ensure_ascii, (and default should be True).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@methane are you suggesting:

parser.add_argument('--ensure_ascii', action='store_false', default=True)
# And then when calling `json.dump`:
ensure_ascii=options.ensure_ascii

The problem here is that it's counterintuitive for --ensure_ascii to result in ensure_ascii=False. In https://bugs.python.org/issue27413, the discussion settled on --no-ensure-ascii, which I'm happy to switch to:

parser.add_argument('--no-ensure-ascii', action='store_true', default=False)
# And then when calling `json.dump`:
ensure_ascii=not options.no_ensure_ascii

Just let me know what's preferred.

parser.add_argument('--indent', default='4', type=parse_indent,
help='Indent level or str for pretty-printing. '
'Use None for the most compact representation. '
r'Use "\t" for tab indentation.')
parser.add_argument('--sort-keys', action='store_true', default=False,
help='sort the output of dictionaries alphabetically by key')
options = parser.parse_args()

# Read input JSON
infile = options.infile or sys.stdin
outfile = options.outfile or sys.stdout
sort_keys = options.sort_keys
with infile:
try:
if sort_keys:
if options.sort_keys:
obj = json.load(infile)
else:
obj = json.load(infile,
object_pairs_hook=collections.OrderedDict)
except ValueError as e:
raise SystemExit(e)

# Export JSON
outfile = options.outfile or sys.stdout
with outfile:
json.dump(obj, outfile, sort_keys=sort_keys, indent=4)
json.dump(obj, outfile,
indent=options.indent,
ensure_ascii=not options.no_escape,
sort_keys=options.sort_keys,
)
outfile.write('\n')


Expand Down
23 changes: 18 additions & 5 deletions Lib/test/test_json/test_tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,22 +68,22 @@ def test_stdin_stdout(self):
self.assertEqual(out.splitlines(), self.expect.encode().splitlines())
self.assertEqual(err, None)

def _create_infile(self):
def _create_infile(self, text):
infile = support.TESTFN
with open(infile, "w") as fp:
self.addCleanup(os.remove, infile)
fp.write(self.data)
fp.write(text)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be better just to add non-ascii data to self.data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a3e2b23.

return infile

def test_infile_stdout(self):
infile = self._create_infile()
infile = self._create_infile(self.data)
rc, out, err = assert_python_ok('-m', 'json.tool', infile)
self.assertEqual(rc, 0)
self.assertEqual(out.splitlines(), self.expect.encode().splitlines())
self.assertEqual(err, b'')

def test_infile_outfile(self):
infile = self._create_infile()
infile = self._create_infile(self.data)
outfile = support.TESTFN + '.out'
rc, out, err = assert_python_ok('-m', 'json.tool', infile, outfile)
self.addCleanup(os.remove, outfile)
Expand All @@ -100,9 +100,22 @@ def test_help_flag(self):
self.assertEqual(err, b'')

def test_sort_keys_flag(self):
infile = self._create_infile()
infile = self._create_infile(self.data)
rc, out, err = assert_python_ok('-m', 'json.tool', '--sort-keys', infile)
self.assertEqual(rc, 0)
self.assertEqual(out.splitlines(),
self.expect_without_sort_keys.encode().splitlines())
self.assertEqual(err, b'')

def test_no_ascii_flag(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_no_ensure_ascii_flag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a3e2b23.

data = '{"json": "🐍 and δ"}'
expect = textwrap.dedent('''\
{
"json": "🐍 and δ"
}
''')
infile = self._create_infile(data)
rc, out, err = assert_python_ok('-m', 'json.tool', '--no_ascii', infile)
self.assertEqual(rc, 0)
self.assertEqual(out.splitlines(), expect.splitlines())
self.assertEqual(err, b'')