Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@printf should use textwidth for "%ns" and "%0.ns" #41068

Closed
stevengj opened this issue Jun 3, 2021 · 6 comments · Fixed by #41085
Closed

@printf should use textwidth for "%ns" and "%0.ns" #41068

stevengj opened this issue Jun 3, 2021 · 6 comments · Fixed by #41085
Labels
display and printing Aesthetics and correctness of printed representations of objects. unicode Related to unicode characters and encodings

Comments

@stevengj
Copy link
Member

stevengj commented Jun 3, 2021

This seems wrong:

julia> @printf("%5s", "foo")
  foo

julia> @printf("%5s", "föó")
föó

julia> @printf("%.2s", "föó")
fo

It seems like @printf (in this code) should really use textwidth to compute string widths for formatting purposes, similar to this discussion on discourse.

(To improve backward compatibility, I suppose we could use textwidth only for %S, which is currently a rarely used synonym for %s, or use another format code entirely.)

@stevengj stevengj added unicode Related to unicode characters and encodings display and printing Aesthetics and correctness of printed representations of objects. labels Jun 3, 2021
@KristofferC
Copy link
Member

Related: #38256

@stevengj
Copy link
Member Author

stevengj commented Jun 3, 2021

Python 3 also gets this wrong:

>>> print('%0.2s' % 'föó')
fo
>>> f'{"föó": <{5}}'
'föó'
>>> f'{"foo": <{5}}'
'foo  '

@ArunS-tack
Copy link
Contributor

ArunS-tack commented Jun 4, 2021

I think even with textwidth, it's giving out the wrong output.

julia> @printf("%5s", "föo")
  fö
julia> @printf("%5s", "foo")
  foo
julia> @printf("%.2s", "föó")
fo

@stevengj
Copy link
Member Author

stevengj commented Jun 4, 2021

@arunsanganal, it works if you use textwidth correctly. I get the correct output:

julia> @printf("%5s", "föo")
  föo
julia> @printf("%5s", "foo")
  foo
julia> Printf.@printf("%.2s", "föó")
fö

with

diff --git a/stdlib/Printf/src/Printf.jl b/stdlib/Printf/src/Printf.jl
index 5f42df50b8..282d0ebdfd 100644
--- a/stdlib/Printf/src/Printf.jl
+++ b/stdlib/Printf/src/Printf.jl
@@ -239,7 +239,7 @@ end
 @inline function fmt(buf, pos, arg, spec::Spec{T}) where {T <: Strings}
     leftalign, hash, width, prec = spec.leftalign, spec.hash, spec.width, spec.precision
     str = string(arg)
-    slen = length(str) + (hash ? arg isa AbstractString ? 2 : 1 : 0)
+    slen = textwidth(str) + (hash ? arg isa AbstractString ? 2 : 1 : 0)
     op = p = prec == -1 ? slen : min(slen, prec)
     if !leftalign && width > p
         for _ = 1:(width - p)
@@ -259,9 +259,9 @@ end
         end
     end
     for c in str
-        p == 0 && break
+        p -= textwidth(c)
+        p < 0 && break
         pos = writechar(buf, pos, c)
-        p -= 1
     end
     if hash && arg isa AbstractString && p > 0
         buf[pos] = UInt8('"')
@@ -753,8 +753,9 @@ plength(f::Spec{Pointer}, x) = max(f.width, 2 * sizeof(x) + 2)
 
 function plength(f::Spec{T}, x) where {T <: Strings}
     str = string(x)
-    p = f.precision == -1 ? (length(str) + (f.hash ? (x isa Symbol ? 1 : 2) : 0)) : f.precision
-    return max(f.width, p) + (sizeof(str) - length(str))
+    sw = textwidth(str)
+    p = f.precision == -1 ? (sw + (f.hash ? (x isa Symbol ? 1 : 2) : 0)) : f.precision
+    return max(f.width, p) + (sizeof(str) - sw)
 end
 
 function plength(f::Spec{T}, x) where {T <: Ints}

@ArunS-tack
Copy link
Contributor

Ah! i had missed the edit on line 259. It does work fine now. 😄

@stevengj
Copy link
Member Author

stevengj commented Jun 4, 2021

I'll have a PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects. unicode Related to unicode characters and encodings
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants