Keep the markup that says which table style parts apply - #22
Merged
Conversation
A load and save kept w:tblStyle but threw away the markup that says which of that style's conditional parts to use, so a styled table came back with its style name intact and drawn with base formatting only. That reads as the style having been lost, which is what #8 reported. Two things were dropped. w:tblLook kept only w:val. Every attribute went missing, so firstRow, lastRow, firstColumn, lastColumn, noHBand and noVBand were all lost. It is now a CT_TblLook carrying w:val alongside the six flags. Both forms are kept because writers disagree about which to emit and readers disagree about which to trust, and OOXML booleans are accepted in the 1/0 and true/false spellings. w:cnfStyle was not modelled at all and vanished from every row and cell. It is now read and written on both CT_TrPr and CT_TcPr, first in the element sequence as the schema requires. Adding the field was not enough on its own. Both types skip writing their properties when every field is unset, and those checks did not know about the new field, so a row or cell carrying only a cnfStyle was parsed and then silently dropped again on the way out. The emptiness checks now include it, and a test covers exactly that case because it is the kind of thing that looks fixed and is not. Refs #8.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the table style symptom in #8, which I had previously been unable to reproduce.
Reproduced
Round-tripping a table styled the way Word writes one:
w:tblStylesurvived, which is why the earlier check looked clean and I reported the symptom as not reproducible. The style reference was never the problem.w:tblLookandw:cnfStyleare what say which conditional parts of that style to turn on: header row emphasis, row banding, first column formatting. Losing them leaves the style name in place and the table drawn with base formatting only, which is exactly what "the style of tables is not preserved" looks like from the outside.Fixed
w:tblLookbecomes aCT_TblLookcarryingw:valalongside the six flags. Both forms are kept, because writers disagree about which to emit and readers disagree about which to trust, and OOXML booleans are accepted in both the1/0andtrue/falsespellings.w:cnfStylewas not modelled at all. It is now read and written onCT_TrPrandCT_TcPr, first in the element sequence as the schema requires.One trap worth recording
Adding the field was not enough. Both types skip writing their properties when every field is unset, and those
is_emptychecks did not know about the new field. So a row or cell carrying only acnfStylewas parsed correctly and then silently dropped again on the way out.I only caught it because I re-ran the round trip after the change rather than trusting that parsing implied writing. There is a test for that specific case now.
Verified
The exact markup from the reproduction above now round-trips byte for byte. Three tests: the full conditional-formatting round trip, the properties-holding-only-cnfStyle case, and both boolean spellings.
Checks
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warningsand the full suite all pass. 361 tests, up from 358.python3 scripts/hash_harness.py --checkpasses with 28 entries matching. The generated samples do not use table styles with conditional formatting, so the baselines are untouched.Note on #8
This is one of four symptoms in that issue. The TOC fields were fixed in #5, the vanishing tables in #9, and the front-page custom XML fields most likely by #9 as well although the reporter has not confirmed. So this is the last one I could act on without their file.
Breaking
CT_TblPr::lookchanges fromOption<String>toOption<CT_TblLook>, and bothCT_TrPrandCT_TcPrgain a field. It belongs in the same 0.4.0 as #20.