Skip to content

deps: update JuliaSyntax to v1.0.0#57188

Merged
KristofferC merged 5 commits intoJuliaLang:masterfrom
inkydragon:cyhan/update-JuliaSyntax
Jan 30, 2025
Merged

deps: update JuliaSyntax to v1.0.0#57188
KristofferC merged 5 commits intoJuliaLang:masterfrom
inkydragon:cyhan/update-JuliaSyntax

Conversation

@inkydragon
Copy link
Copy Markdown
Member

@inkydragon inkydragon commented Jan 28, 2025 •
edited
Loading

This pr:

  • Update JuliaSyntax to v1.0.0
  • Update JuliaSyntaxHighlighting to latest

Note: JuliaSyntax.jl contains some breaking changes in v1.0.0,
so we need to update JuliaSyntaxHighlighting.jl at the same time.

Close #57166

@inkydragon inkydragon added stdlib Julia's standard library DO NOT MERGE Do not merge this PR! labels Jan 28, 2025
@inkydragon inkydragon mentioned this pull request Jan 28, 2025
Closed
3 tasks
inkydragon
inkydragon commented Jan 28, 2025
View reviewed changes
stdlib/JuliaSyntaxHighlighting.version Outdated
@@ -1,4 +1,4 @@
JULIASYNTAXHIGHLIGHTING_BRANCH = main
JULIASYNTAXHIGHLIGHTING_SHA1 = 19bd57b89c648592155156049addf67e0638eab1
JULIASYNTAXHIGHLIGHTING_SHA1 = cdd5803653a0fee6952f4b48e62f07e9381d8676
Copy link
Copy Markdown
Member Author

@inkydragon inkydragon Jan 28, 2025

Choose a reason for hiding this comment

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

The current commit only exists in my PR for JuliaSyntaxHighlighting.jl.
(JuliaLang/JuliaSyntaxHighlighting.jl#9)

So do not merge.

@inkydragon inkydragon added this to the 1.12 milestone Jan 28, 2025
@inkydragon inkydragon force-pushed the cyhan/update-JuliaSyntax branch 2 times, most recently from 8ccddea to 54e1520 Compare January 29, 2025 06:03
inkydragon
inkydragon commented Jan 29, 2025
View reviewed changes
test/syntax.jl
@test Meta.lower(Main, :(a.[1])) == Expr(:error, "invalid syntax \"a.[1]\"")
@test Meta.lower(Main, :(a.{1})) == Expr(:error, "invalid syntax \"a.{1}\"")
@test_broken Meta.lower(Main, :(a.[1])) == Expr(:error, "invalid syntax \"a.[1]\"")
@test_broken Meta.lower(Main, :(a.{1})) == Expr(:error, "invalid syntax \"a.{1}\"")
Copy link
Copy Markdown
Member Author

@inkydragon inkydragon Jan 29, 2025

Choose a reason for hiding this comment

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

Seems a regression

Copy link
Copy Markdown
Member

@KristofferC KristofferC Jan 29, 2025

Choose a reason for hiding this comment

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

Maybe good with an issue at JuliaSyntax.jl so there is something tracking this.

Copy link
Copy Markdown
Member Author

@inkydragon inkydragon Jan 30, 2025

Choose a reason for hiding this comment

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

JuliaLang/JuliaSyntax.jl#532

@inkydragon inkydragon force-pushed the cyhan/update-JuliaSyntax branch from 54e1520 to 58a2047 Compare January 29, 2025 14:06
@inkydragon
Copy link
Copy Markdown
Member Author

inkydragon commented Jan 29, 2025

Not sure if this is a bug or just need to update the test case.

  LoadError: syntax: "(Expr(:escape, Expr(:tuple, Expr(:..., :x))),)" is not a valid function argument name around /cache/build/tester-demeter6-13/julialang/julia-master/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:676

https://buildkite.com/julialang/julia-master/builds/44198#0194b263-5ca4-4277-a75d-13b3c193d6bb/884-1701

julia/test/syntax.jl

Lines 2456 to 2459 in f209eba

macro n37134()
:($(esc(Expr(:tuple, Expr(:..., :x))))->$(esc(:x)))
end
@test @n37134()(2,1) === (2,1)

Test case add in #37534

Seems there is something wrong with esc,
Maybe update this test to:

macro m1()
    quote
        ((x...,)) -> (x)
    end |> esc
end

macro m2()
    quote
        (x...) -> (x)
    end |> esc
end

Those two fixes work for me.

@inkydragon inkydragon force-pushed the cyhan/update-JuliaSyntax branch from 58a2047 to 601e395 Compare January 30, 2025 07:49
@inkydragon inkydragon mentioned this pull request Jan 30, 2025
Open
@inkydragon
Copy link
Copy Markdown
Member Author

inkydragon commented Jan 30, 2025

PR now passes all tests (with a few new broken tests added)

@inkydragon inkydragon mentioned this pull request Jan 30, 2025
Merged
@inkydragon inkydragon marked this pull request as ready for review January 30, 2025 14:09
@KristofferC KristofferC added DO NOT MERGE Do not merge this PR! and removed DO NOT MERGE Do not merge this PR! labels Jan 30, 2025
@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Jan 30, 2025

Looks good, are the JuliaSyntaxHighlighting commits on a stable place now so this can be merged?

@inkydragon
Copy link
Copy Markdown
Member Author

inkydragon commented Jan 30, 2025

are the JuliaSyntaxHighlighting commits on a stable place

Maybe not.
This PR use is a commit from a temporary branch in my forked repo and is therefore unstable.
To get a stable commit hash, we need to merge JuliaLang/JuliaSyntaxHighlighting.jl#9

@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Jan 30, 2025

Let's do the strategy in JuliaLang/JuliaSyntaxHighlighting.jl#9 (comment) (merge this, then merge that PR and change the commit).

@KristofferC KristofferC removed the DO NOT MERGE Do not merge this PR! label Jan 30, 2025
@KristofferC KristofferC merged commit 3e08cb1 into JuliaLang:master Jan 30, 2025
@longemen3000 longemen3000 mentioned this pull request Jan 31, 2025
Closed
@inkydragon inkydragon deleted the cyhan/update-JuliaSyntax branch February 7, 2025 03:13
@nsajko nsajko added the parser Language parsing and surface syntax label Aug 25, 2025
@fingolfin fingolfin mentioned this pull request Oct 27, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KristofferC KristofferC KristofferC left review comments

Assignees

No one assigned

Labels

dependencies parser Language parsing and surface syntax stdlib Julia's standard library

Projects

None yet

Milestone

1.12

Development

Successfully merging this pull request may close these issues.

Tracking Issue: Updating JuliaSyntax.jl

3 participants

@inkydragon @KristofferC @nsajko