Skip to content

Best fit arguments with YAS style #809

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

Draft
efaulhaber wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Best fit arguments with YAS style #809

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9c81ce1
Add some comments
efaulhaber Feb 14, 2024
145db20
Why does that not work?
efaulhaber Feb 14, 2024
0dc032e
patch
domluna Feb 15, 2024
ba2d923
consider the initial segment offset. This is important for YASStyle
domluna Feb 18, 2024
faf5593
Add best fit for calls with
efaulhaber Feb 26, 2024
46263c9
Merge branch 'master' into best-fit-yas
efaulhaber Feb 26, 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
9 changes: 8 additions & 1 deletion src/nest_utils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,17 @@ function find_optimal_nest_placeholders(
max_margin::Int,
)::Vector{Int}
placeholder_inds = findall(n -> n.typ === PLACEHOLDER, fst.nodes)

# For 1 or fewer placeholders, all are optimal
if length(placeholder_inds) <= 1
return placeholder_inds
end
newline_inds = findall(n -> n.typ === NEWLINE, fst.nodes)

# Split `placeholder_inds` at newlines.
# The first entry of `placeholder_groups` will contain all placeholders before the first
# newline, the second entry will contain all placeholders between the first and second
# newline, and so on.
newline_inds = findall(n -> n.typ === NEWLINE, fst.nodes)
placeholder_groups = Vector{Int}[]
i = 1
current_group = Int[]
Expand All @@ -241,6 +247,7 @@ function find_optimal_nest_placeholders(

# @info "groups" placeholder_groups

# Pass individual placeholder groups to the function below
optimal_placeholders = Int[]
for (i, g) in enumerate(placeholder_groups)
optinds = find_optimal_nest_placeholders(
Expand Down
27 changes: 27 additions & 0 deletions src/styles/yas/nest.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,33 @@
function n_call!(ys::YASStyle, fst::FST, s::State)
style = getstyle(ys)

has_closer = is_closer(fst[end])
start_line_offset = s.line_offset

optimal_placeholders =
find_optimal_nest_placeholders(fst, start_line_offset, s.opts.margin)

for i in optimal_placeholders
fst[i] = Newline(length = fst[i].len)
end

# placeholder_inds = findall(n -> n.typ === PLACEHOLDER, fst.nodes)
# for (i, ph) in enumerate(placeholder_inds)
# if i == 1 ||
# i == length(placeholder_inds) ||
# (ph < length(fst) && is_comment(fst[ph+1])) ||
# (ph > 1 && is_comment(fst[ph-1]))
# continue
# end
# fst[ph] = Whitespace(fst[ph].len)
# end

# if has_closer && length(placeholder_inds) > 0
# fst[placeholder_inds[end]] = Whitespace(10)
# end
@info "" fst.nodes
@info "" optimal_placeholders
Copy link
Contributor Author

@efaulhaber efaulhaber Feb 14, 2024

Choose a reason for hiding this comment

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

For some reason, this doesn't find any optimal placeholders:

julia> print(format_text(str, YASStyle(), margin = 36))
┌ Info: 
│   fst.nodes =
│    16-element Vector{JuliaFormatter.FST}:
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 3, "foo", nothing, nothing, JuliaFormatter.AllowNest, 0, 10, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, "(", nothing, nothing, JuliaFormatter.AllowNest, 0, 13, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg1", nothing, nothing, JuliaFormatter.AllowNest, 0, 14, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 18, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg2", nothing, nothing, JuliaFormatter.AllowNest, 0, 20, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 24, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg3", nothing, nothing, JuliaFormatter.AllowNest, 0, 26, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 30, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg4", nothing, nothing, JuliaFormatter.AllowNest, 0, 32, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 36, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg5", nothing, nothing, JuliaFormatter.AllowNest, 0, 38, nothing)
└     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ")", nothing, nothing, JuliaFormatter.AllowNest, 0, 42, nothing)
┌ Info: 
└   optimal_placeholders = Int64[]
function foo(arg1, arg2, arg3, arg4,
             arg5)
    return body
end

When I run the same with SciML style, I get

┌ Info: 
│   fst.nodes =
│    18-element Vector{JuliaFormatter.FST}:
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 4, 3, "foo", nothing, nothing, JuliaFormatter.AllowNest, 0, 10, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 4, 1, "(", nothing, nothing, JuliaFormatter.AllowNest, 0, 13, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 4, 0, "", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 4, 4, "arg1", nothing, nothing, JuliaFormatter.AllowNest, 0, 14, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 4, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 18, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 4, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 4, 4, "arg2", nothing, nothing, JuliaFormatter.AllowNest, 0, 20, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 4, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 24, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 4, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 4, 4, "arg3", nothing, nothing, JuliaFormatter.AllowNest, 0, 26, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 4, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 30, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 4, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 4, 4, "arg4", nothing, nothing, JuliaFormatter.AllowNest, 0, 32, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 4, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 36, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 4, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 4, 4, "arg5", nothing, nothing, JuliaFormatter.AllowNest, 0, 38, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 4, 0, "", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
└     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 4, 1, ")", nothing, nothing, JuliaFormatter.AllowNest, 0, 42, nothing)
┌ Info: 
│   optimal_placeholders =
│    1-element Vector{Int64}:
└     12

