Skip to content

Add compat for JuliaSyntax v1#978

Open
svchb wants to merge 7 commits into
JuliaEditorSupport:masterfrom
svchb:update_julia_syntax_v1
Open

Add compat for JuliaSyntax v1#978
svchb wants to merge 7 commits into
JuliaEditorSupport:masterfrom
svchb:update_julia_syntax_v1

Conversation

@svchb
Copy link
Copy Markdown

@svchb svchb commented May 5, 2026

I kept the changes as minimal as possible.

@svchb
Copy link
Copy Markdown
Author

svchb commented May 5, 2026

@domluna all tests pass locally

Copy link
Copy Markdown
Contributor

@pfitzseb pfitzseb left a comment

Choose a reason for hiding this comment

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

Generally LGTM

Comment thread src/styles/default/pretty.jl Outdated
@svchb svchb requested a review from pfitzseb May 18, 2026 13:26
@svchb
Copy link
Copy Markdown
Author

svchb commented May 19, 2026

@penelopeysm can you review this?

@penelopeysm
Copy link
Copy Markdown
Member

I could, but honestly, it would take me a bit of time! I haven't really gotten into the codebase as much as I'd like, and have been quite busy IRL too. If you don't mind waiting, I could try to get to it over the weekend?

@svchb
Copy link
Copy Markdown
Author

svchb commented May 19, 2026

Sure no worries as long as we get this project moved forward again 🚂

Copy link
Copy Markdown
Contributor

@pfitzseb pfitzseb left a comment

Choose a reason for hiding this comment

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

Ok, finally got a chance to take a proper look at this. It would generally be nice to get some more unit tests for the new utility functions if possible.

Comment thread src/fst.jl
Comment on lines +664 to +669
function is_short_function_def(cst::JuliaSyntax.GreenNode)
kind(cst) === K"function" && haschildren(cst) || return false
idx = findfirst(n -> !JuliaSyntax.is_whitespace(n), children(cst))
isnothing(idx) && return false
return kind(cst[idx]) !== K"function"
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
function is_short_function_def(cst::JuliaSyntax.GreenNode)
kind(cst) === K"function" && haschildren(cst) || return false
idx = findfirst(n -> !JuliaSyntax.is_whitespace(n), children(cst))
isnothing(idx) && return false
return kind(cst[idx]) !== K"function"
end
is_short_function_def(cst::JuliaSyntax.GreenNode) = JuliaSyntax.has_flags(cst, JuliaSyntax.SHORT_FORM_FUNCTION_FLAG)

Comment on lines +271 to +272
elseif has_do_block_call(node)
p_do_call(style, node, s, ctx, lineage)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With this, does the

    elseif k === K"do"
        p_do(style, node, s, ctx, lineage)

ever get called? Seems like only having one of them should be enough (and specifically, why can't you re-use the existing case here)?

return nothing
end

function is_source_operator(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In theory this should be covered by the JuliaSyntax.is_operator_* calls no? Would be nice if we didn't have to look into the file for this.

Comment on lines +24 to +46
function source_kind(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)
span(cst) == 0 && return nothing
offset = Int(offset)
n = Int(span(cst))
val = getsrcval(s.doc, offset:(offset+n-1))
try
return JuliaSyntax.Kind(val)
catch
return nothing
end
end

function source_operator_kind(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)
if JuliaSyntax.is_operator(cst) && !haschildren(cst)
return kind(cst)
elseif kind(cst) === K"Identifier" && !haschildren(cst)
k = source_kind(s, cst, offset)
if !isnothing(k) && JuliaSyntax.is_operator(k)
return k
end
end
return nothing
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need this because of JuliaLang/JuliaSyntax.jl#591, right? Wonder whether this will get fixed upstream at some point...

Comment on lines +112 to +134
function source_op_kind(s::State, cst::JuliaSyntax.GreenNode, op_indices::Vector{Int})
opkind = op_kind(cst)
opkind !== K"None" && opkind !== K"Identifier" && return opkind

offset = Int(s.offset)
childs = children(cst)
for (i, c) in enumerate(childs)
if i in op_indices
k = source_operator_kind(s, c, offset)
if !isnothing(k) && !(kind(cst) === K"dotcall" && k === K".")
return k
end
end
offset += Int(span(c))
end
return opkind
end

function update_operator_indices(childs::Vector{JuliaSyntax.GreenNode{T}}) where {T}
args = findall(n -> !JuliaSyntax.is_whitespace(n), childs)
length(args) < 4 && return Int[]
return args[2:(end-1)]
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like the operator should be easy enough to find by checking in which position it occurs (JS.is_[prefix|infix|suffix]_op_call) and then directly accessing that argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants