Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track namespace info as part of track-state #249

Merged
merged 3 commits into from
Sep 10, 2015
Merged

Track namespace info as part of track-state #249

merged 3 commits into from
Sep 10, 2015

Conversation

Malabarba
Copy link
Member

Goes with clojure-emacs/cider#1301

The track-state middleware now keeps the client informed of loaded
namespaces as well. For each namespace, we transmit a map of:

  • All interns (from symbol to the var's metadata). Only select metadata
    keys are transmitted, currently that's:
    • :indent
    • :cider-instrumented
    • :macro
    • :arglists
    • :test
  • All refers not from clojure.core (from symbol to fully-qualified var name)
  • All aliases (from symbol to namespace name)

@bbatsov
Copy link
Member

bbatsov commented Sep 4, 2015

At a pretty cursory glance the code looks OK. The last two commits should be squash together.

@bbatsov
Copy link
Member

bbatsov commented Sep 4, 2015

I'm also worried about some potential performance implications for namespaces with a ton of interned symbols, but I guess there's not much we can do about this (unless we only submit some delta, which would probably complicate the code a lot).

@Malabarba
Copy link
Member Author

I'm also worried about some potential performance implications for namespaces with a ton of interned symbols

I was worried about that at first, but it really shouldn't be a problem on the nrepl side. Generating a map of all namespaces and printing it takes barely a millisecond on my system, so it should be fast enough even on very slow systems (like an android device).

If there's any performance performance issue, it would have to be on the Emacs side. But @cichli made a good point that the middleware for completion candidates frequently sends messages with pretty large content, and nobody seems to complain it's slow.

@bbatsov
Copy link
Member

bbatsov commented Sep 4, 2015

If there's any performance performance issue, it would have to be on the Emacs side. But @cichli made a good point that the middleware for completion candidates frequently sends messages with pretty large content, and nobody seems to complain it's slow.

Yeah, I'm worried solely about decoding this data on Emacs's side, but let's assume we're lucky and this won't be a problem. A side problem is that this data might dominate the nREPL log, which would limit it's usefulness, but we can think about this additionally.

Address my remarks and let's merge this.

@Malabarba
Copy link
Member Author

A side problem is that this data might dominate the nREPL log,

Good point. I'll look into turning this into a button on the log buffer, so you'd have a clickable ellipsis which expands into the whole thing.

@Malabarba Malabarba force-pushed the track-state branch 2 times, most recently from d6a86c6 to b350819 Compare September 9, 2015 22:02
@Malabarba
Copy link
Member Author

OK. I think it's ready.
Finally managed to add cljs support too.

The track-state middleware now keeps the client informed of loaded
namespaces as well. For each namespace, we transmit a map of:

- All interns (from symbol to the var's metadata). Only select metadata
  keys are transmitted, currently that's:
  - `:indent`
  - `:cider-instrumented`
  - `:macro`
  - `:arglists`
  - `:test`
- All refers not from clojure.core (from symbol to fully-qualified var name)
- All aliases (from symbol to namespace name)
bbatsov added a commit that referenced this pull request Sep 10, 2015
Track namespace info as part of track-state
@bbatsov bbatsov merged commit ad6b076 into master Sep 10, 2015
@bbatsov bbatsov deleted the track-state branch September 10, 2015 05:43
@bbatsov
Copy link
Member

bbatsov commented Sep 10, 2015

👍

@bbatsov
Copy link
Member

bbatsov commented Sep 10, 2015

~/p/cider-nrepl git:master ❯❯❯ ./build.sh deploy clojars
Could not find artifact cider:cider-nrepl:jar:0.10.0-20150910.054519-24 in clojars (https://clojars.org/repo/)
This could be due to a typo in :dependencies or network issues.
If you are behind a proxy, try setting the 'http_proxy' environment variable.
FAILED

@Malabarba can you try pushing to clojars yourself?

@Malabarba
Copy link
Member Author

@bbatsov done! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants