Code

Opened 4 years ago

Closed 8 months ago

Last modified 8 months ago

#13182 closed Cleanup/optimization (fixed)

Remove useless whitespaces in JSON dump with indent option

Reported by: stephaner 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 stephaner 4 years ago.
Remove trailing whitespace and cosmetic changes (import of get_models, spaces)
less-whitespace-json-serialize.diff (1.8 KB) - added by dpn 4 years ago.
dump_without_patch (637 bytes) - added by gptvnt 3 years ago.
dump_with_patch (622 bytes) - added by gptvnt 3 years ago.
13182.json-white-spaces.diff (1.9 KB) - added by julien 3 years ago.
noTrailingWhitespace.diff (1.5 KB) - added by martmatwarne 11 months ago.
less-whitespace-json-serialize-20130820.diff​ (2.1 KB) - added by pjmattal 8 months ago.
Updated to apply to master. Serialize tests pass.

Download all attachments as: .zip

Change History (28)

Changed 4 years ago by stephaner

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

comment:1 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 4 years ago by dpn

comment:2 Changed 4 years ago by dpn

New patch with tests attached.

Changed 3 years ago by gptvnt

Changed 3 years ago by gptvnt

comment:3 Changed 3 years ago by gptvnt

  • Owner changed from nobody to gptvnt
  • Status changed from new to 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 Changed 3 years ago by dpn

  • Needs tests unset

comment:5 Changed 3 years ago by kmike

  • Cc kmike84@… added

comment:6 Changed 3 years ago by lukeplant

  • Type set to New feature

comment:7 Changed 3 years ago by lukeplant

  • Severity set to Normal

Changed 3 years ago by julien

comment:8 Changed 3 years ago by julien

  • Triage Stage changed from Accepted to Ready for checkin
  • Type changed from New feature to Cleanup/optimization

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

comment:9 Changed 3 years ago by anonymous

  • Easy pickings unset

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

comment:10 Changed 3 years ago by jacob

  • Resolution set to wontfix
  • Status changed from assigned to 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 Changed 15 months ago by hekevintran@…

  • Resolution wontfix deleted
  • Status changed from closed to 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 Changed 15 months ago by charettes

  • Cc charette.s@… added

comment:13 Changed 14 months ago by aaugustin

  • Easy pickings set
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

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

comment:14 Changed 11 months ago by martmatwarne

  • Cc martmatwarne added
  • Owner changed from gptvnt to martmatwarne
  • Status changed from new to assigned

Changed 11 months ago by martmatwarne

comment:15 Changed 11 months 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 11 months ago by martmatwarne

  • Patch needs improvement unset

comment:17 Changed 11 months ago by martmatwarne

  • Triage Stage changed from Accepted to Ready for checkin

comment:18 Changed 11 months ago by martmatwarne

  • Triage Stage changed from Ready for checkin to 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 Changed 9 months ago by timo

  • Patch needs improvement set

Changed 8 months ago by pjmattal

Updated to apply to master. Serialize tests pass.

comment:20 Changed 8 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from assigned to 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 Changed 8 months 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.