Add "use_length" argument to str_pad() #190
Conversation
|
I think this is a good idea, but the docs are confusing me. Does this change the current default behaviour? (It probably should since you're usually padding to make strings aligned for output) |
tests/testthat/test-pad.r
Outdated
| @@ -18,3 +18,8 @@ test_that("directions work for simple case", { | |||
| expect_equal(pad("left"), " had") | |||
| expect_equal(pad("both"), " had ") | |||
| }) | |||
|
|
|||
| test_that("padding based of length works", { | |||
| expect_identical(str_pad("\u4e2d", width = 6, side = "both"), " \u4e2d ") | |||
There was a problem hiding this comment.
Needs comment describing what "\u4e2d" is
No, but I think you don't need to be afraid. The default behaviour already had been changed on the stringi's side after version 0.5-2, which pads strings based on the total character widths so that strings are properly aligned for output. Users who want the old behaviour can still choose the old behaviour by Does my explanation make sense? I'm sorry maybe my English is not good enough to document this nicely :( |
|
In that case, this doesn't seem super useful to me given that the point of |
|
Agreed. This is not super useful. Yet, it seems some people like #188 need the behaviour consistent with the default of base Maybe you can close this PR for now and reopen if needed :) |
|
Yeah, let me dig into that issue a bit more. Thanks! |
Conflicts: NEWS.md
Conflicts: NEWS.md
|
Thanks for merging! |
Fixes #188
As I commented on #188,
stri_pad_*()gainsuse_length(gagolews/stringi#149) to control whether to use the total code point width or the number of code points as "width" of a string. But, the corresponding argument is not added on stringr's side so far. This PR simply adds it tostr_pad().As base R's
nchar()has similar argumenttypewhich takes either"bytes","chars"(default), or"width", I guess there must be some people who wants to control this behaviour, so exposing this argument is reasonable.