formatting-stack icon indicating copy to clipboard operation
formatting-stack copied to clipboard

duplicated defaults

Open thumbnail opened this issue 5 years ago • 6 comments

Brief

defaults are duped over https://github.com/nedap/formatting-stack/blob/master/src/formatting_stack/defaults.clj, https://github.com/nedap/formatting-stack/blob/master/src/formatting_stack/branch_formatter.clj and https://github.com/nedap/formatting-stack/blob/master/src/formatting_stack/project_formatter.clj with minor (but existing) differences.

Expected behavior

All defaults live in https://github.com/nedap/formatting-stack/blob/master/src/formatting_stack/defaults.clj

Actual behavior

defaults are duped

Suspected cause

differences in configuration between core/project-formatter/branch-formatter

Environment info

<= v4.0.0

Additional links

Thanks to #130 we can now apply the configuration differences needed in project-formatter/branch-formatter to the defaults.

thumbnail avatar Feb 24 '20 08:02 thumbnail

Maybe the protocols could some new methods:

(defprotocol Formatter (default-strategies [_] "The default strategies that this forrmatter tends to need"))

That way the duplication would be killed. One still should be able to override the default behavior.

vemv avatar Feb 24 '20 08:02 vemv

Maybe the protocols could some new methods:

Interesting approach. Some linters still do some filtering themselves (remove edn files, unrequireable files, blacklisting known-bad files, etc.). Maybe it's more suited as part of #37

That way the duplication would be killed.

The simplest approach would be to remove the defaults in project-formatter and branch-formatter, and depend on the defaults in formatting-stack.defaults, overriding the defaults where necessary.

thumbnail avatar Feb 24 '20 10:02 thumbnail

Traditionally I've seen this as a non-issue since "it's just data".

However I'm starting to see the value/need for this, as I've tried to customize heavly one real project. Some parts could be customized as the examples advertise; others needed more knowledge of the f-s impl.

At the same time it's quite crucial to keep f-s simple/easy (for us and for any casual user). I'd be wary of some DSL, hooks, DAG libs, etc

e.g. an API resembling a deep-merge would seem simple:

{:in-background? false
 :overrides {:linters {::kondo/id {:kondo-clj-options {:linters {:cond-else {:level :warning}}}}}
             :formatters {::newlines/id nil ;; remove it
                          ::cljfmt/id {:third-party-indent-specs {`a :defn}}}}}

As you pointed out somewhere, the :id addition enables this sort of stuff.


Prior to any POC, please let's seek agreement + assignment first.

vemv avatar Feb 25 '20 06:02 vemv

On slack we discussed using a map for the configuration, but decided against that because of the implicit ordering it brings. Your current suggestion 'works around' that problem by only changing the config using a map, but a vector upon initialization, right?

Is this a similar approach as initially suggested in #38 ?


I like the overall approach, nice stuff 👍

thumbnail avatar Feb 25 '20 06:02 thumbnail

Your current suggestion 'works around' that problem by only changing the config using a map, but a vector upon initialization, right?

Correct - vectors (:formatters, :linters etc) are handled, and a single hashmap can override specific aspects of those vectors.

Is this a similar approach as initially suggested in #38 ?

Not sure, as I didn't thought #38 much in advance. I had just thought "supdate seems nice"

vemv avatar Feb 25 '20 07:02 vemv

Taking over this one, seems easy and quite important.

vemv avatar Mar 02 '20 17:03 vemv