Add logic to print diff between expected and actual JSON #408

joepin · 2019-08-15T16:42:33Z

This PR makes the pretty JSON check more verbose when it encounters errors, that way developers can see which lines are causing errors in order to debug.

I haven't seen any talk about adding this anywhere, but would love to hear the maintainers' thoughts on this.

This commit makes the pretty JSON check more verbose when it encounters errors, that way developers can see which lines are causing errors in order to debug.

pre_commit_hooks/pretty_format_json.py

This prints a diff between the given json file and the expected (pretty) output, with this functionality hidden behind a cli flag

pre_commit_hooks/pretty_format_json.py

This commit uses capsys to test the output of the diff, which is now hidden behind the autofix flag if it's disabled

tests/pretty_format_json_test.py

joepin · 2019-09-13T18:36:18Z

@asottile thanks for all the help! Please let me know if my most recent change looks good to you 🙂

asottile · 2019-09-13T18:48:20Z

let's print the filename stuff to stderr (and flush)

joepin · 2019-09-13T19:11:48Z

let's print the filename stuff to stderr (and flush)

I'm not sure what you mean by that - do you mean to say this should be printed to stderr? (As it stands now, the main func prints both the file name thing and the diff output to stdout)

asottile · 2019-09-14T00:42:38Z

let's print the filename stuff to stderr (and flush)

I'm not sure what you mean by that - do you mean to say this should be printed to stderr? (As it stands now, the main func prints both the file name thing and the diff output to stdout)

yes

asottile

close!!!!

pre_commit_hooks/pretty_format_json.py

tests/pretty_format_json_test.py

asottile

joepin · 2019-09-24T19:24:12Z

tests/pretty_format_json_test.py

+    resource_path = get_resource_path('not_pretty_formatted_json.json')
+    expected_retval = 1
+    expected_out = '''\
+--- \n+++ \n@@ -1,6 +1,9 @@


This is the line I was talking about. unified_diff prints:

--- +++ @@ -1,6 +1,9 @@

where the first two lines have a trailing space. This was my temporary solution to pass the test cases and satisfy the pre commit check

ohh this is where the filename is supposed to be

maybe we should pass in a/filename and b/filename so that it prints better?

where filename is the actual filename -- so os.path.join('a', filename) and os.path.join('b', filename)

Ahh that makes sense - maybe we can just put in some pseudo-names? That way it's a little clearer what the output is saying

So it would be

unified_diff(source, target, fromfile='not_pretty.json', tofile='pretty.json')

yeah, here's an example: https://github.com/asottile/reorder_python_imports/blob/df5d5d28abcf8705bd0c2a622480f6099ca56add/reorder_python_imports.py#L511-L515

Gotcha - adding it!

asottile · 2019-09-24T19:44:05Z

pre_commit_hooks/pretty_format_json.py

+                else:
+                    print(
+                        get_diff(
+                            ''.join(contents),


this should just be contents right?

🤔 I remember needing to join it before

I guess not now that we're using splitlines?

asottile · 2019-09-24T20:11:08Z

tests/pretty_format_json_test.py

+  "blah": null,
+  "foo": "bar"
+ }}
+


ugh sorry for one more

this extra newline indicates that it should be print(..., end='') for the diff (the diff itself includes the ending newline already)

No worries - updated!

asottile

joepin · 2019-09-24T21:03:22Z

Awesome! Thanks for all your help and guidance on this one! 😄

asottile · 2019-10-28T22:25:49Z

this has been released in v2.4.0!

Add logic to print line number of JSON errors

3e9db01

This commit makes the pretty JSON check more verbose when it encounters errors, that way developers can see which lines are causing errors in order to debug.

asottile reviewed Aug 15, 2019

View reviewed changes

pre_commit_hooks/pretty_format_json.py Outdated Show resolved Hide resolved

Add option to show expected output

780f202

This prints a diff between the given json file and the expected (pretty) output, with this functionality hidden behind a cli flag

asottile reviewed Aug 16, 2019

View reviewed changes

pre_commit_hooks/pretty_format_json.py Outdated Show resolved Hide resolved

pre_commit_hooks/pretty_format_json.py Outdated Show resolved Hide resolved

Add test case to test diffing function

b28837a

joepin changed the title ~~Add logic to print line number of JSON errors~~ Add logic to print diff between expected and actual JSON Aug 23, 2019

asottile reviewed Aug 23, 2019

View reviewed changes

pre_commit_hooks/pretty_format_json.py Outdated Show resolved Hide resolved

Remove extra cli flag, and update test case

0ff23d4

This commit uses capsys to test the output of the diff, which is now hidden behind the autofix flag if it's disabled

asottile reviewed Sep 13, 2019

View reviewed changes

tests/pretty_format_json_test.py Outdated Show resolved Hide resolved

Remove erroneoous print statement

93b7b66

joepin added 2 commits September 15, 2019 12:51

Print file line stderr

831d2a9

Add test for std err

35c76c4

asottile reviewed Sep 15, 2019

View reviewed changes

pre_commit_hooks/pretty_format_json.py Show resolved Hide resolved

tests/pretty_format_json_test.py Outdated Show resolved Hide resolved

tests/pretty_format_json_test.py Outdated Show resolved Hide resolved

joepin added 3 commits September 15, 2019 13:29

Flush stderr, and formatting

27cd688

Use unified_diff

31e740e

Fix test cases for unified_diff

7d878b5

asottile approved these changes Sep 24, 2019

View reviewed changes

joepin commented Sep 24, 2019

View reviewed changes

Print filenames in the diff

ec6c39e

asottile reviewed Sep 24, 2019

View reviewed changes

Remove unneeded string join

ea8fbf9

asottile reviewed Sep 24, 2019

View reviewed changes

Remove useless newline

d6c0aa5

asottile approved these changes Sep 24, 2019

View reviewed changes

asottile merged commit 277f875 into pre-commit:master Sep 24, 2019

joepin deleted the pretty-json-print-lines branch September 24, 2019 21:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logic to print diff between expected and actual JSON #408

Add logic to print diff between expected and actual JSON #408

joepin commented Aug 15, 2019

joepin commented Sep 13, 2019

asottile commented Sep 13, 2019

joepin commented Sep 13, 2019

asottile commented Sep 14, 2019

asottile left a comment

asottile left a comment

joepin Sep 24, 2019

asottile Sep 24, 2019

asottile Sep 24, 2019

joepin Sep 24, 2019

asottile Sep 24, 2019

joepin Sep 24, 2019

asottile Sep 24, 2019

joepin Sep 24, 2019

asottile Sep 24, 2019

joepin Sep 24, 2019

asottile left a comment

joepin commented Sep 24, 2019

asottile commented Oct 28, 2019

Add logic to print diff between expected and actual JSON #408

Add logic to print diff between expected and actual JSON #408

Conversation

joepin commented Aug 15, 2019

joepin commented Sep 13, 2019

asottile commented Sep 13, 2019

joepin commented Sep 13, 2019

asottile commented Sep 14, 2019

asottile left a comment

Choose a reason for hiding this comment

asottile left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asottile left a comment

Choose a reason for hiding this comment

joepin commented Sep 24, 2019

asottile commented Oct 28, 2019