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

Add support to filter out CIDER's internal namespaces and vars #1724

Merged
merged 1 commit into from
May 1, 2016
Merged

Add support to filter out CIDER's internal namespaces and vars #1724

merged 1 commit into from
May 1, 2016

Conversation

ckoparkar
Copy link
Contributor

@ckoparkar ckoparkar commented Apr 30, 2016

Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (make test)
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)
  • You've updated the refcard (if you made changes to the commands listed there)

Thanks!

For #1564

@@ -204,6 +204,18 @@ Sub-match 1 must be the project path.")
(defvar cider-host-history nil
"Completion history for connection hosts.")

(defcustom cider-filter-regexps '("cider.nrepl" "refactor-nrepl")
Copy link
Member

Choose a reason for hiding this comment

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

As those are regexps, I guess they should start with ^ (beginning of string). I'd also filter nREPL's own namespaces.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we can't think of a better name for the variable. Right now it's not really clear what this is filtering without reading the docstring.

Copy link
Member

Choose a reason for hiding this comment

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

-filtered-namespaces-regexps?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this was applied just to the ns of something of to the entire fully qualified name.

@bbatsov
Copy link
Member

bbatsov commented Apr 30, 2016

Why didn't you place this defcustom in cider-client? After all it's the only place it's used.

@bbatsov
Copy link
Member

bbatsov commented Apr 30, 2016

You'll also have to rebase.

@@ -29,6 +29,7 @@
* Use `cider-apropos-select` instead of `cider-apropos` in `cider-apropos-documentation-select`.
* [#1561](https://github.com/clojure-emacs/cider/issues/1561): Use an appropriate font-lock-face for variables, macros and functions in
the ns-browser.
* [#1564](https://github.com/clojure-emacs/cider/issues/1564): CIDER's internal namespaces and vars are filtered from the ns-browser and apropos functions.
Copy link
Member

Choose a reason for hiding this comment

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

Guess this should be in new features.

Hide all nREPL middleware details from `cider-browse-ns*` and `cider-apropos*` commands by customizing the variable `cider-filter-regexps`.
It should be a list of regexps matching the pattern of namespaces you want to filter out.
@ckoparkar
Copy link
Contributor Author

I've made all the changes and rebased against the latest master. The defcustom is moved to cider-client.el; I'd kept it in cider.el as most of the custom vars are there. Also, the var is now named cider-filtered-namespaces-regexps as @Malabarba suggested; it is only being applied to the namespaces, returned by (all-ns).

@bbatsov bbatsov merged commit a9d1d58 into clojure-emacs:master May 1, 2016
@bbatsov
Copy link
Member

bbatsov commented May 1, 2016

👍

@ckoparkar ckoparkar deleted the feature/hide-internals branch May 1, 2016 08:44
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.

4 participants