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

Correct namespace prefix for inlined deps #59

Merged
merged 1 commit into from
Jun 24, 2019

Conversation

dpsutton
Copy link
Contributor

Before submitting a PR make sure the following things have been done:

  • The commits are consistent with our contribution guidelines
  • You've added tests to cover your change(s)
  • All tests are passing
  • The new code is not generating reflection warnings

Apropos is broken right now and including inlined deps for this reason and another reason that will be patched on cider-nrepl and cider soon.

dpsutton pushed a commit to dpsutton/cider-nrepl that referenced this pull request Jun 23, 2019
The required structure for the 'query/var' function is `{:var-query
{:ns-query {...}}`. The older implementation did not grab those
keywords from the cider message and put them at the correct
level. they were left top level and then missed by the
`query/namespace` function.

In addition to this, there was a bug in orchard which had the wrong
prefix for inlined deps so these were not omitted in _2_ places: both
in the client-side specification of which namespaces to ignore and
orchard's own mechanism to elide cider's internal namespaces.

See related:
- orchard clojure-emacs/orchard#59
- CIDER
dpsutton pushed a commit to dpsutton/cider that referenced this pull request Jun 23, 2019
Confluence of bugs in orchard, cider-nrepl, and CIDER. CIDER was using
the wrong key to send excluded namespaces. cider-nrepl was not putting
this key in the right position for the orchard query. And orchard was
not correctly eliding cider.nrepl inlined deps.

- orchard: clojure-emacs/orchard#59
- nrepl: clojure-emacs/cider-nrepl#618
dpsutton pushed a commit to dpsutton/cider-nrepl that referenced this pull request Jun 23, 2019
The required structure for the 'query/var' function is `{:var-query
{:ns-query {...}}`. The older implementation did not grab those
keywords from the cider message and put them at the correct
level. they were left top level and then missed by the
`query/namespace` function.

In addition to this, there was a bug in orchard which had the wrong
prefix for inlined deps so these were not omitted in _2_ places: both
in the client-side specification of which namespaces to ignore and
orchard's own mechanism to elide cider's internal namespaces.

See related:
- orchard clojure-emacs/orchard#59
- CIDER clojure-emacs/cider#2658
dpsutton pushed a commit to dpsutton/cider-nrepl that referenced this pull request Jun 23, 2019
The required structure for the 'query/var' function is `{:var-query
{:ns-query {...}}`. The older implementation did not grab those
keywords from the cider message and put them at the correct
level. they were left top level and then missed by the
`query/namespace` function.

In addition to this, there was a bug in orchard which had the wrong
prefix for inlined deps so these were not omitted in _2_ places: both
in the client-side specification of which namespaces to ignore and
orchard's own mechanism to elide cider's internal namespaces.

See related:
- orchard clojure-emacs/orchard#59
- CIDER clojure-emacs/cider#2658
dpsutton pushed a commit to dpsutton/cider that referenced this pull request Jun 23, 2019
Confluence of bugs in orchard, cider-nrepl, and CIDER. CIDER was using
the wrong key to send excluded namespaces. cider-nrepl was not putting
this key in the right position for the orchard query. And orchard was
not correctly eliding cider.nrepl inlined deps.

- orchard: clojure-emacs/orchard#59
- nrepl: clojure-emacs/cider-nrepl#618
@dpsutton dpsutton force-pushed the correct-inlined-deps-path branch from b00f8f3 to cc2944d Compare June 23, 2019 17:56
@@ -47,7 +47,9 @@
{:added "0.5"}
[var]
(let [var (as-var var)
all-vars (q/vars {:ns-query {:project? true} :private? true})
all-vars (q/vars {:ns-query {:project? true
:exclude-regexps ["^cider.nrepl.inlined-deps" "^nrepl"]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably put the list of exclusions in some var that we can just check where this is needed.

I also wonder if it won't better for the query methods to just filter those out automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The q/namespace function does filter it out. It just had the wrong prefix. This technically isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reset the branch to just the single commit and force pushed. no longer there.

@dpsutton dpsutton force-pushed the correct-inlined-deps-path branch from 1c098f0 to cc2944d Compare June 24, 2019 18:36
@bbatsov bbatsov merged commit 15a35d7 into clojure-emacs:master Jun 24, 2019
@bbatsov
Copy link
Member

bbatsov commented Jun 24, 2019

Thanks!

bbatsov pushed a commit to clojure-emacs/cider that referenced this pull request Jun 24, 2019
Confluence of bugs in orchard, cider-nrepl, and CIDER. CIDER was using
the wrong key to send excluded namespaces. cider-nrepl was not putting
this key in the right position for the orchard query. And orchard was
not correctly eliding cider.nrepl inlined deps.

- orchard: clojure-emacs/orchard#59
- nrepl: clojure-emacs/cider-nrepl#618
bbatsov pushed a commit to clojure-emacs/cider-nrepl that referenced this pull request Jun 25, 2019
The required structure for the 'query/var' function is `{:var-query
{:ns-query {...}}`. The older implementation did not grab those
keywords from the cider message and put them at the correct
level. they were left top level and then missed by the
`query/namespace` function.

In addition to this, there was a bug in orchard which had the wrong
prefix for inlined deps so these were not omitted in _2_ places: both
in the client-side specification of which namespaces to ignore and
orchard's own mechanism to elide cider's internal namespaces.

See related:
- orchard clojure-emacs/orchard#59
- CIDER clojure-emacs/cider#2658
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