Have defaults settings depend on the Style (dispatch on Style)

[nickrobinson251]

I'm wondering about how to give styles ability to fully specific the format, while still exposing the full flexibility to users who want to tweak the style to their own preferences.

Right now, there's one format method and one of the keywords is style e.g. style=YASStyle, and then other keywords e.g. always_for_in=true controlling other aspects of the formatting.

We get the situation where to format code following a Style Guide (e.g. YAS Guide) you need to both set a Style (e.g. YASStyle) and set the keywords to the ones that fulfil the style (e.g. pass always_for_in=true).

One small issue with this, is it leads to a slightly awkward API (in my opinion), where passing in a Style is not sufficient to have the code formatted in line with the corresponding style guide (since you need to set keywords too).

A "work-around" for this is to use a config file. (Config files are a very useful feature, in my opinion, but for the use-case where you want to exactly follow a style guide, i think we may be able to get an even nicer API without requiring config files, which are still useful for other use-cases.)

A second small issue, since style is a keyword / there's one format method, all the other keyword are the same no matter the style (i.e. same keywords always_for_in, same defaults always_for_in=false), which means we cannot have style-specific keywords (unclear yet if that's flexibility we'd want)

A suggestion for how to improve this situation is to (i) dispatch on Style, and maybe (ii) have Style constructors own the keywords

---

e.g. with option (i)

function format(path, style::YasStyle; always_for_in=true, etc...)
    # code
end

function format(path, style::DefaultStyle; always_for_in=false, etc...)
    # code
end

format(path; style=DefaultStyle(), kwargs...) = format(path, style; kwargs...)   # (thanks Eric for suggesting this below)

then call like

# to get "full" YAS style
format("path/to/file", YASStyle())

# to "customise" YAS e.g.
format("path/to/file", YASStyle(); always_for_in=false)

---

e.g. with both option (i) and option (ii) as well

function format(path, style::YasStyle)
    # code
end

function format(path, style::DefaultStyle)
    # code
end

format(path; kwargs...) = format(path, DefaultStyle(kwargs...))  # maybe

then call like

# to get "full" YAS style
format("path/to/file", YASStyle())

# to "customise" YAS e.g.
format("path/to/file", YASStyle(always_for_in=false))

# possible to have a YAS specific option e.g.
format("path/to/file", YASStyle(always_for_in=false, use_pipes_to_annoy_alex=true))

FWIW, while most of the benefit comes from doing (i), doing both options (i) and (ii) makes most sense to me (and while it's a breaking change to do (ii), it does also allow us to keep the one-arg format(path; kwargs...) method if we want, by passing kwargs to DefaultStyle).

(This issue is kinda related to #212)

---

As an aside: I think it's still a little undecided which settings are controlled by Styles e.g. YASStyle, versus via keywords e.g. always_for_in=true, but "what gets it's own keyword?" seems a different question to the one i'm raising here, which is about Styles being able to fully specify the format (without losing current flexibility)

[nickrobinson251]

This was also raised by @ericphanson in the context of YASStyle #198 (comment)

[ericphanson]

Just wanted to +1 this and mention @ararslan also agreed that just specifying the style should be enough to have compliance with the associated style guide (well, compliance to the extent that is supported/implemented so far).

One other comment is that option (1) can be done in a non-breaking way by also using a method like

format(path; style=DefaultStyle(), kwargs...) = format(path, style; kwargs...)

instead of

format(path; kwargs...) = format(path, DefaultStyle(); kwargs...)

[nickrobinson251]

Great suggestion! I've added that improvement to option (i) above 😺

I think option (i) is a pretty clear win (especially as it's non-breaking!).

And we can always consider (ii) in future if/when we want style-specific keywords?

[nickrobinson251]

@mzgubic just requested this over at invenia/NamedDims.jl#138 (comment) too

[nickrobinson251]

I will work on a PR for this -- i don't think it'll need too much :)

[ericphanson]

thanks for taking this on @nickrobinson251! I'm glad to see the feature :)