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.

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

deps: update JuliaSyntax to v1.0.0 #57188

Merged
merged 5 commits into from
Jan 30, 2025

Conversation

inkydragon
Copy link
Member

@inkydragon inkydragon commented Jan 28, 2025

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
@@ -1,4 +1,4 @@
JULIASYNTAXHIGHLIGHTING_BRANCH = main
JULIASYNTAXHIGHLIGHTING_SHA1 = 19bd57b89c648592155156049addf67e0638eab1
JULIASYNTAXHIGHLIGHTING_SHA1 = cdd5803653a0fee6952f4b48e62f07e9381d8676
Copy link
Member Author

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
@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
Member Author

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
Member

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
Member Author

Choose a reason for hiding this comment

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

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

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
Copy link
Member Author

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

@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
Member

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

@inkydragon
Copy link
Member Author

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
Member

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
8 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue: Updating JuliaSyntax.jl
2 participants