Opened 15 years ago

Closed 11 years ago

Last modified 11 years ago

#13182 closed Cleanup/optimization (fixed)

Remove useless whitespaces in JSON dump with indent option

Reported by: Stephane Raimbault Owned by: martmatwarne
Component: Core (Serialization) Version: dev
Severity: Normal Keywords:
Cc: kmike84@…, charette.s@…, martmatwarne Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

When a JSON dump is done with the indent option, all lines are terminated by a whitespace and a newline.
The trailing whitespace is useless, it increases the file size and emits a warning in my editor and git.

The goal of my patch is to remove these whitespaces (specific to the JSON serializer).

Attachments (7)

dumpdata-json.patch​ (1.7 KB ) - added by Stephane Raimbault 15 years ago.
Remove trailing whitespace and cosmetic changes (import of get_models, spaces)
less-whitespace-json-serialize.diff​ (1.8 KB ) - added by David Novakovic 14 years ago.
dump_without_patch​ (637 bytes ) - added by gptvnt 14 years ago.
dump_with_patch​ (622 bytes ) - added by gptvnt 14 years ago.
13182.json-white-spaces.diff​ (1.9 KB ) - added by Julien Phalip 14 years ago.
noTrailingWhitespace.diff​ (1.5 KB ) - added by martmatwarne 12 years ago.
less-whitespace-json-serialize-20130820.diff​​ (2.1 KB ) - added by pjmattal 11 years ago.
Updated to apply to master. Serialize tests pass.

Download all attachments as: .zip

Change History (28)

by Stephane Raimbault, 15 years ago

Attachment: dumpdata-json.patch​ added

Remove trailing whitespace and cosmetic changes (import of get_models, spaces)

comment:1 by Russell Keith-Magee, 15 years ago

Needs tests: set
Triage Stage: Unreviewed → Accepted

by David Novakovic, 14 years ago

Attachment: less-whitespace-json-serialize.diff​ added

comment:2 by David Novakovic, 14 years ago

New patch with tests attached.

by gptvnt, 14 years ago

Attachment: dump_without_patch​ added

by gptvnt, 14 years ago

Attachment: dump_with_patch​ added

comment:3 by gptvnt, 14 years ago

Owner: changed from nobody to gptvnt
Status: new → assigned

I tested the latest patch and ran the test. It seems to be working fine. I have attached the data dump I got in both the cases (with and without the patch) while testing.

comment:4 by David Novakovic, 14 years ago

Needs tests: unset

comment:5 by Mikhail Korobov, 14 years ago

Cc: kmike84@… added

comment:6 by Luke Plant, 14 years ago

Type: → New feature

comment:7 by Luke Plant, 14 years ago

Severity: → Normal

by Julien Phalip, 14 years ago

Attachment: 13182.json-white-spaces.diff​ added

comment:8 by Julien Phalip, 14 years ago

Triage Stage: Accepted → Ready for checkin
Type: New feature → Cleanup/optimization

The patch looks great. I've just updated it to work with current trunk and rejigged the comments a little.

comment:9 by anonymous, 14 years ago

Easy pickings: unset

As of ​v2.1.5. this is fixed in simplejson.

comment:10 by Jacob, 14 years ago

Resolution: → wontfix
Status: assigned → closed

In that case I'm going to mark this wontfix. An explicitly installed simplejson will override the one Django ships with, so if you care about this then just pip install simplejson

comment:11 by hekevintran@…, 12 years ago

Resolution: wontfix
Status: closed → new
UI/UX: unset

This ticket should be reconsidered because Django 1.5 has removed its copy of simplejson in favour of json from Python's standard library (which is based on version 2.0.9 of simplejson). As a result there is no way to prevent having trailing whitespace in when using the JSON serializer.

See ​https://docs.djangoproject.com/en/dev/releases/1.5/#system-version-of-simplejson-no-longer-used

comment:12 by Simon Charette, 12 years ago

Cc: charette.s@… added

comment:13 by Aymeric Augustin, 12 years ago

Easy pickings: set
Patch needs improvement: set
Triage Stage: Ready for checkin → Accepted

Unfortunately the patch doesn't apply any longer, so this isn't RFC.

comment:14 by martmatwarne, 12 years ago

Cc: martmatwarne added
Owner: changed from gptvnt to martmatwarne
Status: new → assigned

by martmatwarne, 12 years ago

Attachment: noTrailingWhitespace.diff​ added

comment:15 by martmatwarne, 12 years ago

Removing the two lines in the diff fixes the issue. I can't think of a reason why you'd want to have a blank line at the bottom but correct me if I'm wrong.

comment:16 by martmatwarne, 12 years ago

Patch needs improvement: unset

comment:17 by martmatwarne, 12 years ago

Triage Stage: Accepted → Ready for checkin

comment:18 by martmatwarne, 12 years ago

Triage Stage: Ready for checkin → Accepted

Actually this doesn't completely work as it will mean when you do a command such as dumpdata that your prompt doesn't go to the next line

comment:19 by Tim Graham, 11 years ago

Patch needs improvement: set

by pjmattal, 11 years ago

Attachment: less-whitespace-json-serialize-20130820.diff​​ added

Updated to apply to master. Serialize tests pass.

comment:20 by Claude Paroz <claude@…>, 11 years ago

Resolution: → fixed
Status: assigned → closed

In 3e34005b1b4a9a447e3b3fd76e71f0c73d530414:

Fixed #13182 -- Prevented trailing spaces in indented json output

Thanks Stéphane Raimbault for the report and the initial patch.

comment:21 by Ramiro Morales <cramm0@…>, 11 years ago

In c01cd4c4234368b050cb872defe5f16597db4bdb:

Change test added in 3e34005b1b to be more stable.

It could fail when actual serialization JSON field ordering was
different from the hard-coded one. Refs #13182.

Note: See TracTickets for help on using tickets.
Back to Top