gcv / julia-snail

An Emacs development environment for Julia
GNU General Public License v3.0
235 stars 23 forks source link

Fix CSTParser dep #148

Closed MasonProtter closed 6 months ago

MasonProtter commented 7 months ago

The project is currently set to "~3.3.0", but this unnecessarily restricts CSTParser to a version that's broken in julia v1.11.

Unless there's a very good reason to do so, the preferred thing is to trust that package authors are following semantic versioning, and won't release a v3.4 that breaks existing APIs. In this case, 3.4 adjusts to accommodate the fact that the 3.3 was broken on julia v1.11.

MasonProtter commented 7 months ago

Hm, actually no this does not appear to solve the problem in v1.11. I'm still getting

julia> JuliaSnail.start(10051) ; # please wait, time-to-first-plot...
MethodError(convert, (Tokenize.Lexers.Lexer{Base.GenericIOBuffer{Vector{UInt8}}, Tokenize.Tokens.RawToken}, Tokenize.Lexers.Lexer{IOBuffer, Tokenize.Tokens.RawToken} at position: 0), 0x000000000000675b)
MethodError(convert, (Tokenize.Lexers.Lexer{Base.GenericIOBuffer{Vector{UInt8}}, Tokenize.Tokens.RawToken}, Tokenize.Lexers.Lexer{IOBuffer, Tokenize.Tokens.RawToken} at position: 0), 0x000000000000675b)
ERROR: type Array has no field span
Stacktrace:
 [1] getproperty
   @ ./Base.jl:49 [inlined]
 [2] pathat(cst::Vector{Any}, offset::Int64)
   @ Main.JuliaSnail.CST ~/.emacs.d/straight/repos/julia-snail/JuliaSnail.jl:511
 [3] moduleat(encodedbuf::String, byteloc::Int64)
   @ Main.JuliaSnail.CST ~/.emacs.d/straight/repos/julia-snail/JuliaSnail.jl:556
 [4] forcecompile()
   @ Main.JuliaSnail.CST ~/.emacs.d/straight/repos/julia-snail/JuliaSnail.jl:727
 [5] start(port::Int64; addr::String)
   @ Main.JuliaSnail ~/.emacs.d/straight/repos/julia-snail/JuliaSnail.jl:885
 [6] start(port::Int64)
   @ Main.JuliaSnail ~/.emacs.d/straight/repos/julia-snail/JuliaSnail.jl:857
 [7] top-level scope
   @ REPL[1]:1

where the offending function is

function pathat(cst, offset, pos = 0, path = [(expr=cst, start=1, stop=cst.span+1)])
   if cst !== nothing && !CSTParser.isnonstdid(cst)
      for a in cst
         if pos < offset <= (pos + a.span)
            return pathat(a, offset, pos, [path; [(expr=a, start=pos+1, stop=pos+a.span+1)]])
         end
         # jump forward by fullspan since we need to skip over a's trailing whitespace
         pos += a.fullspan
      end
   elseif (pos < offset <= (pos + cst.fullspan))
      return [path; [(expr=cst, start=pos+1, stop=offset+1)]]
   end
   return path
end

So it appears that something is causing for a in cst to give a::Array...

gcv commented 7 months ago

CSTParser has a long-standing history of breaking its APIs, and its maintainers have not even bothered to acknowledge my request for something as simple as a changelog (https://github.com/julia-vscode/CSTParser.jl/issues/240). I’ll try to fix this breakage soon, but I’m very short on time. Also not sure if newer versions of CSTParser work with LTS versions of Julia, which I’d like Snail to support.

This might be a good time to eliminate Snail’s dependency on CSTParser and switch to tree-sitter.

MasonProtter commented 7 months ago

Ah, I see, that's unfortunate. Yeah, best to just drop CSTParser then. Tree-sitter would be a good option, though the writer of julia-ts-mode has apparently switched to neovim and won't be working on julia-ts-mode, and it doesn't seem that a new maintainer has been found yet: https://github.com/JuliaEditorSupport/julia-emacs/issues/205, so that's maybe also not the best option.

Is JuliaSyntax.jl unable to give the info you need from CSTParser.jl?

gcv commented 7 months ago

I haven't looked at JuliaSyntax.jl in detail, but I imagine it should work, at least for Julia 1.10 and newer. Still, I'd rather retain support for 1.6 for as long as it's an LTS release. This may be a good way to go, but:

I'm pretty sure Snail can use tree-sitter without julia-ts-mode. It just needs tree-sitter itself and the Julia grammar (https://github.com/tree-sitter/tree-sitter-julia). tree-sitter is now standard in Emacs 29 (and I hope can be installed in 27 and 28 from an ELPA repository). Installing the grammar is mildly annoying, but I hope I can either document it sufficiently or automate it. Once it's available, all the questions Snail asks of CSTParser can be answered without leaving Elisp. This will be a huge benefit, not only by eliminating an unstable dependency, but also because Julia startup time will improve.

It'll be great to ditch all Julia-side dependencies for core Snail functionality and leave them to extensions.

gcv commented 7 months ago

I think the fix for Julia 1.11 compatibility has not yet been merged into CSTParser: https://github.com/julia-vscode/CSTParser.jl/pull/385

Once that goes in, I'll bump the CSTParser version dependency in Snail. I don't really trust CSTParser to not break on minor version numbers, though, so plan to keep to the "tilde" version specifier string.

It's still valuable to switch tree-sitter, though. https://github.com/gcv/julia-snail/issues/149

MasonProtter commented 7 months ago

I haven't looked at JuliaSyntax.jl in detail, but I imagine it should work, at least for Julia 1.10 and newer. Still, I'd rather retain support for 1.6 for as long as it's an LTS release. This may be a good way to go, but:

JuliaSyntax.jl works fine on LTS.

❯ julia +1.6 -q
julia> using JuliaSyntax

julia> parsestmt(SyntaxNode, "(x + y)*z", filename="foo.jl")
line:col│ tree                                   │ file_name
   1:1  │[call-i]                                │foo.jl
   1:2  │  [call-i]
   1:2  │    x
   1:4  │    +
   1:6  │    y
   1:8  │  *
   1:9  │  z

julia> VERSION
v"1.6.7"

But more generally, JuliaSnail should maybe be a julia package on the general registry so that users can install whatever specific version of the package / deps work with their julia installation.

gcv commented 6 months ago

But more generally, JuliaSnail should maybe be a julia package on the general registry so that users can install whatever specific version of the package / deps work with their julia installation.

This complicates keeping the Emacs and Julia sides in sync. Right now, mutual compatibility of the Julia and Elisp sides of an julia-snail installation is guaranteed because JuliaSnail.jl is part of the Emacs package. But you're right that the current situation leaves much to be desired. It's yet another reason to switch to tree-sitter and have JuliaSnail.jl have no Julia-side dependencies.

Anyway: it looks like CSTParser 3.4.2 fixes this problem, and I pushed a changed Project.toml. Please let me know if it works.