KristofferC/OhMyREPL.jl

REPL becomes irresponsive for some seconds if StyledStrings is loaded after OhMyREPL

Closed this issue · 13 comments

lmiq commented

This is the issue:

  1. Add OhMyREPL and StyledStrings
  2. load OhMyREPL (using OhMyREPL).
  3. load StyledStrings (using StyledStrings`).

Result:

The REPL is returned normally after loading StyledStrings, but becomes irresponsive for some seconds. In my computer, about 10 seconds. In the example below, the multiple julia REPLs shown at the end appear because I typed "enter" multiple times without any response:

% julia --startup-file=no
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.10.4 (2024-06-04)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(@v1.10) pkg> activate --temp
  Activating new project at `/tmp/jl_n7Dg7r`

(jl_n7Dg7r) pkg> add OhMyREPL, StyledStrings
   Resolving package versions...
    Updating `/tmp/jl_n7Dg7r/Project.toml`
  [5fb14364] + OhMyREPL v0.5.26
  [f489334b] + StyledStrings v1.0.2
....
  [83775a58] + Zlib_jll v1.2.13+1
  [8e850ede] + nghttp2_jll v1.52.0+1
  [3f19e933] + p7zip_jll v17.4.0+2
        Info Packages marked with ⌅ have new versions available but compatibility constraints restrict them from upgrading. To see why use `status --outdated -m`

julia> using OhMyREPL

julia> import StyledStrings

julia>

julia>

julia>

julia>

julia>

julia>

julia>

The issue does not occur in Julia 1.11.

KristofferC commented

I think this is due to the type piracy in StyledStrings (cc @tecosaur)

tecosaur commented

I'd suspect so. To backport as much from the 1.11/StyledStrings behavior as possible, the compat "1.0" release of StyledStrings redefines the generic methods for join and a few other widely-used/highly-invalidating functions.

Perhaps there's a better compromise to be struck between complete comparability and latency/invalidations? If so, I don't have much of an intuition about where to strike it.

X-refs: JuliaLang/StyledStrings.jl#57, JuliaLang/StyledStrings.jl#61

lmiq commented

But why there's a lag after the recompilation apparently finished? Waiting for recompilation is, well, ok, but having an irresponsive REPL is another kind of issue.

tecosaur commented

Without doing testing + profiling: no idea on my end.

KristofferC commented

the compat "1.0" release of StyledStrings redefines the generic methods for join and a few other widely-used/highly-invalidating functions.

Oh boy... That's kind of awful. And 1.10 is an LTS so everyone who happens to start depending on StyledStrings will inject these latency mines for people using the LTS for a long time...

This is not an issue with this package, it should be opened in StyledStrings.

tecosaur commented

Oh boy... That's kind of awful. And 1.10 is an LTS so everyone who happens to start depending on StyledStrings will inject these latency mines into the code for a long time.

Yea, I don't want to have those "latency mines" as you call them either, but I'm having trouble seeing how they can be avoided without compromising backwards compatibility. As always, help would be appreciated.

KristofferC commented

You have to make the code so that it is generic and doesn't have to inject its special behavior into every Vararg{AbstractString} function. Right now, AnnotatedString has anointed itself as the "god string type" that is above everyone else which is why it has to do this overwriting so it can win against everyone else.

But this is completely unscalable and "anti Julian", what if someone else would want to do something similar? Whatever packages that got loaded last would "win" but this behavior would be wholly unpredictable.

tecosaur commented

It's not all Vararg{AbstractString} functions that need to be overwritten, only ones that have blessed the String type specifically (such as join). This wasn't scalable to start with.

tecosaur commented

what if someone else would want to do something similar?

If we wanted join to be generic in terms of supported output string types, it would need to be rewritten to be more generic. In Julia 1.10 and earlier it's specialised on String, now it can produce String and AnnotatedString{String}, supporting any AbstractString would be a separate step again.

To me it doesn't seem as though AnnotatedString has "anointed itself as the 'god string type'" so much as tried to sit next to String in some key cases.

KristofferC commented

To me it doesn't seem as though AnnotatedString has "anointed itself as the 'god string type'" so much as tried to sit next to String in some key cases.

It has because it wins vs all string types and the way it does it is via type piracy and method overwriting. And the generic join method is documented to return a String

You might have the same argument about collect providing Arrays but the solution then is not that everyone goes and overwrites collect so that their AbstractArray is returned if any of the argument is that. Then it should have a better API like JuliaLang/julia#36537.

  • 👍1
tecosaur commented

A generic API that doesn't privilege the String would be interesting, but I don't think I'm up for embarking on that project (or at the very least, not alone).

JeffBezanson commented

The fundamental problem here is that annotations are an incompatible extension of the concept of a string. It's similar to the famous "circle-ellipse problem" in OOP. So far an AbstractString has meant "a sequence of code points". If that's all you know about, you can't handle annotations correctly. So the introduction of AnnotatedString is really about adding that notion to AbstractString, but it's implemented as a new subtype. That is never going to be fully natural, which is what leads to attempted solutions with things like type piracy etc.

The String type is a default. When we're not sure what type to return, we pick that one. However that is ok, because String is in fact able to accurately represent any AbstractString, except maybe not efficiently. Introducing an incompatible extension of the concept means a new type needs to be the default, and code needs to be rewritten. That's the real problem here, not some general concern about whether any type ever gets to be "special".

There is obviously a lot of history of string types and algorithms, and people are familiar with them. So personally I don't think it's realistic to add the idea of annotations and expect everyone to be able to work with it. Strings are just too fundamental and popular to take on this new job. This is a really cool feature so it's unfortunate, but IMO it needs to remain an opt-in feature where annotations can only be preserved when you are careful about it, and AbstractString code can continue working only on codepoint sequences.

tecosaur commented

It's an interesting situation.

In conversation a while ago with Scott Jones about the type hierarchy and AnnotatedString, and one thing I put forwards is that perhaps the conceptually purer way of doing things would be to have some AbstractX supertype (perhaps AbstractText? Strings are more than just textual though), of which AbstactString and AnnotatedString are both subtypes.

A consequence of this though would be that a lot of AbstractString code would want to be replaced with AbstractX. That's quite a lot of churn.

The way I see the compromise (I should probably PR an update to the AnnotatedString docstring to make this clearer, I'll bundle this with some other docstring tweaks that seem sensible) is that AnnotatedString introduces extra semantics compared to AbstactString, but they're treated as deliberately fragile. Any time an AnnotatedString is passed to a function that accepts an AbstractString but doesn't want to think about annotations, those annotations just evaporate.

I think this goes some way to resolving the tension, but there are a bunch of basic functions (e.g. join) where it's nice if annotations are preserved if present, and so I think worth doing so.

Perhaps I'm overly optimistic, but despite the complications I find myself encouraged by the improvements I'm seeing it enable/open the door to, things such as:

  • With JuliaSyntaxHighlighting, pretty display of Expr objects in the REPL
  • With JuliaSyntaxHighlighting, highlighting of Julia code in docstrings/markdown
  • Potentially (PR inflight) avoiding a display bug in a PR to Profile where truncation is blindly applied to a string (currently) containing ANSI sequences
  • Allowing emphasis/style in log printing (e.g. @info styled"you did a {styled:bad}, but you probably want {success:good} instead")
  • The work I've just started on fancier error printing
  • REPL Banner code that can be written just once for the color/nocolour case, with extra niceties like a link to the online Pkg docs (if that PR ever gets merged)

and I expect more to come with time.