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

Take into account color and unicode in matrix alignment #45751

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Conversation

Keno
Copy link
Member

@Keno Keno commented Jun 19, 2022

Without this, alignment would count characters rather than
textwidth as well as counting inline escape sequences in
colored output. Fix that by using uncolored printing for
alignment and textwidth rather than number of codepoints.

Keno added a commit to Keno/ModelingToolkit.jl that referenced this pull request Jun 19, 2022
Helps with debugging. Needs JuliaLang/julia#45751
for proper formatting, but works without it.
@Keno Keno force-pushed the kf/alignmentprint branch from a5467e8 to 324c2f8 Compare June 19, 2022 19:16
Keno added a commit to Keno/ModelingToolkit.jl that referenced this pull request Jun 19, 2022
Helps with debugging. Needs JuliaLang/julia#45751
for proper formatting, but works without it.
@Keno Keno force-pushed the kf/alignmentprint branch from 324c2f8 to 7aa2cee Compare June 19, 2022 20:41
Without this, alignment would count characters rather than
textwidth as well as counting inline escape sequences in
colored output. Fix that by using uncolored printing for
alignment and textwidth rather than number of codepoints.
@Keno Keno force-pushed the kf/alignmentprint branch from 7aa2cee to 592e4a4 Compare June 19, 2022 20:58
@Keno
Copy link
Member Author

Keno commented Jun 22, 2022

Test failure is because the branch is missing #45670.

@Keno Keno merged commit 626acd4 into master Jun 22, 2022
@Keno Keno deleted the kf/alignmentprint branch June 22, 2022 03:25
pcjentsch pushed a commit to pcjentsch/julia that referenced this pull request Aug 18, 2022
)

Without this, alignment would count characters rather than
textwidth as well as counting inline escape sequences in
colored output. Fix that by using uncolored printing for
alignment and textwidth rather than number of codepoints.
@YingboMa
Copy link
Contributor

Can this get backported?

@Keno Keno added the backport 1.8 Change should be backported to release-1.8 label Oct 20, 2022
KristofferC pushed a commit that referenced this pull request Oct 27, 2022
Without this, alignment would count characters rather than
textwidth as well as counting inline escape sequences in
colored output. Fix that by using uncolored printing for
alignment and textwidth rather than number of codepoints.

(cherry picked from commit 626acd4)
@KristofferC KristofferC mentioned this pull request Oct 27, 2022
37 tasks
KristofferC pushed a commit that referenced this pull request Oct 28, 2022
Without this, alignment would count characters rather than
textwidth as well as counting inline escape sequences in
colored output. Fix that by using uncolored printing for
alignment and textwidth rather than number of codepoints.

(cherry picked from commit 626acd4)
@mcabbott
Copy link
Contributor

mcabbott commented Nov 1, 2022

Can this be done for Diagonal, etc? They use length(s) at the moment:

function Base.replace_in_print_matrix(A::Diagonal,i::Integer,j::Integer,s::AbstractString)
i==j ? s : Base.replace_with_centered_mark(s)
end

julia/base/arrayshow.jl

Lines 42 to 45 in 8af6731

function replace_with_centered_mark(s::AbstractString;c::AbstractChar = '')
N = length(s)
return join(setindex!([" " for i=1:N],string(c),ceil(Int,N/2)))
end

and get a string from print_matrix_row. So either print_matrix_row has to pass them more information, or we need something like textwidth which strips colour codes.

@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants