Opened 7 years ago

Closed 3 years ago

Last modified 3 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: master
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 7 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 6 years ago.
dump_without_patch (637 bytes) - added by gptvnt 6 years ago.
dump_with_patch (622 bytes) - added by gptvnt 6 years ago.
13182.json-white-spaces.diff (1.9 KB) - added by Julien Phalip 5 years ago.
noTrailingWhitespace.diff (1.5 KB) - added by martmatwarne 3 years ago.
less-whitespace-json-serialize-20130820.diff​ (2.1 KB) - added by pjmattal 3 years ago.
Updated to apply to master. Serialize tests pass.

Download all attachments as: .zip

Change History (28)

Changed 7 years ago by Stephane Raimbault

Attachment: dumpdata-json.patch added

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

comment:1 Changed 7 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: set
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Changed 6 years ago by David Novakovic

comment:2 Changed 6 years ago by David Novakovic

New patch with tests attached.

Changed 6 years ago by gptvnt

Attachment: dump_without_patch added

Changed 6 years ago by gptvnt

Attachment: dump_with_patch added

comment:3 Changed 6 years ago by gptvnt

Owner: changed from nobody to gptvnt
Status: newassigned

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 Changed 6 years ago by David Novakovic

Needs tests: unset

comment:5 Changed 6 years ago by Mikhail Korobov

Cc: kmike84@… added

comment:6 Changed 5 years ago by Luke Plant

Type: New feature

comment:7 Changed 5 years ago by Luke Plant

Severity: Normal

Changed 5 years ago by Julien Phalip

comment:8 Changed 5 years ago by Julien Phalip

Triage Stage: AcceptedReady for checkin
Type: New featureCleanup/optimization

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

comment:9 Changed 5 years ago by anonymous

Easy pickings: unset

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

comment:10 Changed 5 years ago by Jacob

Resolution: wontfix
Status: assignedclosed

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 Changed 4 years ago by hekevintran@…

Resolution: wontfix
Status: closednew
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 Changed 4 years ago by Simon Charette

Cc: charette.s@… added

comment:13 Changed 4 years ago by Aymeric Augustin

Easy pickings: set
Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

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

comment:14 Changed 3 years ago by martmatwarne

Cc: martmatwarne added
Owner: changed from gptvnt to martmatwarne
Status: newassigned

Changed 3 years ago by martmatwarne

Attachment: noTrailingWhitespace.diff added

comment:15 Changed 3 years ago by martmatwarne

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 Changed 3 years ago by martmatwarne

Patch needs improvement: unset

comment:17 Changed 3 years ago by martmatwarne

Triage Stage: AcceptedReady for checkin

comment:18 Changed 3 years ago by martmatwarne

Triage Stage: Ready for checkinAccepted

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 Changed 3 years ago by Tim Graham

Patch needs improvement: set

Changed 3 years ago by pjmattal

Updated to apply to master. Serialize tests pass.

comment:20 Changed 3 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

In 3e34005b1b4a9a447e3b3fd76e71f0c73d530414:

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

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

comment:21 Changed 3 years ago by Ramiro Morales <cramm0@…>

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