Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ZO-4627: Use lxml.etree for content objects instead of lxml.objectify #626

Merged
merged 15 commits into from
Feb 21, 2024
Merged

ZO-4627: Use lxml.etree for content objects instead of lxml.objectify #626

merged 15 commits into from
Feb 21, 2024

Conversation

wosc
Copy link
Member

@wosc wosc commented Feb 19, 2024

Checklist

  • Documentation
  • Changelog
  • Tests
  • Translations

@wosc wosc force-pushed the ZO-4627 branch 3 times, most recently from 6bb475a to 7093089 Compare February 20, 2024 08:06
@wosc wosc marked this pull request as ready for review February 20, 2024 08:21
louika
louika approved these changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@louika louika left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay, ich finds schöner als vorher. Habe aber Fragen. Nur noch ein Test komisch, hier haben sich https://github.com/ZeitOnline/vivi/pull/626/checks?check_run_id=21804316779 attribute/schemata wieder rein gemogelt.
attack

core/src/zeit/content/volume/volume.py Show resolved Hide resolved
core/src/zeit/content/article/edit/paragraph.py Show resolved Hide resolved
core/src/zeit/cms/content/field.txt Show resolved Hide resolved
core/src/zeit/cms/content/property.py Show resolved Hide resolved
core/src/zeit/cms/content/xmlsupport.py Show resolved Hide resolved
core/src/zeit/cms/content/property.py Outdated Show resolved Hide resolved
core/src/zeit/cms/testing.py Outdated Show resolved Hide resolved
core/src/zeit/content/article/README.txt Show resolved Hide resolved
core/src/zeit/cms/content/lxmlpickle.py Show resolved Hide resolved
@wosc
ZO-4627: Support lxml.etree for content objects instead of lxml.objec…
65f0188 
…tify
@wosc
MAINT: Add missing import
ba484a5
@wosc
MAINT: Pretty print huge json fixture to make somewhat editable
11963e4
@wosc
ZO-4627: Search and replace lxml.objectify with lxml.etree (except fo…
b7e3acf 
…r config sources)
@wosc
ZO-4627: Replace objectify API with etree equivalents
7cc4d95
@wosc
ZO-4627: Remove xml ns cleanup, no longer needed, no more objectify t…
ba5207c 
…o clutter them
@wosc
ZO-4627: Since the whole point is to preserve whitespace, we have to …
a3a64f0 
…do our own pretty printing for tests

See https://lxml.de/FAQ.html#why-doesn-t-the-pretty-print-option-reformat-my-xml-output
@wosc
ZO-4627: Source edit cannot pretty print anymore, either
412d518 
In the future, we might want to mark nodes/subtrees that contain
significant whitespace (e.g. article body, markup text etc.), so that
we can still pretty print anything else.
@wosc
ZO-4627: Remove workaround for lxml.objectify edge case
1ea5a20
@wosc
ZO-4627: Adjust assertion, XML preserves whitespace now
489f1c3
@wosc
ZO-4627: Ignore whitespace in tests (since the xml parser does not, a…
94ac908 
…nymore)
@wosc
ZO-4627: Ignore trailing whitespace in wysiwyg subtrees
060cc46 
Result changed from '<p>bar!</p>\n' to '<p>bar!</p>\n    \n',
and it's a bit unclear why.
@wosc
MAINT: Make doctest less overspecific
3d0b51b
@wosc
ZO-4627: Adjust doctest to changed pretty print behaviour
1fea29d
@wosc
MAINT: Remove unused parameter
20ccbf2 
This has existed since forever (762326d) but was not used even then.
It's unclear if in the interim there used to be some subclass that
used it, but anyway nowadays there isn't, so begone!
@louika louika merged commit 2784239 into main Feb 21, 2024
4 checks passed
@louika louika deleted the ZO-4627 branch February 21, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stollero stollero stollero left review comments

@louika louika louika approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wosc @louika @stollero