Substrings mask annotations

[LilithHafner]

SubString{Base.AnnotatedString{String}} (produced by annotated_string[1:2]) is an un-annotated string in the sense that it compares equal to strings with the same content without annotations and unequal to strings with annotations.

julia> astr = Base.AnnotatedString("abc", [(1:2, :a => 1)])
"abc"

julia> astr == "abc"
false

julia> astr[1:2] == "ab"
true

julia> astr[1:2] == Base.AnnotatedString("ab", [(1:2, :a => 1)])
false

julia> typeof(astr[1:2])
SubString{Base.AnnotatedString{String}}

[JeffBezanson]

We could patch up the == methods, but it's a bit whack-a-mole --- because of SubString and things like it, any AbstractString can potentially have annotations, so it seems all AbstractString code needs to handle annotations. Oops!

One way to approach it is to try to make AnnotatedString as closed as possible under various operations, e.g. have indexing return an AnnotatedString instead of a SubString. Possibly an AnnotatedString{SubString} but I'm not sure if that would cause problems too?

[JeffBezanson]

One bug here is that annotations(::SubString) is implemented, making it inconsistent with the behavior in the OP. We could remove those methods and say only AnnotatedStrings have annotations, and use AnnotatedString{SubString} instead of SubString{AnnotatedString} whenever possible. What do you think @tecosaur ?

[tecosaur]

Apologies that I've been rather poor at responding to this; I've found this problem somewhat tricky.

For starters, the behaviour Lilith demonstrates is clearly erroneous, and Jeff's idea of using AnnotatedString{SubString} suggested by itself is somewhat appealing, at first glance.

However, I'm concerned that it will cause its own problems around code that uses parametric substring types. Currently calling match(::Regex, ::AnnotatedString{String}) returns a RegexMatch{Base.AnnotatedString{String}}. Why is this a problem? Well, RegexMatch uses SubString{S<:AbstractString} for the match, and I imagine that similar application code probably exists.

I haven't had any nice ideas on how to resolve this tension, beyond just going for the whack-a-mole SubString specialisation as a potential "least bad" option, but none of these options seem to work nicely and I haven't been able to come up with any nice alternatives 😕.

[tecosaur]

From the comments above, I currently don't have a better idea than implementing a specialised equality methods for SubString + AnnotatedString, despite the potentially whack-a-mole nature.

If anybody has a better thought (cc: @JeffBezanson), please do let me know. If I don't hear anything in the near future I'll make a PR doing this. We can always make the solution better/nicer, but making the test case Lilith shows not broken at least seems like a clear improvement even if the mechanism isn't ideal.

[LilithHafner]

SGTM. Fixing bugs is ususally a good first step :)

[KristofferC]

This is (arguably) a bug in the new StyledStrings library but I don't see this being release blocking.

[tecosaur]

I believe the fix is in 1.11-beta2 anyway?

[KristofferC]

All I know is that this issue is still open. Feel free to close if it has been addressed.