Add linters layer.

[geksilla]

Propose to add linters support within :tools/linters or :tools/error-check.

[dvcrn]

I think linter could / should go into core

[sglyon]

Agreed.

I have the start of rust and go layers and they would both rely on linter. It seems odd to include the linter package in those layers as well as its own dedicated layer.

[dvcrn]

On second thought, maybe including linting should be part of each lang layer. Core could implement a linter framework (if one exists) and the layers just plug into that. I think spacemacs is doing that with flycheck

[sglyon]

I'm pretty sure the linter package is the framework and then linters for specific languages hook into it. See the readme: https://atom.io/packages/linter

[dvcrn]

If that is the case then we should definitely put that into core and have linter support be part of the layers. I think that makes the most sense

[sglyon]

Great, it should require zero config, just a matter of adding :linter to the list of core packages and then maybe adding some toggles for turning it on and off

[geksilla]

Agree, linter package is required for atom. I notice about separate layer because we can setup separate Readme, configurations and other stuff. For each :lang layer we can put required linter package and related configuration, and enable linter if user selected this layer in .proton.

For example https://atom.io/packages/linter-clojure can be included in packages for :lang/clojure with appropriate configuration.

In spacemacs there is separate layer called syntax-checking which based on flycheck. For each lang layer they just adds hook to flycheck-mode with appropriate language using function layer-name/post-init-flycheck.

[syl20bnr]

Here how it works in Spacemacs (I'm interested in knowing the differences with the Atom ecosystem) :

The layer syntax-checking is the owner of the package flycheck which is a framework which has support for a bunch of languages and allows to add new one easily. The syntax-checking layer "declares" its ownership over flycheck by defining the init-flycheck function.

A language layer then declares a function post-init-flycheck which is evaluated only and only if an init-flycheck is defined i.e. The syntax-checking layer is enabled i.e. the flycheck package has a owner.

post-init functions are evaluated after the corresponding init function (there is also support for pre-init functions).

In these post-init-flycheck functions each language enable the linters by adding a hook to flycheck-mode-hook.

For completeness, the ownership of flycheck can be stolen by a layer redefining the init-flycheck function.

There are also hooks available for use-package which are advanced topics, feel free to ask by email or on gitter if you want explanations.

So linters can be toggled on and off globally by just adding the syntax-checking layer or not.

[dvcrn]

Oh hi @syl20bnr! Great to see you here! :smile: Thanks for the insights.

The way I understand it is that Linter (which is 'our' flycheck) does the same thing: Just providing a framework for sub-linters to hook in. It seems like Linter is collecting all installed Linters anyway on start so we wouldn't need a post-init hook. Atom is also lazy loaded so until we open a clojure file (in the case of the clojure layer), it wouldn't activate anything.

I guess a linter layer by itself that makes sure that Linter itself is installed and provides toggles for turning it off makes sense then. The question is how we should proceed with separate linters. The linter-clojure for example has a hard dependency on Linter and will force install it if it is not present yet. We don't want that because it would mess with our layer approach and stateful config. If we can somehow to turn this behaviour off, adding it to the clojure layer seems like the best approach. (Maybe we need to think about forking it and patching that behaviour out?)

Is post-init-{{something}} that we want / need as well? It would give layers more control but I think we don't have a usecase for that yet.

[syl20bnr]

The linter-clojure for example has a hard dependency on Linter and will force install it if it is not present yet. We don't want that because it would mess with our layer approach and stateful config.

In Spacemacs those linter addons are guarded with a (when (configuration-layer/package-usedp 'flycheck) ...). Since the flycheck package is treated as used if and only if there is an init-flycheck function defined, using or not using the layer that defines (owns) this init-flycheck acts as a toggle for all the addons.

Is post-init-{{something}} that we want / need as well? It would give layers more control but I think we don't have a usecase for that yet.

You'll surely need it at one point, pre-init is rarely used though but it does not hurt to have them when there is support for post-init.

One important detail: spacemacs only installs packages with a owner which means it installs only packages with a defined init-xxx function. So when the layer linter is not used then there is no init-linter function, then the addons init-xxx functions are not defined (since linter is not considered to be used, see guard above) so they are not considered to be used, in the end nothing is installed.

[dvcrn]

I thought a little bit more about this. So the current problem is:

1. proton initialises all layers
2. once all packages are collected, proton proceeds with installing
3. when linter-clojure (or friends) are installed, they force install linter
4. on next start, if :tools/linter is not enabled, it will remove linter and install it again

Here are some ideas:

1. We might really need a post-init-{{layer}} or something like that. Once the linter layer is enabled, all sub-linters are getting installed and activated. This completely lazy loads the linter package to when the linter layer is enabled. This is probably the cleanest approach but requires the most work. The problem here is though that clojurescript is compiled so we might need some sort of multimethod for each and every layer. This would increase boilerplate code by a good amount. (Correct me please if this is wrong). Maybe we can use @geksilla modes somehow for this?

2. We keep all linter packages disabled and only activate them on some sort of post-init. This solution is a bit easier since we don't need to change the entire lifecycle. Sub-layers just need to activate/deactivate the package accordingly. Though we need to make somehow sure that the package stays disabled even directly after installation. I think adding a package to disabledPackages and installing it afterwards prevents the package from being activated.

3. Even simpler (though hacky) is to let the tools/linter layer broadcast a config key. Something like linter.enabled. Proton collects all configurations first before proceeding with initialisation. The layers could check if linter.enabled exists. if it does, it means that the tools/linter layer is enabled and we just require the language linter packages. If it doesn't, then we can ignore the linter packages. This might be the easiest and fastest approach since all tools we need are already implemented. A variation of this would be to just check if linter is inside the enabled layers and do something based on that. We could pass this vector to the init-layer! function.

Any other thoughts?

[geksilla]

I like the first idea. For now not sure how we can use mode. I was experimenting with multimethods and post-init hook. @dvcrn you can check this one experimental branch. I've found that we can avoid boilerplate code using get-method and check if we have defmethod for associated dispatch value. I wrote this filter function and used it to execute methods for post-init hooks defined inside the layer. Also added package-deps multimethod which can help to manage additional dependencies like linter -> linter-clojure, e.g. we can disable/enable or install/remove linter-clojure package on linter event.

For testing purpose i've added linter package to core layer. For example usage check clojure layer. Thanks.

[dvcrn]

Wow I didn't know you were already this far. I will cherry-pick some of the changes into the branch I'm currently working on and see if I can get this merged today :+1:

[dvcrn]

Just to repeat what I already wrote in https://github.com/dvcrn/proton/pull/56 (so we have it in 1 place):

The problem with this approach is that clojure will just assign the multimethod to the latest defined one so we can't go with that.

I introduced a new function called register-{{layer/package}}-dependencies:

(register-layer-dependencies :tools/linter 
    [:linter-pep8])

It's a little bit more of state management but it basically collects all layers and builds a small dependency map. On package retrieval, it checks which layers/packages are enabled and fetches all sub-dependencies as well.

[syl20bnr]

Not sure it will help but here is in more details how Spacemacs loads:

1) Collect data

• load used layer files defining the functions they contain, this is done in the order of the used layer list in the user dotfile (so a user can put its own layer at the end of the list and take ownership of any package by defining an init function for them)
• build a list of package objects which corresponds to the packages contained in all used layers
  • when a layer defines an init-xxx function this layer is set as the owner of the package, the last layer to do so wins the ownership
  • we gather a list of pre-init and post-init functions for each package
• sort the package list alphabetically

2) Install missing packages

• for each package in the list built previously we check if they are installed
  • if not and the package has a owner then install it
  • if the package has no owner (ie it has only a list of post-init functions) don't install it

3) Configure packages

