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.

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

Streamlining the ecosystem #559

Open
davidanthoff opened this issue Mar 12, 2022 · 11 comments
Open

Streamlining the ecosystem #559

davidanthoff opened this issue Mar 12, 2022 · 11 comments

Comments

@davidanthoff
Copy link
Contributor

Here is a list of ways that end-users might want to use JuliaFormatter.jl functionality:

  1. Directly via an API from a package
  2. From an editor
  3. GitHub actions for CI
  4. CLI

In terms of options for each of these we have the following, I think:

  1. The API in JuliaFormatter.jl or the DocumentFormat.jl API (I made DocumentFormat.jl a shim for JuliaFormatter yesterday).
  2. Essentially anything that uses the LS. The separate formatting VS Code extension was removed from the marketplace.
  3. https://github.com/julia-actions/julia-format has instructions on how to use JuliaFormatter.jl manually, but is not actually an action. https://github.com/julia-actions/julia-codeformat and https://github.com/julia-actions/julia-codeformat-lint are proper GitHub actions and are using JuliaFormatter under the hood now, and any package that is managed by the package butler is using those.
  4. Nothing, really, I think?

I think there are two aspects we could improve: a) there is a fair bit of duplication, and b) defaults are very different depending on which of these one uses. I think both aspects must be confusing for users.

I think the thorniest issue is the question of defaults. Right now, if a user has a repo without a .JuliaFormatter config file and formats each file with the LS, and then uses the instructions in https://github.com/julia-actions/julia-format or uses JuliaFormatter.jl directly, these different entry points will reformat files in different styles. The root cause is that the default in JuliaFormatter.jl is an opinionated style, and in the VS Code extension use a minimal style as the default.

One radical way to resolve this would be to adopt the current VS Code extension as the default everywhere, in particular also as the default in JuliaFormatter.jl. The benefit of that would be that we could get rid of all duplication and users would get a very predictable and simple experience. So, in this scenario, if someone runs JuliaFormatter.format_file on a folder or file, they would always get exactly the same result that they get if the run format document in the extension. Same thing for the GitHub Actions. In this scenario we would rearchive DocumentFormat.jl again, and I would suggest we turn https://github.com/julia-actions/julia-format into a proper GitHub action that is essentially identical to what https://github.com/julia-actions/julia-codeformat is right now, create a new GitHub action julia-format-lint that is identical to https://github.com/julia-actions/julia-codeformat-lint, change the package butler ecosystem to use those two actions and archive https://github.com/julia-actions/julia-codeformat and https://github.com/julia-actions/julia-codeformat-lint. The (major!) downside of this strategy is that it would be a massively breaking change for JuliaFormatter.jl, and I'm generally not sure whether @domluna wants to go down that road :) It would essentially change JuliaFormatter.jl form an opinionated-by-default formatter to a opt-in-opinionated formatter, and that might really not be the vision for the package.

If we don't want to do that, then I do think we need to have options for 1) and 3) that give users the same defaults that they get from the VS Code extension. I kind of created that yesterday, i.e. DocumentFormat.jl is now a shim around JuliaFormatter.jl, but with the extension defaults, and the existing GitHub Actions are also using those defaults. In that scenario, we would essentially have LS, DocumentFormat.jl and julia-codeformat and julia-codeformat-lint as the "recommended" ways to do code formatting if one lives in the LS ecosystem, and then a set of alternative solutions with JuliaFormatter.jl and julia-format that come with different defaults.

@domluna and @pfitzseb any thoughts? Other options that I didn't think of?

Oh, and about 4), the CLI. I could imagine at some (very) future point in time that juliaup ships with a formatting CLI... Or maybe if we have JuliaLang/juliaup#82 it could be used for that. In general we can probably kick that question down the road?

@domluna
Copy link
Owner

domluna commented Mar 12, 2022

The CLI is outdated right now https://github.com/domluna/JuliaFormatter.jl/blob/master/bin/format.jl

Personally I think a CLI would be wonderful but it would need to leverage PackageCompiler.jl or StaticCompiler.jl (might not work as of yet). https://comonicon.org/dev/ might do something along these lines for CLIs might I could have misinterpreted that part.

@domluna
Copy link
Owner

domluna commented Mar 12, 2022

It would essentially change JuliaFormatter.jl form an opinionated-by-default formatter to a opt-in-opinionated formatter, and that might really not be the vision for the package.

An alternative would be to create a MinimalStyle style and then vscode could set just set

style = "minimal"

in a .JuliaFormatter.toml. For that we need a checklist of what we need to change for that. Depending on how pedantic we want to be it could be a bit tedious. I think currently you have options set that get you close to this but it's unlikely to be a doable by just leveraging options, at least right now, and some choices may not be suitable to be turned into an option. Which is why a custom style may be necessary.

@davidanthoff
Copy link
Contributor Author

