fixed error in def_colwidths() when a column label is too long#384
fixed error in def_colwidths() when a column label is too long#384eanokian wants to merge 11 commits into
def_colwidths() when a column label is too long#384Conversation
| break | ||
| } | ||
| newrow <- possdfii[ii, ] | ||
| newrow <- possdfii[ii, ] # TODO: this line seems odd, |
There was a problem hiding this comment.
correct, this line is just completely wrong.
The correct is something like either
newrow <- possdfii[which.max(possdfii$colwidth), ]
or
newrow <- possdfii[which.min(possdfii$colwidth), ]
depending on whether we want to choose the most conservative (max) or most agressive (min) packing. I think aggressive is right because that leaves the most room to successfully pack other columns.
More accurately I think what we want is the narrowest column width that takes up exactly one line less than the the column label did previously, so in addition to the above, the previous
possdfii <- possdf[possdf$col_num == colii & possdf$lbl_lines < old_lbl_lns, ]
would need to change to
possdfii <- possdf[possdf$col_num == colii & possdf$lbl_lines == old_lbl_lns - 1, ]
I think the code after those changes would be safe but an escape hatch that immediately bails if old_lbl_lns is already 1 wouldn't hurt anything
| } else { | ||
| msg <- paste("Unable to reduce label rows required.") | ||
| } | ||
| message(msg) |
There was a problem hiding this comment.
yeah I noticed this too, crazy
| while (!done) { | ||
| oldwdths <- curdf$colwidth | ||
| curdf <- constrict_lbl_lns(curdf, possdf, verbose = verbose) | ||
| curdf <- constrict_lbl_lns(curdf, possdf, avail_spc = spcleft, verbose = verbose) |
| oldwdths <- curdf$colwidth | ||
| curdf <- constrict_lbl_lns(curdf, possdf, verbose = verbose) | ||
| curdf <- constrict_lbl_lns(curdf, possdf, avail_spc = spcleft, verbose = verbose) | ||
| if (all.equal(curdf$colwidth, oldwdths)) { |
There was a problem hiding this comment.
this needs to be isTRUE(all.equal(curdf$colwidth, oldwidths))
Only reason the code ever worked is that this code was either never reached at all or never reached when it was false, likely the former.
2 participants
Pull Request
Fixes #281
Checks