Development - #123
Merged
Merged
Conversation
The 0.4.0 section had accumulated in merge order rather than reading order, which left the entries for one issue split across the section with another issue's wedged between them. Reordered so the new capabilities come first, then the precedence rule they all follow, then the fixes, then the two template changes an organisation has to pick up, with the breaking change last. No wording changed - the bullets are the same text, rearranged. Three entries that came out of a filed issue's work were missing its link, so they now carry it. The four entries with no link are incidental fixes with no issue behind them, which matches how earlier sections read. cran-comments.md still described the release as though it were only the column-header merge work, because it was written before the rest of 0.4.0 existed. It now covers what the release actually contains. CRAN-SUBMISSION is deliberately untouched: it records the 0.3.1 submission and is rewritten when 0.4.0 is actually submitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tidy the 0.4.0 release notes and update cran-comments
…107) attr() partial matches, so a `labels` attribute - the value labels haven attaches to coded variables when it reads an XPT - was answering a request for `label`. has_labels_() then reported a variable label that was never there and the header machinery tried to treat a named numeric vector as header text, so clintable() errored before rendering anything. Both reads now ask for the attribute exactly. That is only half of it. flextable reads column labels of its own accord, and clinify never told it not to, so `use_labels = FALSE` did not leave labels alone: the raw label string went into the header, "||" delimiter and all, and there was no way to ask for anything else, since passing use_labels through the dots errors as a duplicate argument. The flag is now forwarded, so it means the same thing for both packages. Forwarding also gives a way past the part of this that is upstream: flextable's own collect_labels() partial matches the same way, so `use_labels = TRUE` on a value-labelled column still errors inside labelizor(). Turning labels off is now a genuine escape hatch where before there was none. Worth reporting upstream. Value label rendering is untouched - a column carrying both a variable label and value labels still renders its values as the labels, which is the reason not to simply switch flextable's handling off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
as_clintable() puts 9 points above the first header row and 9 below the last, but setting the column headers rebuilds the header part and that dropped it. So a plain clintable kept clinify's spacing while any table with custom headers or a "||" column label - the default path - silently sat on flextable's 5 point default instead. Rendered output confirmed it: 5/5 on both header rows where a plain table gets 9. The padding is now factored into one place and applied again after the header is rebuilt, rather than only once at construction. It stays a starting point applied before anything the caller does, so a later flextable::padding() or clin_header_pad() still wins - there are tests for both. No existing snapshot moved, because none of them capture header paragraph spacing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The spacing is applied as the table renders, after the option styler and after anything the caller did, so a call covering every header row overwrote a per-row flextable::padding() set beforehand. A project could therefore have a house wide header buffer, or the handful of tables that need a different gap on one row, but not both - which is the normal situation in a real output suite, and why the CDISC pilot could not adopt the verb at all. `above` and `below` now take either one value for every row in scope or one value per row, and `rows` aims a call at particular header rows and leaves the others alone. So the exception can be stated through the API - above = c(18, 34) - or the house call can be narrowed to the rows it means - rows = 1 - and in both cases a per-row padding on an untouched row survives. `rule_to_body` stays a single value, since there is only ever one first body row per page to space away from the rule. The message for a bad `above`/`below` is now plural, since a value per row is allowed; the existing assertions on it were updated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stop reading value labels as a variable label, and honour use_labels (#107)
# Conflicts: # NEWS.md # tests/testthat/test-clintable.R
Keep the header spacing when the column headers are set (#101)
# Conflicts: # NEWS.md
# Conflicts: # NEWS.md
Let clin_header_pad() space header rows differently (#113)
The last of the three vertical controls #97 asked for. clin_row_height() owned the body, titles and footnotes but nothing reached the column header, so tightening a wrapped arm label meant a raw flextable::line_spacing(part = "header") in the project's styling function. The header needs two levers, not one, and measuring them settled which matters. `header` bounds the header rows the way `body` bounds the body rows - Word gets trHeight/atLeast - but under "atleast" it is a floor, so a header cell holding three lines still grows past it and the multi-line case is untouched. `header_leading` is what closes the gap between those lines: it lands as w:line, so 0.75 draws them three quarters as far apart. Both are available, and the docs say plainly which one reaches a wrapped label. Leading is the one measurement in this function that is not a length. flextable and Word both express it as a multiple of single spacing, so there is no points form to convert to and `unit` does not apply to it. It is named and documented as a multiple for that reason. Applied as the table renders rather than when the verb is called, because a styling function that calls line_spacing(part = "all") - as the CDISC pilot's does - would otherwise undo it. There is a test standing that styler up and checking the leading survives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add header pitch and leading to clin_row_height() (#117)
A second clin_row_height() replaced the first, so anything the earlier call had set reverted to its default with no error or warning. The natural pattern - a house wide call, then a per table exception - was therefore impossible without restating every argument, and getting it wrong rendered a 3.189% pixel difference silently. The reporter's argument for merging is right and it is a cleaner case than the composition question on #113: the state being merged is clinify's own config, where "not specified" is unambiguous, so there is nothing to guess. It was not one function. clin_header_pad() lost `above` the same way, and clin_spanner_rule() lost `border`. All three now fold only the arguments a call actually names into what was already there. clin_table_align() carries a single value, so replacing was already correct there. Two things the fix turns on: Arguments with a default cannot be recognised by being NULL - `rule` and `border` are always populated - so the merge keys off missing() rather than is.null(). And missing() has to be captured before match.arg() runs, because assigning to a formal clears its missing status. Without that, a second call still reset `rule` to "atleast" while looking correct everywhere else. There is a test for exactly that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `merge` docs explained the mechanism - identical, adjacent header cells are merged - but never the symptom, which is that the run then renders as one label centred over all of it. The repeats are not there to read any more. That matters most for the bottom header row, which holds each column's own label. Six columns each labelled "Baseline" come out as a single Baseline spanning all six, so five labels are simply gone from the output with nothing to say so. `merge = "spanners"` is the answer and was already there; only the description of what it saves you from was missing. Found while investigating #120, where this had been misreported as a header wrapping difference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Let a second call refine the config verbs instead of replacing it (#119)
Document what merging a row of repeated header labels looks like (#120)
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
1 participant
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.
No description provided.