bpo-37534: Adding ability to add Standalone Document Declaration when generating XML documents #14912

henryh9n · 2019-07-22T23:31:40Z

When using minidom module to generate XML documents the ability to add Standalone Document Declaration is added.

In case it's set to None, which is by default, the declaration is omitted, otherwise it is set to 'yes' when a True value is passed and 'no' when False is passed.
All the changes are made to generate a document in compliance with Extensible Markup Language (XML) 1.0 (Fifth Edition) W3C Recommendation (available here: https://www.w3.org/TR/xml/#sec-prolog-dtd).
As minidom module is missing documentation for document generator, no changes are made for current functionality. Documentation may be added separately later.

Resolves: 37534

https://bugs.python.org/issue37534

…534).

ZackerySpytz · 2019-07-23T05:02:47Z

Why are there no unit tests?

henryh9n · 2019-08-19T23:33:42Z

Why are there no unit tests?

Forgot to include those. :D See I'm all new in here.
Will add immediately.

…n useless space.

…ors.

…thon into fix-issue-37534

henryh9n · 2019-08-20T13:40:31Z

@ZackerySpytz I've added the unit tests and made a few other changes. Can you please pay me a favour to review those?
Also the Azure Pipeline timed out and cancelled on the macOS PR Test without any failure and I guess there's nothing I can do.

zware · 2019-09-18T20:07:59Z

@scoder, you're about the closest we have to an xml.minidom expert as far as I can tell; would you mind to review this?

scoder

Thanks, looks mostly good. Some more tests will uncover the missing space in formatting.

Docstrings would be nice. I'm aware that they are currently missing, but writing them shouldn't be all that much work, and we have to start somewhere.

The news entry seems a bit verbose, but it's your choice if you want to change it.

scoder · 2019-09-18T20:42:40Z

Lib/test/test_minidom.py

+        self.assertEqual(doc.toxml(standalone=True),
+             '<?xml version="1.0" standalone=\'yes\'?><foo>\u20ac</foo>')
+        self.assertEqual(doc.toxml(standalone=False),
+             '<?xml version="1.0" standalone=\'no\'?><foo>\u20ac</foo>')


I think both values should use the same quotes here. Since both the version and the encoding used double quotes, please also use double quotes for standalone.

Please also add a test that declares an encoding.

Yup, missed the test case with an encoding.
Added and the quote styles are made consistent either.

scoder · 2019-09-18T20:42:40Z

Lib/xml/dom/minidom.py

+            standalone = ''
+
+        writer.write(
+            '<?xml version="1.0" {encoding}{standalone}?>{newline}'.format(


Feel free to use f-strings.

Right. Done as well.

scoder · 2019-09-18T20:42:40Z

Lib/xml/dom/minidom.py

+                 standalone=None):
+        # In case standalone declaration is set
+        if standalone is not None:
+            standalone = "standalone='{}'".format('yes' if standalone else 'no')


I would put a space before standalone=… here.

Absolutely. Added.

scoder · 2019-09-18T20:42:40Z

Misc/ACKS

@@ -1883,3 +1883,4 @@ Robert Leenders
 Tim Hopper
 Dan Lidral-Porter
 Ngalim Siregar
+Henrik Harutyunyan


The intended name sorting is alphabetically by last name. Please try to adhere to that.

bedevere-bot · 2019-09-18T20:42:43Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

henryh9n · 2019-09-18T23:24:32Z

I have made the requested changes; please review again

bedevere-bot · 2019-09-18T23:24:34Z

Thanks for making the requested changes!

@scoder: please review the changes made to this pull request.

henryh9n · 2019-09-18T23:29:30Z

Docstrings would be nice. I'm aware that they are currently missing, but writing them shouldn't be all that much work, and we have to start somewhere.

The next thing I'm gonna do is to enhance the documentation for the module, so I'd rather leave it for now.

scoder · 2019-09-21T13:34:05Z

Lib/test/test_minidom.py

+        self.assertEqual(doc.toxml(standalone=None),
+                         '<?xml version="1.0" ?><foo>\u20ac</foo>')
+        self.assertEqual(doc.toxml(standalone=True),
+            '<?xml version="1.0"  standalone="yes"?><foo>\u20ac</foo>')


There shouldn't be a double space before standalone. It's unfortunate that there is a space after the version even without further parameters, but then, maybe there should just be a space after each of the parameters?
In any case: we should keep the same serialisation as before for all cases without standalone, and make the ones with the new parameter consistent with what was there before.

@scoder the double space there looks very much distracting, one the other hand adding a space after each of the parameters (i.e. encoding) causes failures in some of the past tests.

How about doing something like this:

if encoding and standalone: standalone = ' ' + standalone

… fix-issue-37534

henryh9n · 2019-11-05T15:22:20Z

@scoder Are you reviewing this sometime soon? :)

