Newlines handled differently compared to old parser

[mortenpi]

I am not really sure if this is a bug, or maybe it's a bug in the original parser, but it looks like newlines are handled differently in JuliaSyntax.

This broke Documenter's tests (for some reason it's blacklisted in PkgEval), since we do some exact string checks there for parsed sub-expressions, so the cursor value changes threw those tests off. It's not a problem to handle this with version guards (JuliaDocs/Documenter.jl#2146), but it would be good to know whether this change might change back at some point or not.

Here are some Meta.parse calls in 1.9 vs today's master:

[email protected]> Meta.parse("x += 3\nγγγ_γγγ\nγγγ\n\n", 22; raise=true)
(:γγγ, 29)

[email protected]> Meta.parse("x += 3\nγγγ_γγγ\nγγγ\n\n", 22; raise=true)
(:γγγ, 30)

[email protected]> Meta.parse("x = Int[]\r\n\r\npush!(x, 1)\r\n\n", 14)
(:(push!(x, 1)), 27)

[email protected]> Meta.parse("x = Int[]\r\n\r\npush!(x, 1)\r\n\n", 14)
(:(push!(x, 1)), 28)

[email protected]> Meta.parse("x = Int[]\n\npush!(x, 1)\n\n", 12)
(:(push!(x, 1)), 24)

[email protected]> Meta.parse("x = Int[]\n\npush!(x, 1)\n\n", 12)
(:(push!(x, 1)), 25)

[email protected]> Meta.parse("1 + 1\n2 + 2\n\n", 7)
(:(2 + 2), 13)

[email protected]> Meta.parse("1 + 1\n2 + 2\n\n", 7)
(:(2 + 2), 14)

[email protected]> Meta.parse("1 + 1\n\n\n", 1)
(:(1 + 1), 7)

[email protected]> Meta.parse("1 + 1\n\n\n", 1)
(:(1 + 1), 9)

Slightly more interesting case with a comment line:

[email protected]> Meta.parse("1 + 1\n# comment\n\n", 1)
(:(1 + 1), 7)

[email protected]> Meta.parse("1 + 1\n# comment\n\n", 1)
(:(1 + 1), 18)

[c42f]

Hmm yes the flisp parser is less greedy here with consuming any trailing whitespace.

I don't think there's a real right or wrong here, but being more compatible is better than less. Also it makes sense to not read a (potentially) big chunk of whitespace until the caller asks for it.

So it's worth looking into changing this.

[mortenpi]

This might be related, or maybe not, but I just noticed that the manual build has heaps of errors of the form:

┌ Error: JuliaSyntax parser failed — falling back to flisp!
│   exception =
│    BoundsError: attempt to access empty SubString{String} at index [0]

https://buildkite.com/julialang/julia-master/builds/25269#0188f911-51ec-4803-b48e-9a568e55e706/699-736

Simply running make docs locally should be enough to replicate it. I think these come from some, perhaps somewhat weird Meta.parse invocations by Documenter. If I find some time, I'll try to extract MWEs.

[c42f]

attempt to access empty SubString{String} at index [0]

I think this is unrelated, and should already have been fixed in #317 - we just need to bump the JuliaSyntax version upstream in Julia?

[mortenpi]

I think this is unrelated, and should already have been fixed in #317 - we just need to bump the JuliaSyntax version upstream in Julia?

Yes, it seems like you're right. I set JULIASYNTAX_SHA1 to 747702b, rebuilt Julia and the errors seem to be gone 🙂

[timholy]

This issue is the source of CI failures on CodeTracking (which is the lowest-level package in the debugger & Revise stacks). I can presumably fix the issue in CodeTracking, but if this is something that should be changed here, I thought it best to check.

MWE that mimics the test failure in CodeTracking:

julia 1.9

Does not include the comment that follows the method definition:

julia> stmtstr = """
       plus(a, b) = a + b

       # Issue #81
       f() = nothing
       """
"plus(a, b) = a + b\n\n# Issue #81\nf() = nothing\n"

julia> ex, iend = Meta.parse(stmtstr, 1)
(:(plus(a, b) = begin
          #= none:1 =#
          a + b
      end), 20)

julia> stmtstr[iend:end]   # the comment is still "left over"
"\n# Issue #81\nf() = nothing\n"

julia 1.10

Does include the comment:

julia> ex, iend = Meta.parse(stmtstr, 1)
(:(plus(a, b) = begin
          #= none:1 =#
          a + b
      end), 33)

julia> stmtstr[iend:end]     # the comment got consumed as part of that previous method definition
"f() = nothing\n"

Again, I'm not certain what the right answer is either. Feel free to close this issue and if so I will fix it in CodeTracking.

[timholy]

Is changing this behavior on the table? Especially if #484 will be a breaking change? I could presumably do the work myself, but wondering if @c42f would prefer to leave it the way it is?

[c42f]

Sorry I dropped the ball on this one :(

I think we can just be compatible - if you have the bandwidth to tackle this @timholy that would be amazing. I'm drowning a bit right now 😬