From 9c81ce13a1bf76c6c0f836b8811d4727601cb4eb Mon Sep 17 00:00:00 2001 From: Erik Faulhaber <44124897+efaulhaber@users.noreply.github.com> Date: Wed, 14 Feb 2024 14:27:46 +0100 Subject: [PATCH 1/5] Add some comments --- src/nest_utils.jl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/nest_utils.jl b/src/nest_utils.jl index dc458b879..dace0b80a 100644 --- a/src/nest_utils.jl +++ b/src/nest_utils.jl @@ -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[] @@ -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( From 145db2030f5d92840e4ee574d726f3649ca3a9c2 Mon Sep 17 00:00:00 2001 From: Erik Faulhaber <44124897+efaulhaber@users.noreply.github.com> Date: Wed, 14 Feb 2024 14:28:31 +0100 Subject: [PATCH 2/5] Why does that not work? --- src/styles/yas/nest.jl | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/styles/yas/nest.jl b/src/styles/yas/nest.jl index 7f0690a4f..e16726096 100644 --- a/src/styles/yas/nest.jl +++ b/src/styles/yas/nest.jl @@ -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 + # 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 From 0dc032e7263e3ffbd72c0632e05754c78f13c376 Mon Sep 17 00:00:00 2001 From: Dominique Luna Date: Wed, 14 Feb 2024 19:52:07 -0500 Subject: [PATCH 3/5] patch --- src/nest_utils.jl | 13 +++++-------- src/styles/sciml/nest.jl | 3 +-- src/styles/yas/nest.jl | 7 +++++++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/nest_utils.jl b/src/nest_utils.jl index dace0b80a..62fc33973 100644 --- a/src/nest_utils.jl +++ b/src/nest_utils.jl @@ -216,7 +216,7 @@ Finds the optimal placeholders to turn into a newlines such that the length of t """ function find_optimal_nest_placeholders( fst::FST, - start_line_offset::Int, + indent_offset::Int, max_margin::Int, )::Vector{Int} placeholder_inds = findall(n -> n.typ === PLACEHOLDER, fst.nodes) @@ -253,7 +253,7 @@ function find_optimal_nest_placeholders( optinds = find_optimal_nest_placeholders( fst, g, - start_line_offset, + indent_offset, max_margin, last_group = i == length(placeholder_groups), ) @@ -267,7 +267,7 @@ end function find_optimal_nest_placeholders( fst::FST, placeholder_inds::Vector{Int}, - initial_offset::Int, + indent_offset::Int, max_margin::Int; last_group::Bool = false, )::Vector{Int} @@ -292,8 +292,6 @@ function find_optimal_nest_placeholders( end end - # @info "" dp placeholder_inds - N = size(dp, 1) function find_best_segments(s::Int) @@ -332,7 +330,6 @@ function find_optimal_nest_placeholders( return best_split end - fst_line_offset = fst.indent # Calculate best splits for each number of segments s segments = Tuple{Int,Int}[] for s in 1:N @@ -340,9 +337,9 @@ function find_optimal_nest_placeholders( fits = true for (i, s) in enumerate(segments) if i == 1 - fits &= fst_line_offset + dp[first(s), last(s)] <= max_margin + fits &= indent_offset + dp[first(s), last(s)] <= max_margin else - fits &= fst_line_offset + dp[first(s), last(s)] <= max_margin + fits &= indent_offset + dp[first(s), last(s)] <= max_margin end end fits && break diff --git a/src/styles/sciml/nest.jl b/src/styles/sciml/nest.jl index 1b1c8d262..6df6eaf3d 100644 --- a/src/styles/sciml/nest.jl +++ b/src/styles/sciml/nest.jl @@ -97,8 +97,7 @@ function _n_tuple!(ss::SciMLStyle, fst::FST, s::State) false end - optimal_placeholders = - find_optimal_nest_placeholders(fst, start_line_offset, s.opts.margin) + optimal_placeholders = find_optimal_nest_placeholders(fst, fst.indent, s.opts.margin) for i in optimal_placeholders fst[i] = Newline(length = fst[i].len) diff --git a/src/styles/yas/nest.jl b/src/styles/yas/nest.jl index e16726096..cf6110955 100644 --- a/src/styles/yas/nest.jl +++ b/src/styles/yas/nest.jl @@ -49,6 +49,13 @@ function n_call!(ys::YASStyle, fst::FST, s::State) f = n -> n.typ === PLACEHOLDER || n.typ === NEWLINE + indent_offset = s.line_offset + sum(length.(fst[1:2])) + optimal_placeholders = find_optimal_nest_placeholders(fst, indent_offset, s.opts.margin) + + for i in optimal_placeholders + fst[i] = Newline(length = fst[i].len) + end + nodes = fst.nodes::Vector for (i, n) in enumerate(nodes) if i == 3 From ba2d923bc3ee0a2c17c1f2575df577490d57cbf2 Mon Sep 17 00:00:00 2001 From: Dominique Luna Date: Sun, 18 Feb 2024 15:06:50 -0500 Subject: [PATCH 4/5] consider the initial segment offset. This is important for YASStyle --- src/nest_utils.jl | 44 ++++++++++++++++++++++++++++++++-------- src/styles/sciml/nest.jl | 2 +- src/styles/yas/nest.jl | 4 +++- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/nest_utils.jl b/src/nest_utils.jl index 62fc33973..aaeaeb063 100644 --- a/src/nest_utils.jl +++ b/src/nest_utils.jl @@ -212,11 +212,33 @@ function find_all_segment_splits(n::Int, k::Int) end """ -Finds the optimal placeholders to turn into a newlines such that the length of the arguments on each line is as close as possible while following margin constraints. +Finds the optimal placeholders to turn into a newlines such that the length of +the arguments on each line is as close as possible while following margin constraints. + +first_segment_offset: The offset of the first line of the segment. +segment_offset: The offset of the rest of the segments. +max_margin: The maximum margin allowed. + +The offsets are used with paired with segment lengths to determine if the margin is exceeded. +The offsets for the first segment and the rest are typically the same value. + +A scenario in which they are not is YASStyle, where the first argument cannot be nested +and so to determine the "best fit" the `first_segment_offset` is the length up until +the the first placeholder. Example: + +``` +function foo(arg1, arg2, ....) + ^(First Placeholder) + body +end +``` + +length of "function foo(arg1," is the `first_segment_offset`. """ function find_optimal_nest_placeholders( fst::FST, - indent_offset::Int, + first_segment_offset::Int, + segment_offset::Int, max_margin::Int, )::Vector{Int} placeholder_inds = findall(n -> n.typ === PLACEHOLDER, fst.nodes) @@ -253,7 +275,8 @@ function find_optimal_nest_placeholders( optinds = find_optimal_nest_placeholders( fst, g, - indent_offset, + i == 1 ? first_segment_offset : segment_offset, + segment_offset, max_margin, last_group = i == length(placeholder_groups), ) @@ -267,14 +290,15 @@ end function find_optimal_nest_placeholders( fst::FST, placeholder_inds::Vector{Int}, - indent_offset::Int, + first_segment_offset::Int, + segment_offset::Int, max_margin::Int; last_group::Bool = false, )::Vector{Int} # Function to calculate the length of a segment segment_length = (start_idx::Int, end_idx::Int) -> begin - if placeholder_inds[end] == end_idx && last_group + if end_idx == placeholder_inds[end] && last_group sum(length.(fst[start_idx:end])) + fst.extra_margin else sum(length.(fst[start_idx:end_idx-1])) @@ -282,6 +306,7 @@ function find_optimal_nest_placeholders( end n = length(placeholder_inds) + # @info "" placeholder_inds dp = fill(0, n - 1, n - 1) # Initialize the lengths of segments with single placeholders @@ -292,6 +317,7 @@ function find_optimal_nest_placeholders( end end + N = size(dp, 1) function find_best_segments(s::Int) @@ -330,17 +356,19 @@ function find_optimal_nest_placeholders( return best_split end + # @info "" offset dp max_margin # Calculate best splits for each number of segments s segments = Tuple{Int,Int}[] for s in 1:N segments = find_best_segments(s) fits = true for (i, s) in enumerate(segments) - if i == 1 - fits &= indent_offset + dp[first(s), last(s)] <= max_margin + val = if i == 1 + first_segment_offset + dp[first(s), last(s)] else - fits &= indent_offset + dp[first(s), last(s)] <= max_margin + segment_offset + dp[first(s), last(s)] end + fits &= val <= max_margin end fits && break end diff --git a/src/styles/sciml/nest.jl b/src/styles/sciml/nest.jl index 6df6eaf3d..fe3d139ae 100644 --- a/src/styles/sciml/nest.jl +++ b/src/styles/sciml/nest.jl @@ -97,7 +97,7 @@ function _n_tuple!(ss::SciMLStyle, fst::FST, s::State) false end - optimal_placeholders = find_optimal_nest_placeholders(fst, fst.indent, s.opts.margin) + optimal_placeholders = find_optimal_nest_placeholders(fst, fst.indent, fst.indent, s.opts.margin) for i in optimal_placeholders fst[i] = Newline(length = fst[i].len) diff --git a/src/styles/yas/nest.jl b/src/styles/yas/nest.jl index cf6110955..95d72357f 100644 --- a/src/styles/yas/nest.jl +++ b/src/styles/yas/nest.jl @@ -50,7 +50,9 @@ function n_call!(ys::YASStyle, fst::FST, s::State) f = n -> n.typ === PLACEHOLDER || n.typ === NEWLINE indent_offset = s.line_offset + sum(length.(fst[1:2])) - optimal_placeholders = find_optimal_nest_placeholders(fst, indent_offset, s.opts.margin) + first_ph_idx = findfirst(n -> n.typ === PLACEHOLDER, fst.nodes::Vector) + first_line_indent_offset = s.line_offset + sum(length.(fst[1:first_ph_idx])) + optimal_placeholders = find_optimal_nest_placeholders(fst, first_line_indent_offset, indent_offset, s.opts.margin) for i in optimal_placeholders fst[i] = Newline(length = fst[i].len) From faf559350a1459cb3fe82e980f1a2fae1386b81b Mon Sep 17 00:00:00 2001 From: Erik Faulhaber <44124897+efaulhaber@users.noreply.github.com> Date: Mon, 26 Feb 2024 09:31:42 +0100 Subject: [PATCH 5/5] Add best fit for calls with --- src/styles/yas/nest.jl | 40 +++++++++----------------------- test/yas_style.jl | 52 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/styles/yas/nest.jl b/src/styles/yas/nest.jl index 95d72357f..4718ff9a7 100644 --- a/src/styles/yas/nest.jl +++ b/src/styles/yas/nest.jl @@ -1,33 +1,6 @@ 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 - # 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 @@ -49,9 +22,13 @@ function n_call!(ys::YASStyle, fst::FST, s::State) f = n -> n.typ === PLACEHOLDER || n.typ === NEWLINE - indent_offset = s.line_offset + sum(length.(fst[1:2])) + start_line_offset = s.line_offset + indent_offset = start_line_offset + sum(length.(fst[1:2])) first_ph_idx = findfirst(n -> n.typ === PLACEHOLDER, fst.nodes::Vector) - first_line_indent_offset = s.line_offset + sum(length.(fst[1:first_ph_idx])) + first_line_indent_offset = start_line_offset + if first_ph_idx !== nothing + first_line_indent_offset += sum(length.(fst[1:first_ph_idx])) + end optimal_placeholders = find_optimal_nest_placeholders(fst, first_line_indent_offset, indent_offset, s.opts.margin) for i in optimal_placeholders @@ -88,6 +65,11 @@ function n_call!(ys::YASStyle, fst::FST, s::State) nest!(style, n, s) end end + + s.line_offset = start_line_offset + walk(unnest!(style; dedent = false), fst, s) + s.line_offset = start_line_offset + walk(increment_line_offset!, fst, s) end n_curly!(ys::YASStyle, fst::FST, s::State) = n_call!(ys, fst, s) n_ref!(ys::YASStyle, fst::FST, s::State) = n_call!(ys, fst, s) diff --git a/test/yas_style.jl b/test/yas_style.jl index 07b520c9b..ec5edec85 100644 --- a/test/yas_style.jl +++ b/test/yas_style.jl @@ -278,13 +278,13 @@ var = fcall(arg1, arg2, arg3, # comment arg4, arg5)""" @test yasfmt(str_, 4, 80, join_lines_based_on_source = false) == str - @test yasfmt(str_, 4, 29, join_lines_based_on_source = false) == str + @test yasfmt(str_, 4, 30, join_lines_based_on_source = false) == str str = """ var = fcall(arg1, arg2, arg3, # comment arg4, arg5)""" - @test yasfmt(str_, 4, 28, join_lines_based_on_source = false) == str + @test yasfmt(str_, 4, 29, join_lines_based_on_source = false) == str @test yasfmt(str_, 4, 23, join_lines_based_on_source = false) == str str = """ @@ -446,7 +446,8 @@ @testset "issue 321 - exponential inline comments !!!" begin str = """ - scaled_ticks, mini, maxi = optimize_ticks(scale_func(lmin), scale_func(lmax); k_min=4, # minimum number of ticks + scaled_ticks, mini, maxi = optimize_ticks(scale_func(lmin), scale_func(lmax); + k_min=4, # minimum number of ticks k_max=8)""" @test yasfmt(str, 4, 92, whitespace_in_kwargs = false) == str end @@ -501,7 +502,8 @@ @test yasfmt(str_, 4, 22) == str_ str = """ - T[10 20; 30 40; + T[10 20; + 30 40; 50 60; 10 10]""" @@ -772,4 +774,46 @@ @test format_text(str, YASStyle(), variable_call_indent = ["SVector", "test"]) == formatted_str2 end + + @testset "optimal nesting" begin + str = """ + function foo(arg1, arg2, arg3, arg4, arg5) + return body + end + """ + + @test format_text(str, YASStyle(), margin = 42) == str + + fstr = """ + function foo(arg1, arg2, arg3, + arg4, arg5) + return body + end + """ + @test format_text(str, YASStyle(), margin = 41) == fstr + # should be 30? might be a unnesting off by 1 error + @test format_text(str, YASStyle(), margin = 31) == fstr + + fstr = """ + function foo(arg1, arg2, + arg3, + arg4, arg5) + return body + end + """ + @test format_text(str, YASStyle(), margin = 30) == fstr + @test format_text(str, YASStyle(), margin = 24) == fstr + + fstr = """ + function foo(arg1, + arg2, + arg3, + arg4, + arg5) + return body + end + """ + @test format_text(str, YASStyle(), margin = 23) == fstr + @test format_text(str, YASStyle(), margin = 10) == fstr + end end