JuliaLang / StyledStrings.jl

Write with ✨ style ✨ and ease
https://julialang.github.io/StyledStrings.jl/
MIT License
26 stars 9 forks source link

Loading external StyledString package removes markdown highlighting #91

Open KristofferC opened 2 weeks ago

KristofferC commented 2 weeks ago

image

This is when having this package as the active project and loading it.

topolarity commented 1 week ago

Do we have a root cause for this yet?

tecosaur commented 1 week ago

I don't, but I have noticed that with StyledStrings as the active project you can see this in a Julia REPL session:

help?> thing
# Observe working syntax highlighting

julia> using StyledStrings

help?> thing
# Syntax highlighting breaks

julia> using Markdown

help?> thing
# Syntax highlighting works again

Checking Base.loaded_modules_order it seems like two versions of StyledStrings, JuliaSyntaxHighlighting, and Markdown are loaded by doing this:

julia> Base.loaded_modules_order
22-element Vector{Module}:
 Core
 Base
 Main
 FileWatching
 Libdl
 Artifacts
 SHA
 Sockets
 LinearAlgebra
 OpenBLAS_jll
 libblastrampoline_jll
 Random
 Base64
 StyledStrings
 JuliaSyntaxHighlighting
 Markdown
 InteractiveUtils
 Unicode
 REPL
 StyledStrings
 JuliaSyntaxHighlighting
 Markdown

julia> length(unique(Base.loaded_modules_order))
22
topolarity commented 1 week ago

If I had to guess, StyledStrings has some code that is embedding Face types: https://github.com/JuliaLang/StyledStrings.jl/blob/da41b6a8ea4d4e355159e9c791111c445cc6805c/src/styledmarkup.jl#L725

but then getface here is effectively doing a isa Face check: https://github.com/JuliaLang/StyledStrings.jl/blob/da41b6a8ea4d4e355159e9c791111c445cc6805c/src/io.jl#L237

which runs into an issue almost exactly like the one I accidentally created with TOML recently (https://github.com/JuliaLang/Pkg.jl/issues/4017#issuecomment-2377589989)

Basically require_stdlib means that multiple StyledStrings can exist, which means that the _ansi_writer code has to support the Face type for all of the loaded StyledStrings.

topolarity commented 1 day ago

I think I found the issue - The second copy of StyledStrings contains a default "FACES" dictionary (w/o the Markdown faces) and then overrides Base.write(::IO, ::Base.AnnotatedString) so that any old AnnotatedStrings floating about start seeing the "FACES" only from the second loaded copy of StyledStrings. In particular, the AnnotatedString's created by Markdown are suddenly missing their Face definitions.

Mis-behavior like this is really easy to get with require_stdlib + type-piracy, so I've started to document the rules of the game in https://github.com/JuliaLang/julia/pull/56005

The part that matters is:

A stdlib must not access any global state that may differ between stdlib copies in type-pirated methods

Given that guideline, I think we only have a few options here:

The basic problem is that a type-pirated method must be simultaneously correct for all copies of a stdlib.