scoder · 2020-01-28T07:24:44Z

The implementation is good now, but the documentation needs updating, since the signatures are wrong now. See xml.dom.minidon.rst.

henryh9n · 2020-02-23T17:00:19Z

The implementation is good now, but the documentation needs updating, since the signatures are wrong now. See xml.dom.minidon.rst.

@scoder I've updated the docs. You can have a look and merge now.

Doc/library/xml.dom.minidom.rst

codecov · 2020-02-28T07:53:26Z

Codecov Report

Merging #14912 into master will increase coverage by 0.35%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #14912       +/-   ##
===========================================
+ Coverage   82.86%   83.21%    +0.35%     
===========================================
  Files        1860     1571      -289     
  Lines      568810   415181   -153629     
  Branches    41940    44478     +2538     
===========================================
- Hits       471328   345493   -125835     
+ Misses      88307    60039    -28268     
- Partials     9175     9649      +474

Impacted Files	Coverage Δ
Lib/test/test_binhex.py	`13.33% <0.00%> (-79.78%)`	⬇️
Lib/sqlite3/dbapi2.py	`34.88% <0.00%> (-65.12%)`	⬇️
Lib/binhex.py	`17.84% <0.00%> (-58.49%)`	⬇️
Lib/ctypes/test/test_structures.py	`78.23% <0.00%> (-19.13%)`	⬇️
Lib/test/test_audit.py	`66.19% <0.00%> (-16.03%)`	⬇️
Lib/pprint.py	`77.42% <0.00%> (-15.81%)`	⬇️
Lib/test/libregrtest/setup.py	`54.32% <0.00%> (-5.14%)`	⬇️
Lib/test/test_distutils.py	`66.66% <0.00%> (-4.77%)`	⬇️
Lib/crypt.py	`82.85% <0.00%> (-4.45%)`	⬇️
Lib/test/test_capi.py	`89.42% <0.00%> (-4.43%)`	⬇️
... and 836 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42acb7b...51a5f01. Read the comment docs.

scoder · 2020-02-29T08:21:23Z

Thanks!

Adding standalone declaration to minidom XML document writer (Issue37…

c37145a

…534).

the-knights-who-say-ni added the CLA signed label Jul 22, 2019

bedevere-bot added the awaiting review label Jul 22, 2019

blurb-it bot and others added 6 commits Aug 20, 2019

📜🤖 Added by blurb_it.

4035046

Adding standalone variable to toxml() method and removing the mistake…

e7b1c40

…n useless space.

Adding missing unit tests and adding my name to the list of contribut…

e8b1b18

…ors.

Merge branch 'fix-issue-37534' of https://github.com/hharutyunyan/cpy…

d96fd7a

…thon into fix-issue-37534

Merge branch 'master' into fix-issue-37534

c291a64

Minor fix to make the encoding tag backwards compatable.

3c8db77

zware requested a review from scoder Sep 18, 2019

Merge remote-tracking branch 'upstream/master'

edc492a

scoder requested changes Sep 18, 2019

View changes

bedevere-bot added awaiting changes and removed awaiting review labels Sep 18, 2019

hharutyunyan added 5 commits Sep 18, 2019

