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

strings/annotated test relies on that StyledStrings have been loaded #54561

Open
KristofferC opened this issue May 23, 2024 · 3 comments
Open
Assignees
Labels
strings "Strings!"
Milestone

Comments

@KristofferC
Copy link
Member

KristofferC commented May 23, 2024

This is a reduction of a test in test/strings/annotated.jl that avoids loading StyledStrings (which now gets loaded when loading Test.jl):

str1 = Base.AnnotatedString("test", [(1:4, :label => 5)])
str2 = Base.AnnotatedString("case", [(2:3, :label => "oomph")])
@assert join([str1, str1], ' ') ==
    Base.AnnotatedString("test test",
                 [(1:4, :label => 5),
                  (6:9, :label => 5)])

Running this fails with:

ERROR: LoadError: AssertionError: join([str1, str1], ' ') == Base.AnnotatedString("test test", [(1:4, :label => 5), (6:9, :label => 5)])

However, if we add

using StyledStrings

to the top, the assert does not fire.

The reason for this is type piracy in StyledStrings: JuliaLang/StyledStrings.jl#61.

It is weird that things like join on base types give different results depending on if an stdlib has been (transitively) loaded.
The rendering of StyledStrings should probably move to Base to fix this.

@KristofferC KristofferC added the strings "Strings!" label May 23, 2024
@KristofferC KristofferC added this to the 1.12 milestone May 23, 2024
@tecosaur tecosaur self-assigned this May 29, 2024
@tecosaur
Copy link
Contributor

tecosaur commented May 29, 2024

@KristofferC to be clearer about what the plan/hope is here, are you thinking we ship 1.11 as-is, then move the code around in 1.12?

@KristofferC
Copy link
Member Author

I'm coming around thinking that maybe pulling the plug completely on the StyledString stuff in stdlibs is the best way forward. Huge issues when it comes to the design (split between stdlibs and Base) and latency regressions that have been left unchecked for a year. Enough is starting to be enough.

@tecosaur tecosaur reopened this Mar 25, 2025
@tecosaur
Copy link
Contributor

(oops, misclick).

The split is a pain, and makes trying to solve the invalidations a bit of a challenge. I'd very much like to solve this, but I have yet to come up with a clean idea for doing so with the split as it currently is.

With that said, aside from pulling the plug I would very much like to explore whether there's any scope to adjust the base/stdlib split in such a way that we maintain the current functionality but resolve the invalidation issues (with more or less in Base), and/or see if some of the idea's I've been chatting with Cody on-and-off have the potential to make a meaningful dent in invalidations/latency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
strings "Strings!"
Projects
None yet
Development

No branches or pull requests

2 participants