There are only two extra placeholders with SciML style. One after the opening parenthesis, one before the closing parenthesis. @domluna Why does it find an optimal placeholder there after arg3, but not with the FST above without the two extra placeholders?
I tried looking into find_optimal_nest_placeholders, but it's too complicated.

Copy link
Owner

@domluna domluna Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

de690b9

this seems to work. with yas the indent is different

@efaulhaber

Copy link
Contributor Author

@efaulhaber efaulhaber Feb 15, 2024

Choose a reason for hiding this comment

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

It now works for margin 36,

julia> print(format_text(str, YASStyle(), margin = 36))
┌ Info: 
│   fst.nodes =
│    16-element Vector{JuliaFormatter.FST}:
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 3, "foo", nothing, nothing, JuliaFormatter.AllowNest, 0, 10, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, "(", nothing, nothing, JuliaFormatter.AllowNest, 0, 13, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg1", nothing, nothing, JuliaFormatter.AllowNest, 0, 14, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 18, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg2", nothing, nothing, JuliaFormatter.AllowNest, 0, 20, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 24, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg3", nothing, nothing, JuliaFormatter.AllowNest, 0, 26, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 30, nothing)
│     JuliaFormatter.FST(JuliaFormatter.NEWLINE, -1, -1, 0, 1, "\n", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg4", nothing, nothing, JuliaFormatter.AllowNest, 0, 32, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 36, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg5", nothing, nothing, JuliaFormatter.AllowNest, 0, 38, nothing)
└     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ")", nothing, nothing, JuliaFormatter.AllowNest, 0, 42, nothing)
┌ Info: 
│   optimal_placeholders =
│    1-element Vector{Int64}:
└     11
function foo(arg1, arg2, arg3,
             arg4, arg5)
    return body
end

but not for 38:

julia> print(format_text(str, YASStyle(), margin = 38))
┌ Info: 
│   fst.nodes =
│    16-element Vector{JuliaFormatter.FST}:
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 3, "foo", nothing, nothing, JuliaFormatter.AllowNest, 0, 10, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, "(", nothing, nothing, JuliaFormatter.AllowNest, 0, 13, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg1", nothing, nothing, JuliaFormatter.AllowNest, 0, 14, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 18, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg2", nothing, nothing, JuliaFormatter.AllowNest, 0, 20, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 24, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg3", nothing, nothing, JuliaFormatter.AllowNest, 0, 26, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 30, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg4", nothing, nothing, JuliaFormatter.AllowNest, 0, 32, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ",", nothing, nothing, JuliaFormatter.AllowNest, 0, 36, nothing)
│     JuliaFormatter.FST(JuliaFormatter.PLACEHOLDER, 1, 1, 0, 1, " ", nothing, nothing, JuliaFormatter.AllowNest, 0, -1, nothing)
│     JuliaFormatter.FST(JuliaFormatter.IDENTIFIER, 1, 1, 0, 4, "arg5", nothing, nothing, JuliaFormatter.AllowNest, 0, 38, nothing)
└     JuliaFormatter.FST(JuliaFormatter.PUNCTUATION, 1, 1, 0, 1, ")", nothing, nothing, JuliaFormatter.AllowNest, 0, 42, nothing)
┌ Info: 
└   optimal_placeholders = Int64[]
function foo(arg1, arg2, arg3, arg4,
             arg5)
    return body
end

Copy link
Owner

@domluna domluna Feb 18, 2024

Choose a reason for hiding this comment

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

ok another patch: 6175baa

The initial segment could have a different offset than the other segments. Particularly, this is true for YAS, where it's illegal to put the initial argument on the next line.

Copy link
Owner

@domluna domluna Feb 18, 2024

Choose a reason for hiding this comment

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

@efaulhaber ^

Copy link
Contributor Author

@efaulhaber efaulhaber Feb 26, 2024

Choose a reason for hiding this comment

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

Thanks! Best fit now works for calls with YASStyle. But I broke #807 again...

Copy link
Contributor Author

@efaulhaber efaulhaber Feb 26, 2024

Choose a reason for hiding this comment

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

Hold on, now it works. Probably a Revise issue.

I'll hopefully find time this week to check the failing tests and make this work for n_vect! as well.


# With `variable_call_indent`, check if the caller is in the list
if caller_in_list(fst, s.opts.variable_call_indent) &&
length(fst.nodes::Vector{FST}) > 5
Expand Down