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

Extend the default doc xref regexp #2781

Merged
merged 4 commits into from
Jan 19, 2020
Merged

Conversation

yuhan0
Copy link
Contributor

@yuhan0 yuhan0 commented Jan 17, 2020

This allows cider-doc buffers to recognize [[var]] syntax and fully qualified symbols as xref links.

Both are fairly common in docstrings in core and 3rd-party libraries that I've personally encountered, and it's otherwise somewhat annoying to navigate to a referenced var or function. (E.g. executing M-x cider-docin a *cider-doc* buffer doesn't pick up the symbol at point).

If this is acceptable I'll go ahead with adding tests and changelogs :)


Before submitting the 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)
  • All code passes the linter (make lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

To recognize [[var]] syntax and fully qualified symbols as xref links
@bbatsov
Copy link
Member

bbatsov commented Jan 18, 2020

That's a nice improvement! I believe we did something similar a while ago in clojure-mode so that more xref formats are font-locked there properly.

(rx (or (: "`" (group-n 1 (+ (not space))) "`") ; `var`
(eval-and-compile
(defcustom cider-doc-xref-regexp
(rx-to-string
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the eval-and-compile be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah of course, changing it now-

(not space) was too permissive and would include commas and other punctuation.
@yuhan0
Copy link
Contributor Author

yuhan0 commented Jan 19, 2020

I looked into writing tests for this, but realised there were none written for the entire cider-doc namespace- also it's not clear how to test functions which rely on a REPL connection.

@bbatsov
Copy link
Member

bbatsov commented Jan 19, 2020

The best approach is to simply stub the functions that require a REPL connection.

@yuhan0
Copy link
Contributor Author

yuhan0 commented Jan 19, 2020

I added a test for the regexp matching in cider-util, which should cover the changes for this PR.

@bbatsov bbatsov merged commit 6ad997e into clojure-emacs:master Jan 19, 2020
@bbatsov
Copy link
Member

bbatsov commented Jan 19, 2020

Great work! Thanks! 🙇

@bbatsov
Copy link
Member

bbatsov commented Jan 20, 2020

@yuhan0 Btw, it'd be a good idea to update the font-locking of fully-qualified vars appearing in docstrings in clojure-mode as well for consistency.

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