Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Terminate :statement parsing at newlines #493

Merged
merged 2 commits into from
Aug 17, 2024
Merged

Terminate :statement parsing at newlines #493

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfbff26
Terminate :statement parsing at newlines
timholy Aug 11, 2024
347f5b5
Pick one answer
timholy Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/hooks.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ function core_parser_hook(code, filename::String, lineno::Int, offset::Int, opti
end
parse!(stream; rule=options)
if options === :statement
bump_trivia(stream)
bump_trivia(stream; skip_newlines=false)
if peek(stream) == K"NewlineWs"
bump(stream)
end
end

if any_error(stream)
Expand Down
12 changes: 11 additions & 1 deletion test/hooks.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function _unwrap_parse_error(core_hook_result)
end

@testset "Hooks for Core integration" begin
@testset "whitespace parsing" begin
@testset "whitespace and comment parsing" begin
@test JuliaSyntax.core_parser_hook("", "somefile", 1, 0, :statement) == Core.svec(nothing, 0)
@test JuliaSyntax.core_parser_hook("", "somefile", 1, 0, :statement) == Core.svec(nothing, 0)

Expand All @@ -20,6 +20,16 @@ end

@test JuliaSyntax.core_parser_hook(" x \n", "somefile", 1, 0, :statement) == Core.svec(:x,4)
@test JuliaSyntax.core_parser_hook(" x \n", "somefile", 1, 0, :atom) == Core.svec(:x,2)

# https://github.com/JuliaLang/JuliaSyntax.jl/issues/316#issuecomment-1870294857
stmtstr =
"""
plus(a, b) = a + b

# Issue #81
f() = nothing
"""
@test JuliaSyntax.core_parser_hook(stmtstr, "somefile", 1, 0, :statement)[2] ∈ 19:21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the range 19:21 for?

Copy link
Sponsor Member Author

@timholy timholy Aug 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see that there's a truly unique "right" answer. So I was allowing anything that parses all the code but terminates before the comment starts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh I see :)

If we're choosing a particular somewhat-arbitrary but well-defined behavior I'd prefer to be precise with the test so we can detect any unexpected changes.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I went with 19, which is the earliest this can terminate. I believe this probably matches the flisp parser, but not sure.

I wrote a "wide" test initially because of the annoyances I've experienced over time maintaining tests sensitive to what turn out to be irrelevant details (e.g., whether you add a space between elements when printing tuples). So I've sometimes started to ask, "what behavior do I most care about, and what's the widest test I can write that still enforces that behavior?" But I fully understand where you're coming from here, and it probably makes sense given that this is the origin of such choices rather than the receiver of such choices.

end

@testset "filename and lineno" begin
Expand Down
Loading