• for each package with a owner
  • call pre-init functions
  • call the init function
  • call post-init functions

4) Uninstall not used packages

• collect the list of installed packages that are not used
• desinstall them

Important remarks:

• Spacemacs does not load all the layers at startup, it only loads the used ones. Only when the user invokes helm-spacemacs to introspect all the layers Spacemacs loads all of them but without building any new package object.
• The design aims to avoid explicit dependencies between layers and between packages, nowhere there is a list of dependencies, there are only lists of what is used and the layer system resolves all this into a simple flat list of packages ordered alphabetically so it is simple to predict the loading order. This is possible thanks to the eval-after-load macro of elisp which allows to delay a configuration until a file is loaded.
• The design aims for layer isolation as much as possible and this is the most important property of the whole system, this isolation allow us to configure and integrate hundreds of packages while keeping the possibility to exclude any package at will. For instance a package A owned by layer L should never configure something in L that should go in layer L', it means in practice that we list package A in both layers L and L' with an init function in L and post-init function in L'. The idea is that when we look at the list of packages for L' we get the whole picture, we have an exhaustive list of configured packages for this layer, there is nothing configured for L' hidden in other layers. So if we take the simple example of flycheck, the layer syntax-checking will never add the hooks for the language layers, instead we add flycheck in the language layers and hook it in a post-init function, so when one looks at the package list of a given language they can quickly see if flycheck is supported or not (if it is then it will be listed in the list).