But we would still be stuck with a situation where the different user facing pieces behave differently if there is no .JuliaFormatter.toml, right?

@davidanthoff
Copy link
Contributor Author

Actually, PkgDev.jl also exposes DocumentFormat.jl, completely forgot about that. So just one more piece, but probably not too important here, we can just make sure that PkgDev.format always does the same thing as the extension.

@domluna
Copy link
Owner

domluna commented Mar 12, 2022

But we would still be stuck with a situation where the different user facing pieces behave differently if there is no .JuliaFormatter.toml, right?

I suppose since it would default to DefaultStyle and the default option settings. I'm not strictly opposed to the breaking change of switching the default style to the minimal style but I'm worried it would be very confusing for users.

@davidanthoff
Copy link
Contributor Author

I'm worried it would be very confusing for users.

Yep, it would... Do you have a sense how people are using JuliaFormatter? Is it primarily directly, or are most using it via some editor integration?

I just realized, that even with a .JuliaFormatter.toml, in many cases will get different results depending on how they start the formatting, I think. Essentially, unless they actually explicitly specify a style in the config file, defaults will still play a role, even with a config file, right?

@domluna
Copy link
Owner

domluna commented Mar 13, 2022

I just realized, that even with a .JuliaFormatter.toml, in many cases will get different results depending on how they start the formatting, I think. Essentially, unless they actually explicitly specify a style in the config file, defaults will still play a role, even with a config file, right?

right it's a slightly bigger issue because the minimal style will have different options than the default style and so unless they have default style explicitly set in the format settings file a bunch of the options they don't explicitly touch will change as well.

@domluna
Copy link
Owner

domluna commented Mar 14, 2022

didn't quite explain it well enough in my last comment

function options(style::YASStyle)
    return (;
        always_for_in = true,
        whitespace_ops_in_indices = true,
        remove_extra_newlines = true,
        import_to_using = true,
        pipe_to_function_call = true,
        short_to_long_function_def = true,
        always_use_return = true,
        whitespace_in_kwargs = false,
        join_lines_based_on_source = true,
    )
end

defaults

Base.@kwdef struct Options
    indent::Int = 4
    margin::Int = 92
    always_for_in::Union{Bool,Nothing} = false
    whitespace_typedefs::Bool = false
    whitespace_ops_in_indices::Bool = false
    remove_extra_newlines::Bool = false
    import_to_using::Bool = false
    pipe_to_function_call::Bool = false
    short_to_long_function_def::Bool = false
    always_use_return::Bool = false
    whitespace_in_kwargs::Bool = true
    annotate_untyped_fields_with_any::Bool = true
    format_docstrings::Bool = false
    align_struct_field::Bool = false
    align_assignment::Bool = false
    align_conditional::Bool = false
    align_pair_arrow::Bool = false
    conditional_to_if::Bool = false
    normalize_line_endings::String = "auto"
    align_matrix::Bool = false
    join_lines_based_on_source::Bool = false
    trailing_comma::Union{Bool,Nothing} = true
    indent_submodule::Bool = false
end

So even if they had the style explicitly set to "yas" (in this case), if options not covered by the YAS options are changed then the format result could unintentionally change as well.

This can be easily solved by encapsulating all the options in the options function for each style.

@davidanthoff
Copy link
Contributor Author

This can be easily solved by encapsulating all the options in the options function for each style.

Yes, that sounds like a good idea no matter what we do about the defaults!

If we were to go the radical route and change the defaults here in JuliaFormatter, here is an idea how we could make the transition a little less horrible for existing users of JuliaFormatter: for a while (say a couple of months), we could change the format* functions to require a style argument, i.e. essentially remove the ability to call these functions without specifying a style. And then, after a couple of months, we could reenable the functionality to call them without an explicit style argument, and make the minimal style the default. The primary benefit of that would be that at least the user experience would be "hey, we made a breaking change and you need to change how you call format", rather than a new release that all of a sudden just formats things entirely differently without a warning. Still not great, but maybe a little less painful?

@davidanthoff
Copy link
Contributor Author

@domluna and @pfitzseb I'm wondering how we can somehow resolve this?

@davidanthoff
Copy link
Contributor Author

davidanthoff commented May 31, 2022

We had a very constructive call on this, and @pfitzseb wrote down the following roadmap in chat:

JuliaFormatter 1.x

  • Non-default styles cannot rely on default style at all - Set all options in each style #605
  • Show warning for JuliaFormatter.format without a style
  • Finish MinimalStyle implementation - Add MinimalStyle #606
  • Find new name for JuliaFormatter1 default style
  • Maybe automatically write style="JuliaFormatter1Default" into existing tomls without an explicit style

JuliaFormatter 2.x

  • Error for JuliaFormatter.format without a style

JuliaFormatter 3.x

  • Make MinimalStyle the new default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants