Add annotate! method for AnnotatedIOBuffer #53284
Conversation
tecosaur
added
domain:io
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
force-pushed
the
add-annotate-method-for-annio
branch
from
2a0c9e2
to
8c9d008
Compare
|
Ah ooops, somehow I had the tests fixed locally but the fix managed to escape the commit I pushed. Resolved now. |
The way I'd look at/frame that is that the string of an |
|
Does it have relevance for the hash or equality of the string though? |
vtjnash
added
the
status:merge me
|
Annotations are ignored when compared to other |
|
That violates transitivity of |
|
The behaviour of |
|
IMO it probably should be that an unannotated string is equal to an annotated string iff there aren't any annotations. |
|
agree preferable IMO to retain transitivity one can always strip/empty the annotations before comparing |
|
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? |
inkydragon
removed
the
status:merge me
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. (cherry picked from commit 6e1d062)
Backported PRs: - [x] #53361 <!-- 🤖 [master] Bump the SparseArrays stdlib from c9f7293 to cb602d7 --> - [x] #53300 <!-- allow external absint to hold custom data in `codeinst.inferred` --> - [x] #53342 <!-- Add `Base.wrap` to docs --> - [x] #53372 <!-- Silence warnings in `test/file.jl` --> - [x] #53357 <!-- 🤖 [master] Bump the Pkg stdlib from 6dd0e7c9e to 76070d295 --> - [x] #53373 <!-- fix sysimage-native-code=no option with pkgimages --> - [x] #53333 <!-- More consistent return value for annotations, take 2 --> - [x] #53354 <!-- fix code coverage bug in tail position and `else` --> - [x] #53407 <!-- fix sysimage-native-code=yes option --> - [x] #53388 <!-- Fix documentation: thread pool of main thread --> - [x] #53355 <!-- Fix synchronization issues on the GC scheduler. --> - [x] #53429 <!-- Subtype: skip slow-path in `local_∀_∃_subtype` if inputs contain no ∃ typevar. --> - [x] #53437 <!-- Add debug variant of loader_trampolines.o --> - [x] #53284 <!-- Add annotate! method for AnnotatedIOBuffer --> - [x] #53466 <!-- [MozillaCACerts_jll] Update to v2023-12-12 --> - [x] #53467 <!-- [LibGit2_jll] Update to v1.7.2 --> - [x] #53326 <!-- RFC: when loading code for internal purposes, load stdlib files directly, bypassing DEPOT_PATH, LOAD_PATH, and stale checks --> - [x] #53332 - [x] #53320 <!-- Add `Sys.isreadable, Sys.iswriteable`, update `ispath` --> - [x] #53476 Contains multiple commits, manual intervention needed: - [ ] #53285 <!-- Add update mechanism for Terminfo, and common user-alias data --> Non-merged PRs with backport label: - [ ] #53424 <!-- yet more atomics & cache-line fixes on work-stealing queue --> - [ ] #53408 <!-- task splitting: change additive accumulation to multiplicative --> - [ ] #53403 <!-- Move parallel precompilation to Base --> - [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` --> - [ ] #53391 <!-- Default to the medium code model in x86 linux --> - [ ] #53125 <!-- coverage: count coverage where explicitly requested by inference only --> - [ ] #52694 <!-- Reinstate similar for AbstractQ for backward compatibility -->
Just checked (was about to make a "transitive equality for AnnotatedStrings") PR, but it looks like this is how it works already. The # 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 |
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.
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.
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.