-
Notifications
You must be signed in to change notification settings - Fork 78
SciMLStyle #588
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
Merged
Merged
SciMLStyle #588
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
fdb3111
SciMLStyle
YingboMa cb98f20
Tests
YingboMa d4327cb
More tests
YingboMa 93b07b5
oops
YingboMa 0362b81
Merge branch 'master' into myb/scimlstyle
YingboMa 11540c6
More test
YingboMa 1f23611
Don't break `in`
YingboMa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,8 @@ using CommonMark: | |
| TableRule, | ||
| FrontMatterRule | ||
|
|
||
| export format, format_text, format_file, format_md, DefaultStyle, YASStyle, BlueStyle | ||
| export format, | ||
| format_text, format_file, format_md, DefaultStyle, YASStyle, BlueStyle, SciMLStyle | ||
|
|
||
| abstract type AbstractStyle end | ||
|
|
||
|
|
@@ -96,6 +97,8 @@ include("styles/yas/pretty.jl") | |
| include("styles/yas/nest.jl") | ||
| include("styles/blue/pretty.jl") | ||
| include("styles/blue/nest.jl") | ||
| include("styles/sciml/pretty.jl") | ||
| include("styles/sciml/nest.jl") | ||
|
|
||
| include("nest_utils.jl") | ||
|
|
||
|
|
@@ -549,6 +552,23 @@ function format_text(text::AbstractString, style::AbstractStyle; kwargs...) | |
| return format_text(text, style, opts) | ||
| end | ||
|
|
||
| function format_text(text::AbstractString, style::SciMLStyle; maxiters = 3, kwargs...) | ||
| isempty(text) && return text | ||
| opts = Options(; merge(options(style), kwargs)...) | ||
| # We need to iterate to a fixpoint because the result of short to long | ||
| # form isn't properly formatted | ||
| formatted_text = format_text(text, style, opts) | ||
| iter = 1 | ||
| while formatted_text != text | ||
| iter > maxiters && | ||
| error("formatted_text hasn't reached to a fixpoint in $iter iterations") | ||
| text = formatted_text | ||
| formatted_text = format_text(text, style, opts) | ||
| iter += 1 | ||
| end | ||
| return formatted_text | ||
| end | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That feels quite hacky? Also, not clear how this would interact with a potential partial formatting in an IDE? Wouldn't it make sense to require that a second call to |
||
|
|
||
| function format_text(text::AbstractString, style::AbstractStyle, opts::Options) | ||
| cst, ps = CSTParser.parse(CSTParser.ParseState(text), true) | ||
| line, offset = ps.lt.endpos | ||
|
|
@@ -811,11 +831,15 @@ function parse_config(tomlfile) | |
| end | ||
| end | ||
| if (style = get(config_dict, "style", nothing)) !== nothing | ||
| @assert (style == "default" || style == "yas" || style == "blue") "currently $(CONFIG_FILE_NAME) accepts only \"default\" or \"yas\" or \"blue\" for the style configuration" | ||
| @assert ( | ||
| style == "default" || style == "yas" || style == "blue" || style == "sciml" | ||
| ) "currently $(CONFIG_FILE_NAME) accepts only \"default\" or \"yas\", \"blue\", or \"sciml\" for the style configuration" | ||
| config_dict["style"] = if (style == "yas" && @isdefined(YASStyle)) | ||
| YASStyle() | ||
| elseif (style == "blue" && @isdefined(BlueStyle)) | ||
| BlueStyle() | ||
| elseif (style == "sciml" && @isdefined(SciMLStyle)) | ||
| SciMLStyle() | ||
| else | ||
| DefaultStyle() | ||
| end | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| for f in [ | ||
| :n_call!, | ||
| :n_curly!, | ||
| :n_ref!, | ||
| :n_macrocall!, | ||
| :n_typedcomprehension!, | ||
| :n_typedvcat!, | ||
| :n_tuple!, | ||
| :n_braces!, | ||
| :n_parameters!, | ||
| :n_invisbrackets!, | ||
| :n_comprehension!, | ||
| :n_vcat!, | ||
| :n_bracescat!, | ||
| :n_generator!, | ||
| :n_filter!, | ||
| :n_flatten!, | ||
| :n_using!, | ||
| :n_export!, | ||
| :n_import!, | ||
| :n_chainopcall!, | ||
| :n_comparison!, | ||
| :n_for!, | ||
| #:n_vect! | ||
| ] | ||
| @eval function $f(ss::SciMLStyle, fst::FST, s::State) | ||
| style = getstyle(ss) | ||
| $f(YASStyle(style), fst, s) | ||
| end | ||
| end | ||
|
|
||
| function n_binaryopcall!(ss::SciMLStyle, fst::FST, s::State; indent::Int = -1) | ||
| style = getstyle(ss) | ||
| line_margin = s.line_offset + length(fst) + fst.extra_margin | ||
| if line_margin > s.opts.margin && | ||
| fst.ref !== nothing && | ||
| CSTParser.defines_function(fst.ref[]) | ||
| transformed = short_to_long_function_def!(fst, s) | ||
| transformed && nest!(style, fst, s) | ||
| end | ||
|
|
||
| if findfirst(n -> n.typ === PLACEHOLDER, fst.nodes) !== nothing | ||
| n_binaryopcall!(DefaultStyle(style), fst, s; indent = indent) | ||
| return | ||
| end | ||
|
|
||
| start_line_offset = s.line_offset | ||
| walk(increment_line_offset!, fst.nodes[1:end-1], s, fst.indent) | ||
| nest!(style, fst[end], s) | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| """ | ||
| SciMLStyle() | ||
|
|
||
| Formatting style based on [SciMLStyle](https://github.com/SciML/SciMLStyle). | ||
|
|
||
| !!! note | ||
| This style is still work-in-progress. | ||
|
|
||
| Configurable options with different defaults to [`DefaultStyle`](@ref) are: | ||
| - `whitespace_ops_in_indices` = true | ||
| - `remove_extra_newlines` = true | ||
| - `always_for_in` = true | ||
| - `whitespace_typedefs` = true, | ||
| - `normalize_line_endings` = "unix" | ||
| """ | ||
| struct SciMLStyle <: AbstractStyle | ||
| innerstyle::Union{Nothing,AbstractStyle} | ||
| end | ||
| SciMLStyle() = SciMLStyle(nothing) | ||
|
|
||
| @inline getstyle(s::SciMLStyle) = s.innerstyle === nothing ? s : s.innerstyle | ||
|
|
||
| function options(style::SciMLStyle) | ||
| return (; | ||
| always_for_in = true, | ||
| always_use_return = false, | ||
| annotate_untyped_fields_with_any = true, | ||
| conditional_to_if = false, | ||
| import_to_using = false, | ||
| join_lines_based_on_source = true, | ||
| normalize_line_endings = "unix", | ||
| pipe_to_function_call = false, | ||
| remove_extra_newlines = true, | ||
| short_to_long_function_def = false, | ||
| whitespace_in_kwargs = true, | ||
| whitespace_ops_in_indices = true, | ||
| whitespace_typedefs = true, | ||
| ) | ||
| end | ||
|
|
||
| function is_binaryop_nestable(::SciMLStyle, cst::CSTParser.EXPR) | ||
| (CSTParser.defines_function(cst) || is_assignment(cst)) && return false | ||
| (cst[2].val in ("=>", "->", "in")) && return false | ||
| return true | ||
| end | ||
|
|
||
| const CST_T = [CSTParser.EXPR] | ||
| const TUPLE_T = [CSTParser.EXPR, Vector{CSTParser.EXPR}] | ||
| for f in [ | ||
| :p_import, | ||
| :p_using, | ||
| :p_export, | ||
| :p_invisbrackets, #:p_curly, :p_braces, | ||
| :p_call, | ||
| :p_tuple, | ||
| :p_vcat, | ||
| :p_typedvcat, | ||
| :p_ref, | ||
| :p_comprehension, | ||
| :p_typedcomprehension, #:p_whereopcall, | ||
| :p_generator, | ||
| :p_filter, | ||
| :p_flatten, #:p_vect | ||
| ] | ||
| Ts = f === :p_tuple ? TUPLE_T : CST_T | ||
| for T in Ts | ||
| @eval function $f(ss::SciMLStyle, cst::$T, s::State; kwargs...) | ||
| style = getstyle(ss) | ||
| $f(YASStyle(style), cst, s; kwargs...) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| function p_begin(ss::SciMLStyle, cst::CSTParser.EXPR, s::State) | ||
| style = getstyle(ss) | ||
| t = FST(Begin, cst, nspaces(s)) | ||
| add_node!(t, pretty(style, cst[1], s), s) | ||
| if length(cst) == 2 | ||
| add_node!(t, Whitespace(1), s) | ||
| add_node!(t, pretty(style, cst[end], s), s, join_lines = true) | ||
| else | ||
| stmts_idxs = 2:length(cst)-1 | ||
| # Don't nest into multiple lines when there's only one statement | ||
| if length(stmts_idxs) == 1 | ||
| add_node!(t, Whitespace(1), s) | ||
| add_node!(t, pretty(style, cst[2], s), s, join_lines = true) | ||
| add_node!(t, Whitespace(1), s) | ||
| add_node!(t, pretty(style, cst[end], s), s, join_lines = true) | ||
| else | ||
| s.indent += s.opts.indent | ||
| nodes = CSTParser.EXPR[] | ||
| for i = 2:length(cst)-1 | ||
| push!(nodes, cst[i]) | ||
| end | ||
| add_node!(t, p_block(style, nodes, s), s, max_padding = s.opts.indent) | ||
| s.indent -= s.opts.indent | ||
| add_node!(t, pretty(style, cst[end], s), s) | ||
| end | ||
| end | ||
| t | ||
| end | ||
|
|
||
| function p_macrocall(ys::SciMLStyle, cst::CSTParser.EXPR, s::State) | ||
| style = getstyle(ys) | ||
| t = FST(MacroCall, cst, nspaces(s)) | ||
|
|
||
| args = get_args(cst) | ||
| nest = length(args) > 0 && !(length(args) == 1 && unnestable_node(args[1])) | ||
| has_closer = is_closer(cst[end]) | ||
|
|
||
| !has_closer && (t.typ = MacroBlock) | ||
| nospace = length(2:length(cst)-1) > 1 | ||
|
|
||
| # same as CSTParser.Call but whitespace sensitive | ||
| for (i, a) in enumerate(cst) | ||
| if CSTParser.is_nothing(a) | ||
| s.offset += a.fullspan | ||
| continue | ||
| end | ||
|
|
||
| # Yes: | ||
| # `@parameters a=a b=b` | ||
| # | ||
| # No: | ||
| # `@parameters a = a b = b` | ||
| n = pretty(style, a, s, nospace = nospace) | ||
| if CSTParser.ismacroname(a) | ||
| add_node!(t, n, s, join_lines = true) | ||
| if length(args) > 0 | ||
| loc = cursor_loc(s) | ||
| if t[end].line_offset + length(t[end]) < loc[2] | ||
| add_node!(t, Whitespace(1), s) | ||
| end | ||
| end | ||
| elseif CSTParser.is_comma(a) && i < length(cst) && !is_punc(cst[i+1]) | ||
| add_node!(t, n, s, join_lines = true) | ||
| add_node!(t, Placeholder(1), s) | ||
| elseif is_closer(n) | ||
| add_node!( | ||
| t, | ||
| n, | ||
| s, | ||
| join_lines = true, | ||
| override_join_lines_based_on_source = true, | ||
| ) | ||
| elseif i > 1 && is_opener(cst[i-1]) | ||
| add_node!( | ||
| t, | ||
| n, | ||
| s, | ||
| join_lines = true, | ||
| override_join_lines_based_on_source = true, | ||
| ) | ||
| elseif t.typ === MacroBlock | ||
| if has_closer | ||
| add_node!(t, n, s, join_lines = true) | ||
| if i < length(cst) - 1 && cst[i+1].head != :parameters | ||
| add_node!(t, Whitespace(1), s) | ||
| end | ||
| else | ||
| padding = is_block(n) ? 0 : -1 | ||
| add_node!(t, n, s, join_lines = true, max_padding = padding) | ||
| i < length(cst) && add_node!(t, Whitespace(1), s) | ||
| end | ||
| else | ||
| if has_closer | ||
| add_node!(t, n, s, join_lines = true) | ||
| else | ||
| padding = is_block(n) ? 0 : -1 | ||
| add_node!(t, n, s, join_lines = true, max_padding = padding) | ||
| end | ||
| end | ||
| end | ||
| # move placement of @ to the end | ||
| # | ||
| # @Module.macro -> Module.@macro | ||
| t[1] = move_at_sign_to_the_end(t[1], s) | ||
| t | ||
| end | ||
|
|
||
| function p_unaryopcall(ds::SciMLStyle, cst::CSTParser.EXPR, s::State; kwargs...) | ||
| style = getstyle(ds) | ||
| t = FST(Unary, cst, nspaces(s)) | ||
| if length(cst) == 1 | ||
| if cst.head.fullspan != 0 | ||
| add_node!(t, pretty(style, cst.head, s), s, join_lines = true) | ||
| end | ||
| add_node!(t, pretty(style, cst[1], s), s, join_lines = true) | ||
| else | ||
| add_node!(t, pretty(style, cst[1], s), s) | ||
| add_node!(t, pretty(style, cst[2], s), s, join_lines = true) | ||
| end | ||
| t | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| @testset "SciML Style" begin | ||
| str = raw""" | ||
| @noinline require_complete(m::Matching) = | ||
| m.inv_match === nothing && throw(ArgumentError("Backwards matching not defined. `complete` the matching first.")) | ||
| """ | ||
| formatted_str = raw""" | ||
| @noinline function require_complete(m::Matching) | ||
| m.inv_match === nothing && | ||
| throw(ArgumentError("Backwards matching not defined. `complete` the matching first.")) | ||
| end | ||
| """ | ||
| @test format_text(str, SciMLStyle()) == formatted_str | ||
|
|
||
| str = raw""" | ||
| begin include("hi") end | ||
| """ | ||
| @test format_text(str, SciMLStyle()) == str | ||
|
|
||
| str = raw""" | ||
| begin include("hi"); 1 end | ||
| """ | ||
| formatted_str = raw""" | ||
| begin | ||
| include("hi") | ||
| 1 | ||
| end | ||
| """ | ||
| @test format_text(str, SciMLStyle()) == formatted_str | ||
|
|
||
| str = raw""" | ||
| BipartiteGraph(fadj::AbstractVector, badj::Union{AbstractVector,Integer} = maximum(maximum, fadj); metadata = nothing) = BipartiteGraph(mapreduce(length, +, fadj; init = 0), fadj, badj, metadata) | ||
| """ | ||
| formatted_str = raw""" | ||
| function BipartiteGraph(fadj::AbstractVector, | ||
| badj::Union{AbstractVector, Integer} = maximum(maximum, fadj); | ||
| metadata = nothing) | ||
| BipartiteGraph(mapreduce(length, +, fadj; init = 0), fadj, badj, metadata) | ||
| end | ||
| """ | ||
| @test format_text(str, SciMLStyle()) == formatted_str | ||
|
|
||
| str = raw""" | ||
| @parameters a=b b=c | ||
| """ | ||
| formatted_str = raw""" | ||
| @parameters a=b b=c | ||
| """ | ||
| @test format_text(str, SciMLStyle()) == formatted_str | ||
|
|
||
| str = raw""" | ||
| @parameters a=b | ||
| """ | ||
| formatted_str = raw""" | ||
| @parameters a = b | ||
| """ | ||
| @test format_text(str, SciMLStyle()) == formatted_str | ||
|
|
||
| str = raw""" | ||
| function my_large_function(argument1, argument2, | ||
| argument3, argument4, | ||
| argument5, x, y, z) | ||
| foo(x) + goo(y) | ||
| end | ||
| """ | ||
|
|
||
| formatted_str = raw""" | ||
| function my_large_function(argument1, argument2, | ||
| argument3, argument4, | ||
| argument5, x, y, z) | ||
| foo(x) + goo(y) | ||
| end | ||
| """ | ||
|
|
||
| @test format_text(str, SciMLStyle()) == formatted_str | ||
| end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should just make this part work properly then we wouldn't have to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@YingboMa can you give a minimal example of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just https://github.com/domluna/JuliaFormatter.jl/pull/588/files#diff-60c9ecaf817f34a35b04e83d428e16cd44a2bbf06b642a1fffc8f67d39e2aa9dR2-R12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok made an issue to track it, since this is function is only applicable to SciMLStyle this is fine for now.