julia-vscode / DocumentFormat.jl

Auto-formatter for Julia
Other
62 stars 18 forks source link

WIP: add additional passes #26

Closed domluna closed 5 years ago

domluna commented 6 years ago

add for loop pass, rewrites= in for loops to in add docstring pass which normalizes all global docstrings to

"""
doc for f
"""
function f()
    ...
end

the above also simplifies indenting the initial line of the docstring.

davidanthoff commented 6 years ago

rewrites= in for loops to in

Is that a good idea? Using = in for loops is perfectly legit in julia, why would a document formatter change that? I think we should restrict document formatting to essentially white space changes, but nothing else.

domluna commented 6 years ago

Some styles guides advocate for in instead of =

  1. http://www.juliaopt.org/JuMP.jl/latest/style.html
  2. https://github.com/jrevels/YASGuide

It's also more consistent with other languages. Fair enough though, it could be put in a style linter or something like that.

davidanthoff commented 6 years ago

I think having optional, configurable, more invasive and opinionated style formatting would be great! But as a default it seems we should be more conservative.

domluna commented 6 years ago

doc format doesn't work for single quote global docs, i.e.:

"Test for function definition expressions."
isdef(ex) = ismatch(or_(:(function _(__) _ end),
                        :(f_(__) = _)),
                    ex)
ZacLN commented 6 years ago

I agree on the need for this to be optional, could you add a struct that gets passed to the various pass functions? i.e. a struct with a convert_iterator_ops field.

For the above what do you expect the formatter to do?

domluna commented 6 years ago

@ZacLN it should align to

"""
Test for function definition expressions.
"""
isdef(ex) = ismatch(or_(:(function _(__) _ end),
                        :(f_(__) = _)),
                    ex)

which it now does.

domluna commented 6 years ago

Added a for loop option and docstrings with escapes that turn into CSTParser.EXPR{CSTParse.StringH} are correctly formatted. I added methods to CSTParser.str_value to do this, https://github.com/ZacLN/DocumentFormat.jl/pull/26/files#diff-8f8b649677f922163282f3d951be629aR107.

Side note, any objections to having a more functional CSTParser.str_value? Essentially getting closer to CSTParser.str_value(CSTParse.parse(s)) == s.

I've been using https://raw.githubusercontent.com/MikeInnes/MacroTools.jl/master/src/utils.jl to test the output and it formats the entire file now.

formatted output

domluna commented 6 years ago

@ZacLN this should be g2g pending comments.

Side note: I'm working on a width aware formatter in the same vein as http://homepages.inf.ed.ac.uk/wadler/papers/prettier/prettier.pdf in a separate branch.

ZacLN commented 5 years ago

Thans Dom, I'll have a look at this tomorrow

ZacLN commented 5 years ago

I've made a PR to this branch, will merge this after that is merged

domluna commented 5 years ago

@ZacLN thanks, I've merged your changes.