Fork me on GitHub
#proton
<
2016-01-05
>
dvcrn01:01:48

finally back in the office

dvcrn01:01:55

time to get some stuff done simple_smile

sglyon01:01:42

Nice, looking forward to it

sglyon01:01:53

I still have a couple layers to push out of my .proton

dvcrn02:01:24

go for it!

dvcrn06:01:50

@geksilla, @sglyon I pushed some changes that disable debug messages by default. Go into proton/config/proton.cljs and just set debug to true to get it back

sglyon06:01:15

Thanks for the heads up

sglyon06:01:00

@dvcrn: how happy are you with how we implemented the linter system?

dvcrn06:01:47

I think in general the 'optional package' system (layers enabling / disabling packages) is not good yet

dvcrn06:01:09

I also noticed that proton installed javascript linters for me even though I don't use the js layer

sglyon06:01:08

Hmm that’s odd.

sglyon06:01:27

Actually now that I think of it I saw that too — I just assumed I had the javascript layer on

dvcrn06:01:48

yeah it seems like there is no check if the actual layer is enabled. It just pulls in all dependencies

dvcrn06:01:27

moving register-layer-depenencies into init should fix this. Let me try

sglyon06:01:48

So do you have all the julia packages too? (question only makes sense if you don’t have the layer enabled, obviously)

dvcrn06:01:41

the bug is with the implementation of (register-layer-dependencies

dvcrn06:01:52

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

dvcrn06:01:14

there is no way the package manager can filter based on this information that :linter-clojure should only get installed when the clojure layer is on

sglyon06:01:27

I thought it was kinda backwards there

sglyon06:01:55

To me I would think that we’d have some method a layer can implement that specifies other layers as dependencies

dvcrn06:01:02

when tools/linter is on, install linter-clojure but not: when tools/linter is on, AND clojure is on, install linter-clojure

sglyon06:01:38

So we kinda have a design question here

sglyon06:01:54

In the clojure layer we have a linter package

sglyon06:01:20

What you described above sounds to me like this package (clojure linting) will only work properly if BOTH tools/linter and lang/clojure are enabled

sglyon06:01:44

Is that the behavior you want?

dvcrn06:01:31

well, the difference here is that register-layer-depenencies modifies state the moment it gets called. the other ones are multimethods that get called when core does it's thing. Moving the register-layer-dependencies call into init fixes this because it will not modify the state if init didn't get called which it only will when the layer is enabled. Might be a bit hacky though

sglyon06:01:10

That’s probably right, but I don’t love the solution

sglyon06:01:49

What’s the issue with having register-layer-dependencies be a multimethod just like the rest of the config methods?

dvcrn07:01:58

you mean like register-layer-dependencies :lang/clojure :tools/linter [:linter-clojure]?

dvcrn07:01:05

could work

sglyon07:01:09

That is kinda how I imagined it

dvcrn07:01:14

In general, I think we should think a bit more about code structure

dvcrn07:01:22

(I'm guilty of that too)

sglyon07:01:42

The lang/clojure layer depends on the tools/linter layer for linting support — if the user wants both, then install the linter-clojure package

sglyon07:01:02

Otherwise don't

sglyon07:01:15

BTW, moving the register-layer-dependencies call into init-layer worked just how you thought

sglyon07:01:42

The javascript linters were only installed when both the lang/javascript and tools/linter layers were enabled

dvcrn07:01:08

I think we need a batter framework for disabling / enabling stuff dynamically

dvcrn07:01:15

Really really don't like it the way we are doing it right now

dvcrn07:01:38

My disabled packages look like this:

disabledPackages: [
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "linter-jshint"
      "atom-ternjs"
      "javascript-snippets"
      "autocomplete-modules"
      "docblockr"
      "react"
      "react-snippets"
      "linter-eslint"
      "linter-jshint"
    ]

dvcrn07:01:37

I thought that might have solved it but apparently didn't:

(if (or (is-activated? package-name) force)
       (do
         (helpers/console! (str "disabling package " package-name))
         (.disablePackage packages package-name)))))

sglyon07:01:49

I had to rm ~/.atom/config.cson 3 times today because things got wacky

sglyon07:01:05

So I agree, it would be great to come up with something better

dvcrn07:01:22

most settings should be completely stateless because we kill them anyway on start

dvcrn07:01:26

just this damn disabledPackages

sglyon07:01:19

So the reason we can’t just completely wipe config.cson when proton starts is we try to remember things like toggles (disabled tabs, status bar, etc)?

dvcrn07:01:33

we do that, just not the packages

dvcrn07:01:43

because we had the problem that things got jumpy

dvcrn07:01:04

like it deactivates and activates in too close time and then calls get ignored

dvcrn07:01:13

so even though atom thinks tab-bar is disabled, it is not

sglyon07:01:20

Yeah… hmm. Is there anything we can hack on from the outside, or should we start looking at changes to core atom packages

dvcrn07:01:48

we just need to make sure that disable-package can't multiple times disable something

dvcrn07:01:07

so if linter-jshint is off, it will not get added to disabledPackages again

sglyon07:01:35

It probably isn’t as simple as reading in the config array, calling distinct, and writing it out again is it?

dvcrn07:01:51

that's what I thought I did 😐

dvcrn07:01:02

(defn is-activated? [package-name]
  (let [package-names (->> (.getActivePackages packages) js->clj (map #(.-name %)))
        filtered-packages (filter #(= package-name %) package-names)]
    (> (count filtered-packages) 0)))

dvcrn07:01:30

ah wait, I misread