Fork me on GitHub
#cider
<
2018-05-05
>
dominicm05:05:57

Refactor nrepl has a locals op

bozhidar08:05:49

Using tools.analyzer. šŸ™‚

šŸ‘ 4
bozhidar08:05:21

The problem with it is that overall complexity of the code rose a lot by adding it, not to mention some features there require parsing all project files which is pretty slow. Thatā€™s why I was wary of relying on it in CIDER itself. If itā€™s usage is limited to locals thatā€™d be fine, though.

šŸ˜¢ 4
bozhidar12:05:48

(and thereā€™s also the fact that to generate the AST you have to evaluate the code, which not everyone is expecting) šŸ™‚

āœ… 4
dominicm12:05:44

interesting thought: - apropos - test - ns ops - more? all have ns & var filtering designed in a custom ad-hoc way. It would be really useful to extend apropos to support search-ns being a filtered list based on whether it's a project ns or not, for example.

dominicm12:05:40

I wonder if it should be a DSL supporting and/or? Probably too complicated right?

dominicm12:05:49

yeah, no client would ever send that

dominicm12:05:13

Something quite short works surprisingly well

dominicm12:05:05

It's a good opportunity to have ns-vars return vars from multiple namespaces too, e.g. {:op "ns-vars", :ns-query {:filter-regexps [#"clojure\.core"]} Get all clojure core namespaces {:op "ns-vars", :ns-query {:exactly 'clojure.core}} Only clojure core {:op "ns-vars", :ns-query {:project? true}} Only vars in the current project {:op "ns-vars", :ns-query {}} All vars

dominicm12:05:35

Adding metadata support as requested for tests becomes more generally useful too!

bozhidar12:05:52

> all have ns & var filtering designed in a custom ad-hoc way. It would be really useful to extend apropos to support search-ns being a filtered list based on whether itā€™s a project ns or not, for example.

bozhidar12:05:13

Thatā€™s a very good point! Unfortunately the API evolved naturally (chaotically) and we didnā€™t give much thought to making it uniform. Actually Iā€™ve been thinking about this for a while now both for orchard and cider-nrepl and I think itā€™s really important to work in this direction. The workā€™s not much (or complex), but would make the API usage way easier and consistent.

bozhidar12:05:42

Same goes for refactor-nrepl - many things there are wildly different from cider-nrepl and thereā€™s no reason for this difference most of the time.

dominicm13:05:58

cider is organic growth, so is orchard šŸ˜›. I suppose there's rarely much motivation to do a sweeping "fix" like this.

dominicm13:05:07

If I make this change, I'm likely to cause downstream pain for you.

dominicm13:05:04

I could make it in a backwards compatible way, now that I think about it, only using ns-query if none of the "deprecated" attributes are there.

bozhidar13:05:07

Iā€™m not very worried about this - updating the client usages doesnā€™t take much time. Iā€™m willing to deal with that effort in the name of better future for everyone. šŸ™‚

bozhidar13:05:15

Yeah, if youā€™re willing to make it work in backward compatible manner thatā€™d be best, although ideally we should also send some deprecation warnings to the clients, so they authors would update them.

bozhidar13:05:35

Iā€™m not fond of the approach of keeping around compatibility code forever.

dominicm13:05:20

Heresy! Accretion is the future.

dominicm13:05:50

Oh wow, I could totally kill test-ns, as it's just a way of doing (ns/list-vars {:ns-query {:exactly #{'clojure.core-test}} :test? true})

dominicm14:05:08

@benedek these changes would also work very nicely with your new op for listing tests

bozhidar14:05:40

Yep, that seems reasonable to me.

dominicm14:05:06

There's one little hack in here for special symbols, but otherwise this is looking to be a fairly clean abstraction.

dominicm14:05:38

I'm trying to figure out: A) Whether apropos has much value beyond sorting B) Whether to match the regex behaviour, or to extend it to allow a regex for both doc & name.

lmergen15:05:00

hey, i'm debugging an issue with emacs + saving files being slow, and i think cider is to blame -- as soon as i have established some connections, emacs file saving starts using 100% CPU for about 15 seconds

lmergen15:05:18

i did elp-instrument-package on cider, and it looks like cider-resolve-var has an elapsed time of 13s

lmergen15:05:26

i have 3 repl sessions as we speak

lmergen15:05:15

anyone has an idea what might cause this ?

lmergen15:05:33

i'm running 0.17snapshot, according to cider-version

lmergen15:05:26

it seems like this started getting worse quite recently

bozhidar15:05:10

@lmergen Seems thatā€™s caused by dynamic indentation. You can just disable it.

lmergen15:05:31

does it have to do with agressive-indent-mode ?

bozhidar15:05:39

Itā€™s strange that so many people started reported issues with dynamic font-locking and dynamic indentation lately - we havenā€™t really changed them in years.

bozhidar15:05:48

> does it have to do with agressive-indent-mode ?

bozhidar15:05:07

The interaction between the two is very painful as the dynamic indent is not as fast as the static one.

bozhidar15:05:23

Iā€™d never use agressive-indent-mode with ciderā€™s dynamic indent.

lmergen15:05:02

let me test without agressive indent

bozhidar15:05:04

Thatā€™s one more reminder that we need to cache this info to avoid the excessive calls to cider-resolve-var.

lmergen15:05:16

100% this is the issue

lmergen15:05:24

i disabled the mode and everything's fine

bozhidar15:05:40

(basically you canā€™t know the indentation for something before you ā€œresolveā€ it - know what it truly is, and thatā€™s not super fast)

bozhidar15:05:59

Btw, Iā€™ll likely cut the new CIDER release on Monday. If anyone wants to get some last minute fixes in - the weekend is the time to do so. That applies to me as well. šŸ˜„

dominicm15:05:34

is that reasonable? or should I retain this behaviour?

dominicm15:05:47

I'm thinking of, at the least, pushing it to the nrepl layer

dominicm15:05:06

the filtering should be taking regexes directly.

bozhidar15:05:53

Yeah, definitely. Some things in orchard look odd just because they simply retained semantics of their middleware usage.

bozhidar16:05:25

I guess youā€™re referring to the format, right? No idea why this is done on the Clojure side. Maybe to make the clients have to work less.

dominicm16:05:48

yeah, the formatting looks way off to me.

dominicm16:05:15

That was the only thing I could think of, but it seems like it masks complexity, it stops you from using any of the other flags.

bozhidar16:05:22

Yeah, I agree.

bozhidar16:05:39

Likely I didnā€™t even notice this when the code was originally written.

dominicm22:05:22

I've got a nice set of tests setup for the var/ns filtering, looks great. Passes the apropos tests too, might go back to those and decide about call signatures, but otherwise looking good.

dominicm22:05:47

Oh, there's an apropos test which tests the namespace list, which is gone now. I'll re-implement some of those in terms that make more sense as a whole.

dominicm22:05:57

okay, now the tests are really passing šŸ™‚ I commented out the ordering rules and didn't notice.

dominicm23:05:08

I'm expecting to do a pull request on orchard tomorrow morning, and then start finishing my changes to cider tomorrow, hopefully a pull request in the afternoon.