Add annotate! method for AnnotatedIOBuffer

[tecosaur]

The annotate! function provides a convenient way of adding annotations to an AnnotatedString/AnnotatedChar without accessing any of the implementation details of the Annotated* types.

When AnnotatedIOBuffer was introduced, an appropriate annotations method was added, but annotate! was missed. To correct that, we refactor the current annotate! method for AnnotatedString to a more general helper function _annotate! that operates on the annotation vector itself, and use this new helper method to easily provide an annotate! method for AnnotatedIOBuffer.

[tecosaur]

Ah ooops, somehow I had the tests fixed locally but the fix managed to escape the commit I pushed. Resolved now.

[tecosaur]

I do still find it strange that there is a mutation API for one particular AbstractString type, though not new here.

The way I'd look at/frame that is that the string of an AnnotatedString is still immutable, but the annotations layer sits on top of that, and while mutable has no bearing on the actual content of the string.

[vtjnash]

Does it have relevance for the hash or equality of the string though?

[tecosaur]

Annotations are ignored when compared to other AbstractStrings, but == on two AnnotatedStrings does also compare the annotations. IIRC there was some discussion on how equality should work in the original PR, though I forget the details at this point.

[vtjnash]

That violates transitivity of == though, which is not mandatory, but is usually expected

[tecosaur]

The behaviour of == with AnnotatedStrings is something I found difficult to work out, I'm personally still open to being convinced it should be done differently, it doesn't seem clear-cut to me.

[oscardssmith]

IMO it probably should be that an unannotated string is equal to an annotated string iff there aren't any annotations.

[adienes]

agree preferable IMO to retain transitivity

one can always strip/empty the annotations before comparing

[fingolfin]

I also agree with @oscardssmith's proposal. Transitivity is not just nice, but in my experience failure to adhere to it can lead to really weird and difficult to debug situations, e.g. when using strings as key in dictionaries.

But that's not a blocker for this PR which does not modify hashing / equality code, does it?

[tecosaur]

IMO it probably should be that an unannotated string is equal to an annotated string iff there aren't any annotations.

Just checked (was about to make a "transitive equality for AnnotatedStrings") PR, but it looks like this is how it works already. The cmp implementation may need tweaking though.

# Avoid the iteration fallback with comparison
cmp(a::AnnotatedString, b::AbstractString) = cmp(a.string, b)
cmp(a::AbstractString, b::AnnotatedString) = cmp(a, b.string)
# To avoid method ambiguity
cmp(a::AnnotatedString, b::AnnotatedString) = cmp(a.string, b.string)

==(a::AnnotatedString, b::AnnotatedString) =
    a.string == b.string && a.annotations == b.annotations

==(a::AnnotatedString, b::AbstractString) = isempty(a.annotations) && a.string == b
==(a::AbstractString, b::AnnotatedString) = isempty(b.annotations) && a == b.string