Using f-strings for formatting the prolog.

70a69a9

Missing space added before standalone declaratoin.

b603224

Using double quotes for standalone declaration.

59a3495

Test case with encoding declaration added.

3a0594b

Last name alphabetical ordering fixed.

162486a

bedevere-bot removed the awaiting changes label Sep 18, 2019

bedevere-bot added the awaiting change review label Sep 18, 2019

bedevere-bot requested a review from scoder Sep 18, 2019

scoder reviewed Sep 21, 2019

View changes

hharutyunyan added 2 commits Oct 24, 2019

Merge branch 'master' of https://github.com/hharutyunyan/cpython into…

e941fc1

… fix-issue-37534

Fixing the double-space issue in document declarations.

5b8cee5

henryh9n requested a review from scoder Oct 24, 2019

csabella requested review from scoder and removed request for scoder Jan 25, 2020

Use f-strings instead of mixing them with format()

e108c3d

scoder approved these changes Jan 28, 2020

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels Jan 28, 2020

Documents modified to cover the new features.

3b38bbe

csabella requested a review from scoder Feb 25, 2020

scoder reviewed Feb 28, 2020

View changes

Doc/library/xml.dom.minidom.rst Outdated Show resolved Hide resolved

Fiy typo in Doc/library/xml.dom.minidom.rst

51a5f01

scoder merged commit dc04a05 into python:master Feb 29, 2020

bedevere-bot removed the awaiting merge label Feb 29, 2020

sthagen mentioned this pull request Feb 29, 2020

bpo-37534: Allow adding Standalone Document Declaration when generati… sthagen/python-cpython#280

Merged

henryh9n deleted the fix-issue-37534 branch Mar 1, 2020

jamadden mentioned this pull request Nov 4, 2020

Add support for Python 3.9. zopefoundation/Products.GenericSetup#99

Merged

bpo-37534: Adding ability to add Standalone Document Declaration when generating XML documents #14912

bpo-37534: Adding ability to add Standalone Document Declaration when generating XML documents #14912

henryh9n commented Jul 22, 2019 •

edited by bedevere-bot

ZackerySpytz commented Jul 23, 2019

henryh9n commented Aug 19, 2019

henryh9n commented Aug 20, 2019

zware commented Sep 18, 2019

scoder left a comment

scoder Sep 18, 2019

henryh9n Sep 18, 2019

scoder Sep 18, 2019

henryh9n Sep 18, 2019

scoder Sep 18, 2019

henryh9n Sep 18, 2019

scoder Sep 18, 2019

bedevere-bot commented Sep 18, 2019

henryh9n commented Sep 18, 2019

bedevere-bot commented Sep 18, 2019

henryh9n commented Sep 18, 2019

scoder Sep 21, 2019

henryh9n Sep 22, 2019

henryh9n commented Nov 5, 2019

scoder commented Jan 28, 2020

henryh9n commented Feb 23, 2020

codecov bot commented Feb 28, 2020

scoder commented Feb 29, 2020

bpo-37534: Adding ability to add Standalone Document Declaration when generating XML documents #14912

bpo-37534: Adding ability to add Standalone Document Declaration when generating XML documents #14912

Conversation

henryh9n commented Jul 22, 2019 • edited by bedevere-bot

When using minidom module to generate XML documents the ability to add Standalone Document Declaration is added.

ZackerySpytz commented Jul 23, 2019

henryh9n commented Aug 19, 2019

henryh9n commented Aug 20, 2019

zware commented Sep 18, 2019

scoder 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

bedevere-bot commented Sep 18, 2019

henryh9n commented Sep 18, 2019

bedevere-bot commented Sep 18, 2019

henryh9n commented Sep 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henryh9n commented Nov 5, 2019

scoder commented Jan 28, 2020

henryh9n commented Feb 23, 2020

codecov bot commented Feb 28, 2020

Codecov Report

scoder commented Feb 29, 2020

henryh9n commented Jul 22, 2019 •

edited by bedevere-bot