Closed davidanthoff closed 2 weeks ago
Why would indexing into a string with the last_byte
make sense? Seems the same conceptually as doing "α"[sizeof("α")]
.
Yeah, I also realized, I should probably be using range
instead of using first_byte
and last_byte
. At least for range
I assume it should return valid indices, right? My PR would fix that, the repo for the problem with range
is:
using JuliaSyntax
src = read(joinpath(homedir(), ".julia/dev/Sunny/test/test_tensors.jl"), String)
tree = parseall(SyntaxNode, src, filename="foo.jl")
code_range = range(tree[2][end][end])
src[code_range]
which also errors.
If anyone else is facing this issue, a way to bypass it until there's a permanent fix is to find your lines ending in non-ASCII Unicode characters and make that no longer the case.
Thanks @DaniGlez!
Here's a regexp (VSCode flavour) for finding non-ASCII characters that end lines:
[^\x00-\x7f]$
there seems to be a fair bit of code that assumes 1-byte characters
JuliaSyntax
works on byte indices internally and it's (usually) quite careful to convert to valid character indices in the occasional cases where this is required.
So unicode generally works and there's a pile of tests for this in test/source_files.jl
However there must be a bug in an edge case for non-ascii at line end
Nice properties of byte ranges are that (a) the green tree exactly covers the source buffer with unit ranges, with no gaps (b) the ranges (in principle) require no further interpreting of unicode chars to pull out a string (not yet exploited in practice! as this bug shows!)
I think all I'm looking for is some function that returns a range that has valid string indices and can be used directly with the underlying string. Does that exist? If I understand @c42f's comment in my PR correct, then Base.range
(to be renamed to byte_range
) is also not supposed to do that, so is there any other way?
As a user of this, I would also say that at the level of SyntaxNodes, users are probably thinking in terms of string indices, not byte indices. I understand that byte stuff makes sense at the green tree level, but isn't that an abstraction leakage if I have to deal with this stuff when dealing with SyntaxNode
s? I think it would be nice if there was a (documented) API at the SyntaxNode
level that just operates on string index level.
For now I'm going to hack around all of this downstream, we can't have this bug crash in the wild, but it would be much nicer if there was some "official" way to get these indices.
I think all I'm looking for is some function that returns a range that has valid string indices and can be used directly with the underlying string. Does that exist?
Not exactly. Here's what exists so far:
SyntaxNode
ranges in conjunction with SourceFile
which does the index conversion (as well as other useful stuff like line and column number computation).sourcetext(node)
.If I understand @c42f's comment in my PR correct, then Base.range (to be renamed to byte_range) is also not supposed to do that
Correct. By design, JuliaSyntax
works in terms of byte ranges and only does the conversion to character indices when necessary.
For now I'm going to hack around all of this downstream
Ok. If you can point me at the place you're using this downstream, I'm sure we can work out the right design to unhack things in the future :-)
My hack is https://github.com/julia-vscode/TestItemDetection.jl/blob/e19b96801a10718ca39b2c15cad18d0aae123993/src/packagedef.jl#L2. Essentially I just want that :)
In terms of design, I generally just guess that "end-users" of this package that use SyntaxNode
want to deal with string indices, not byte indices. From the consumer's point of view, the whole byte index stuff strikes me as an implementation detail that I shouldn't really be exposed to in terms of the API I deal with :)
I saw the SourceFile
type. We have a very similar thing in JuliaWorkspaces, and it is a bit different now than I originally described. Would be nice if we could unify all of that at some point to use the same types.
Essentially I just want that :)
Right - this is pretty much what SourceFile
does here:
Except that SourceFile
also supports a byte offset (for when we are logically only parsing a piece of the text, not the whole thing). By the way, SourceFile
has prevind
, etc, supported.
I feel the core issue here is the question "what's a good representation of source code?" SourceFile
isn't the whole answer, and neither is a string (probably). Essentially we're back to wanting a proper answer to #190 ?
For now I guess a quick fix would be to add a char_range()
function which would do what your our_range()
does? It's not wonderful (not a solution to #190), but it would be an improvement.
For GreenNode
, char_range
would need additional file::SourceFile
and position::Int
arguments. Or some such.
For now I guess a quick fix would be to add a
char_range()
function
Yes, I think that would be great.
For
GreenNode
,char_range
would need additionalfile::SourceFile
andposition::Int
arguments.
I think it would also be reasonable to say that the "char"-based interface is only available at the SyntaxNode
level, if someone is digging down to the green node level, they just have to deal with this themselves? That is the more low level interface in any case.
We have a crash from the VS Code extension that seems to originate from JuliaSyntax. Repo steps are:
pkg> dev Sunny
src = read(joinpath(homedir(), ".julia/dev/Sunny/test/test_tensors.jl"), String)
tree = parseall(SyntaxNode, src, filename="foo.jl")
i = JuliaSyntax.last_byte(tree[2][end][end])
src[i]
ERROR: StringIndexError: invalid index [2142], valid nearby indices [2140]=>'′', [2143]=>'\r'