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

Best fit arguments with YAS style #809

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

Best fit arguments with YAS style #809

wants to merge 6 commits into from

Conversation

efaulhaber
Copy link
Contributor

No description provided.

efaulhaber
efaulhaber commented Feb 14, 2024
View reviewed changes
src/styles/yas/nest.jl Outdated
Comment on lines 4 to 29
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

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

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

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

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

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

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

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.

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

@domluna domluna domluna left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@efaulhaber @domluna