Fix #38256 (lpad/rpad defined in terms of textwidth)

[mimame]

Fix #38256 (lpad/rpad defined in terms of textwidth)

Julia is awesome and I hope to be useful 😊

[JeffBezanson]

Great, thanks! This should have a NEWS entry since it could technically change the behavior of some code.

[JeffBezanson]

Also the doc string needs to be changed, since it says the function pads in terms of code points.

[mimame]

Hi @JeffBezanson, I'm really sorry by the delay and I hope the fix still can be backported to the 1.6 release 😞
I've updated the doc and added the entry in the NEWS.md file
Don't hesitate to require more changes I promise to pay attention

[oscardssmith]

This is definitely too late to backport to 1.6. We're already in RC2. Glad this change is getting made though.

[StefanKarpinski]

It would be great to have this finished. It still needs a compat entry explaining that the behavior changed in 1.7 and the NEWS conflict needs to be fixed. Otherwise it looks good to go. Very close!

[stevengj]

Added the compat annotation and fixed the merge conflict.

[vtjnash]

something going wrong with this? seeing this fail sometimes on master:
https://build.julialang.org/#builders/1/builds/4447

Error in testset Tar:
Error During Test at /buildworker/worker/tester_linuxaarch64/build/share/julia/stdlib/v1.7/Tar/test/runtests.jl:879
  Got exception outside of a @test
  DivideError: integer division error
  Stacktrace:
    [1] div
      @ ./int.jl:278 [inlined]
    [2] divrem
      @ ./div.jl:162 [inlined]
    [3] divrem
      @ ./div.jl:158 [inlined]
    [4] lpad(s::String, n::Int64, p::String)
      @ Base ./strings/util.jl:352
    [5] macro expansion
      @ /buildworker/worker/tester_linuxaarch64/build/share/julia/stdlib/v1.7/Tar/test/runtests.jl:910 [inlined]
    [6] macro expansion
      @ /buildworker/worker/package_linuxaarch64/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1282 [inlined]
    [7] macro expansion
      @ /buildworker/worker/tester_linuxaarch64/build/share/julia/stdlib/v1.7/Tar/test/runtests.jl:881 [inlined]
    [8] macro expansion
      @ /buildworker/worker/package_linuxaarch64/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1282 [inlined]
    [9] top-level scope
      @ /buildworker/worker/tester_linuxaarch64/build/share/julia/stdlib/v1.7/Tar/test/runtests.jl:872
   [10] include
      @ ./Base.jl:417 [inlined]
   [11] macro expansion
      @ /buildworker/worker/tester_linuxaarch64/build/share/julia/test/testdefs.jl:24 [inlined]
   [12] macro expansion
      @ /buildworker/worker/package_linuxaarch64/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1282 [inlined]
   [13] macro expansion
      @ /buildworker/worker/tester_linuxaarch64/build/share/julia/test/testdefs.jl:23 [inlined]
   [14] macro expansion
      @ ./timing.jl:368 [inlined]
   [15] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
      @ Main /buildworker/worker/tester_linuxaarch64/build/share/julia/test/testdefs.jl:21
   [16] (::Distributed.var"#106#108"{Distributed.CallMsg{:call_fetch}})()
      @ Distributed /buildworker/worker/package_linuxaarch64/build/usr/share/julia/stdlib/v1.7/Distributed/src/process_messages.jl:278
   [17] run_work_thunk(thunk::Distributed.var"#106#108"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
      @ Distributed /buildworker/worker/package_linuxaarch64/build/usr/share/julia/stdlib/v1.7/Distributed/src/process_messages.jl:63
   [18] macro expansion
      @ /buildworker/worker/package_linuxaarch64/build/usr/share/julia/stdlib/v1.7/Distributed/src/process_messages.jl:278 [inlined]
   [19] (::Distributed.var"#105#107"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
      @ Distributed ./task.jl:411

[stevengj]

The Tar.jl test is calling lpad("\x7f"*"\xff"^7, 11, "\0"), which now fails at divrem because the padding string "\0" has textwidth zero.

One option would be to fall back to padding by length in the case of a zero-width padding string.

[giordano]

This is causing problems also for me in rpaded strings sent over MPI, where I do want the length size, not the textwidth size

[StefanKarpinski]

You probably actually want the ncodeunits size in that case, right?

[giordano]

I had tried that, but wasn't working. What I'm sending via MPI is the Vector{Char} of the string, so I believe its size corresponds to length. I replaced rpad-ing the string with fill(' ', total_length) and then copying the chars into this vector.

[StefanKarpinski]

Oh, interesting. That's a very inefficient way to send a string but yes, you would want length there.

[GordStephen]

I'm seeing a similar issue where a fixed-length Vector{Char} needs to be padded with null bytes. I'll manually pre-fill and copy the chars, but a fallback for zero-width padding strings/chars would be nice.