Skip to content

Introduce Java doc comment rendering #3472

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

Merged
merged 27 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
43f574c
Introduce Java doc comment rendering
vemv Sep 20, 2023
d0f0819
Shorten `cider-docview-render-info` when called from Company
vemv Sep 22, 2023
e5cf775
Rename `cider-context` -> `cider-completion-context`
vemv Sep 22, 2023
4074afa
Extract `cider-completion--bounds-of-non-string-symbol-at-point`
vemv Sep 22, 2023
f8286c9
Document new features
vemv Sep 22, 2023
8c336c2
Refine integration with tooltips
vemv Sep 22, 2023
c91a296
Introduce `cider-docstring--dumb-trim`
vemv Sep 22, 2023
aab2ce9
Fix linting
vemv Sep 22, 2023
97b1761
Tweak alignment for tooltips
vemv Sep 22, 2023
9084c4d
Add ellipsis to trimmed strings
vemv Sep 22, 2023
7331b29
Remove temp flag
vemv Sep 22, 2023
9b3ce28
cider-docstring--dumb-trim: clean up other whitespace
vemv Sep 22, 2023
fe71782
Don't render `see also` when :shorter
vemv Sep 22, 2023
8850264
Set a smarter default for `cider-company-docsig-render-docstring`
vemv Sep 22, 2023
5a438f2
Document `company-show-doc-buffer` integration
vemv Sep 22, 2023
d18baef
nil safe
vemv Sep 22, 2023
73e9680
cider-docview-render-info: format args better
vemv Sep 22, 2023
f1d5257
Make completion contexts work in repls
vemv Sep 23, 2023
7e77c20
Add cider-docstring-tests.el
vemv Sep 23, 2023
2d831b1
Generate `test/*.edn` files programatically
vemv Sep 27, 2023
a04f436
Observe `annotated-arglists`
vemv Oct 6, 2023
69fd34a
Add more tests
vemv Oct 6, 2023
b9dcc30
Discard `cider-company-docsig-render-docstring` defcustom
vemv Oct 6, 2023
6ab5381
Rename shorter -> compact
vemv Oct 6, 2023
548a57b
Get the full context for eldoc
vemv Oct 6, 2023
952bbbc
Bump Orchard
vemv Oct 6, 2023
e0fd855
Refine Company docs
vemv Oct 6, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .dir-locals.el
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,10 @@
(fill-column . 80)
(sentence-end-double-space . t)
(emacs-lisp-docstring-fill-column . 75)
;; slightly increase the maximum (applies to checkdoc and the byte compiler alike)
(byte-compile-docstring-max-column 100)
(checkdoc-symbol-words . ("top-level" "major-mode" "macroexpand-all" "print-level" "print-length"))
(checkdoc-package-keywords-flag)
(checkdoc-arguments-in-order-flag)
(checkdoc-verb-check-experimental-flag)
;; allow commas to indicate that the first sentence continues, which enables longer first sentences
(checkdoc-permit-comma-termination-flag t)
(elisp-lint-indent-specs . ((if-let* . 2)
(when-let* . 1)
(let* . defun)
Expand Down
11 changes: 10 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jobs:
matrix:
os: [macos-latest, ubuntu-latest, windows-latest]
emacs_version: ['26.3', '27.2', '28.2', '29.1']
java_version: ['11', '17']
Copy link
Member Author

Choose a reason for hiding this comment

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

I introduced a JVM matrix (soon to be expanded with JDK 21)


steps:
- name: Set up Emacs
Expand Down Expand Up @@ -67,7 +68,7 @@ jobs:
with:
distribution: 'temurin'
# shadow requires java 11
java-version: 11
java-version: ${{matrix.java_version}}

- name: Install Clojure Tools
# Use SHA until
Expand All @@ -91,3 +92,11 @@ jobs:
# be GH connectivity runner issues. We attempt to address this
# problem by rerunning the tests more than once.
eldev -p -dtTC test --test-type integration || eldev -p -dtTC test --test-type integration

- name: Run tests that need enrich-classpath
if: "!startsWith(matrix.os, 'windows')"
run: |
cd dev; ../clojure.sh clojure -M:gen; cd -
Copy link
Member Author

@vemv vemv Sep 27, 2023

Choose a reason for hiding this comment

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

now the test/File.edn files are generated programatically.

As a nice side-effect, we get to exercise our clojure.sh script, and with it, enrich-classpath.

Both get an extra exercise given the JVM matrix.

We could build more tests like this in a future - nice middle ground between reliable and realistic tests (since there's no nrepl connection - only data).

wc -l test/File.edn
eldev -p -dtTC test --test-type enrich || eldev -p -dtTC test --test-type enrich

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ cider-pkg.el
cider-refcard.aux
cider-refcard.log
doc/auto/
test/*.edn
11 changes: 9 additions & 2 deletions Eldev
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,32 @@
(eldev-add-loading-roots 'test "test/utils")
(eldev-add-extra-dependencies 'runtime '(:package logview :optional t))

;; slightly increase the maximum (applies to checkdoc and the byte compiler alike)
(setq byte-compile-docstring-max-column 100)
Copy link
Member

Choose a reason for hiding this comment

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

The Emacs maintainers advised against doing this, that's why I had changed it. Normally it's best to have development settings in .dir-locals.el (you can have them nested in various folders)

Copy link
Member Author

Choose a reason for hiding this comment

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

It didn't work otherwise though 😞

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I can't imagine how - after all .dir-locals are applied to the entire directory. Unless Eldev is suppressing those somehow.

Copy link
Member Author

@vemv vemv Sep 27, 2023

Choose a reason for hiding this comment

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

I don't know

There's no mention of .dir-locals in the Eldev repo, which might be a signal that it isn't considered as a config mechanism (it does mention four levels of configuration in the readme)

Copy link
Member

Choose a reason for hiding this comment

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

@doublep Does Eldev ignore dir-locals intentionally? If so, I guess we should have the config in both places.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it simply doesn't do anything special. Which means they are not activated through some automatic magic.

it does mention four levels of configuration in the readme

Those don't include .dir-locals. All ways of configuration are Eldev-specific, but let you evaluate arbitrary Elisp code, not just set variables to constant values.


;; allow commas to indicate that the first sentence continues, which enables longer first sentences
(setq checkdoc-permit-comma-termination-flag t)

(defvar cider-test-type 'main)
(setf eldev-standard-excludes `(:or ,eldev-standard-excludes
;; Avoid including files in test "projects".
(eldev-pcase-exhaustive cider-test-type
(`main "./test/*/")
(`integration '("./test/" "!./test/integration"))
(`enrich '("./test/" "!./test/enrich"))
(`all '("./test/*/" "!./test/integration")))
"test/integration/projects"
;; This file is _supposed_ to be excluded
;; from automated testing.
"test/cider-tests--no-auto.el"))

(eldev-defoption cider-test-selection (type)
"Select tests to run; type can be `main', `integration' or `all'"
"Select tests to run; type can be `main', `integration', `enrich' or `all'"
:options (-T --test-type)
:for-command test
:value TYPE
:default-value cider-test-type
(unless (memq (intern type) '(main integration all))
(unless (memq (intern type) '(main integration enrich all))
(signal 'eldev-wrong-option-usage `("unknown test type `%s'" ,type)))
(setf cider-test-type (intern type)))

Expand Down
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,15 @@ lint: clean
compile: clean
eldev -dtT compile --warnings-as-errors

test-all: clean
test/File.edn:
cd dev; ../clojure.sh clojure -M:gen

test-all: clean test/File.edn
eldev -dtT -p test --test-type all

test-enrich: clean test/File.edn
eldev -dtT -p test --test-type enrich

test-integration: clean
eldev -dtT -p test --test-type integration

Expand Down
19 changes: 11 additions & 8 deletions cider-client.el
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
(require 'spinner)

(require 'cider-connection)
(require 'cider-completion-context)
(require 'cider-common)
(require 'cider-util)
(require 'nrepl-client)
Expand Down Expand Up @@ -520,15 +521,15 @@ When multiple matching vars are returned you'll be prompted to select one,
unless ALL is truthy."
(when (and var (not (string= var "")))
(let ((var-info (cond
((cider-nrepl-op-supported-p "info") (cider-sync-request:info var))
((cider-nrepl-op-supported-p "info") (cider-sync-request:info var nil nil (cider-completion-get-context t)))
((cider-nrepl-op-supported-p "lookup") (cider-sync-request:lookup var))
(t (cider-fallback-eval:info var)))))
(if all var-info (cider--var-choice var-info)))))

(defun cider-member-info (class member)
"Return the CLASS MEMBER's info as an alist with list cdrs."
(when (and class member)
(cider-sync-request:info nil class member)))
(cider-sync-request:info nil class member (cider-completion-get-context t))))


;;; Requests
Expand Down Expand Up @@ -646,13 +647,14 @@ CONTEXT represents a completion context for compliment."
nil
'abort-on-input))

(defun cider-sync-request:info (symbol &optional class member)
"Send \"info\" op with parameters SYMBOL or CLASS and MEMBER."
(defun cider-sync-request:info (symbol &optional class member context)
"Send \"info\" op with parameters SYMBOL or CLASS and MEMBER, honor CONTEXT."
(let ((var-info (thread-first `("op" "info"
"ns" ,(cider-current-ns)
,@(when symbol `("sym" ,symbol))
,@(when class `("class" ,class))
,@(when member `("member" ,member)))
,@(when member `("member" ,member))
,@(when context `("context" ,context)))
(cider-nrepl-send-sync-request (cider-current-repl)))))
(if (member "no-info" (nrepl-dict-get var-info "status"))
nil
Expand All @@ -669,13 +671,14 @@ CONTEXT represents a completion context for compliment."
nil
(nrepl-dict-get var-info "info"))))

(defun cider-sync-request:eldoc (symbol &optional class member)
"Send \"eldoc\" op with parameters SYMBOL or CLASS and MEMBER."
(defun cider-sync-request:eldoc (symbol &optional class member context)
"Send \"eldoc\" op with parameters SYMBOL or CLASS and MEMBER, honor CONTEXT."
(when-let* ((eldoc (thread-first `("op" "eldoc"
"ns" ,(cider-current-ns)
,@(when symbol `("sym" ,symbol))
,@(when class `("class" ,class))
,@(when member `("member" ,member)))
,@(when member `("member" ,member))
,@(when context `("context" ,context)))
(cider-nrepl-send-sync-request (cider-current-repl)
'abort-on-input))))
(if (member "no-eldoc" (nrepl-dict-get eldoc "status"))
Expand Down
122 changes: 122 additions & 0 deletions cider-completion-context.el
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
;;; cider-completion-context.el --- Context parsing -*- lexical-binding: t -*-

;; Copyright © 2013-2023 Bozhidar Batsov, Artur Malabarba and CIDER contributors
;;
;; Author: Bozhidar Batsov <bozhidar@batsov.dev>
;; Artur Malabarba <bruce.connor.am@gmail.com>

;; This program is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with this program. If not, see <http://www.gnu.org/licenses/>.

;; This file is not part of GNU Emacs.

;;; Commentary:

;; Context-parsing utilities. Extracted from cider-completion.el.

;;; Code:

(defcustom cider-completion-use-context t
"When true, uses context at point to improve completion suggestions."
:type 'boolean
:group 'cider
:package-version '(cider . "0.7.0"))

(defun cider-completion--bounds-of-non-string-symbol-at-point ()
"Returns the bounds of the symbol at point, unless it's inside a string."
(let ((sap (symbol-at-point)))
(when (and sap (not (nth 3 (syntax-ppss))))
(bounds-of-thing-at-point 'symbol))))

(defun cider-completion-symbol-start-pos ()
"Find the starting position of the symbol at point, unless inside a string."
(car (cider-completion--bounds-of-non-string-symbol-at-point)))

(defun cider-completion-symbol-end-pos ()
"Find the end position of the symbol at point, unless inside a string."
(cdr (cider-completion--bounds-of-non-string-symbol-at-point)))

(defun cider-completion-get-info-context-at-point ()
"Extract a context at point that is suitable for eldoc and info ops.
Note that this context is slightly different than that of
`cider-completion-get-context-at-point': this one does not include
the current symbol at point."
(when (save-excursion
(condition-case _
(progn
(up-list)
(check-parens)
t)
(scan-error nil)
(user-error nil)))
(save-excursion
(let* ((pref-start (cider-completion-symbol-start-pos))
(context (cider-defun-at-point))
(end (cider-completion-symbol-end-pos))
(_ (beginning-of-defun-raw))
(expr-start (point))
(_ (if (derived-mode-p 'cider-repl-mode)
(goto-char (point-max))
(end-of-defun)))
(expr-end (point)))
(string-remove-suffix "\n"
(concat (when pref-start (substring context 0 (- pref-start expr-start)))
"__prefix__"
(substring context (- (- expr-end end)))))))))

(defun cider-completion-get-context-at-point ()
"Extract the context at point.
If point is not inside the list, returns nil; otherwise return \"top-level\"
form, with symbol at point replaced by __prefix__."
(when (save-excursion
(condition-case _
(progn
(up-list)
(check-parens)
t)
(scan-error nil)
(user-error nil)))
(save-excursion
(let* ((pref-end (point))
(pref-start (cider-completion-symbol-start-pos))
(context (cider-defun-at-point))
(_ (beginning-of-defun-raw))
(expr-start (point)))
(concat (when pref-start (substring context 0 (- pref-start expr-start)))
"__prefix__"
(substring context (- pref-end expr-start)))))))

(defvar cider-completion-last-context nil)

(defun cider-completion-get-context (&optional info)
"Extract context depending (maybe of INFO type).

Output depends on `cider-completion-use-context' and the current major mode."
(let ((context (if cider-completion-use-context
;; We use ignore-errors here since grabbing the context
;; might fail because of unbalanced parens, or other
;; technical reasons, yet we don't want to lose all
;; completions and throw error to user because of that.
(or (ignore-errors
(if info
(cider-completion-get-info-context-at-point)
(cider-completion-get-context-at-point)))
"nil")
"nil")))
(if (string= cider-completion-last-context context)
":same"
(setq cider-completion-last-context context)
context)))

(provide 'cider-completion-context)
;;; cider-completion-context.el ends here
68 changes: 7 additions & 61 deletions cider-completion.el
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,12 @@

(require 'cider-client)
(require 'cider-common)
(require 'cider-completion-context)
(require 'cider-doc)
(require 'cider-docstring)
(require 'cider-eldoc)
(require 'nrepl-dict)

(defcustom cider-completion-use-context t
"When true, uses context at point to improve completion suggestions."
:type 'boolean
:group 'cider
:package-version '(cider . "0.7.0"))

(defcustom cider-annotate-completion-candidates t
"When true, annotate completion candidates with some extra information."
:type 'boolean
Expand Down Expand Up @@ -116,55 +112,6 @@ if the candidate is not namespace-qualified."
:group 'cider
:package-version '(cider . "0.9.0"))

(defvar cider-completion-last-context nil)

(defun cider-completion-symbol-start-pos ()
"Find the starting position of the symbol at point, unless inside a string."
(let ((sap (symbol-at-point)))
(when (and sap (not (nth 3 (syntax-ppss))))
(car (bounds-of-thing-at-point 'symbol)))))

(defun cider-completion-get-context-at-point ()
"Extract the context at point.
If point is not inside the list, returns nil; otherwise return \"top-level\"
form, with symbol at point replaced by __prefix__."
(when (save-excursion
(condition-case _
(progn
(up-list)
(check-parens)
t)
(scan-error nil)
(user-error nil)))
(save-excursion
(let* ((pref-end (point))
(pref-start (cider-completion-symbol-start-pos))
(context (cider-defun-at-point))
(_ (beginning-of-defun-raw))
(expr-start (point)))
(concat (when pref-start (substring context 0 (- pref-start expr-start)))
"__prefix__"
(substring context (- pref-end expr-start)))))))

(defun cider-completion-get-context ()
"Extract context depending on `cider-completion-use-context' and major mode."
(let ((context (if (and cider-completion-use-context
;; Important because `beginning-of-defun' and
;; `ending-of-defun' work incorrectly in the REPL
;; buffer, so context extraction fails there.
(derived-mode-p 'clojure-mode))
;; We use ignore-errors here since grabbing the context
;; might fail because of unbalanced parens, or other
;; technical reasons, yet we don't want to lose all
;; completions and throw error to user because of that.
(or (ignore-errors (cider-completion-get-context-at-point))
"nil")
"nil")))
(if (string= cider-completion-last-context context)
":same"
(setq cider-completion-last-context context)
context)))

(defun cider-completion--parse-candidate-map (candidate-map)
"Get \"candidate\" from CANDIDATE-MAP.
Put type and ns properties on the candidate"
Expand Down Expand Up @@ -253,7 +200,7 @@ performed by `cider-annotate-completion-function'."
(cider-complete prefix) prefix pred)))))
:annotation-function #'cider-annotate-symbol
:company-kind #'cider-company-symbol-kind
:company-doc-buffer #'cider-create-doc-buffer
:company-doc-buffer #'cider-create-compact-doc-buffer
:company-location #'cider-company-location
:company-docsig #'cider-company-docsig))))

Expand Down Expand Up @@ -281,11 +228,10 @@ in the buffer."

(defun cider-company-docsig (thing)
"Return signature for THING."
(let* ((eldoc-info (cider-eldoc-info thing))
(ns (lax-plist-get eldoc-info "ns"))
(symbol (lax-plist-get eldoc-info "symbol"))
(arglists (lax-plist-get eldoc-info "arglists")))
(when eldoc-info
(when-let ((eldoc-info (cider-eldoc-info thing)))
(let* ((ns (lax-plist-get eldoc-info "ns"))
(symbol (lax-plist-get eldoc-info "symbol"))
(arglists (lax-plist-get eldoc-info "arglists")))
(format "%s: %s"
(cider-eldoc-format-thing ns symbol thing
(cider-eldoc-thing-type eldoc-info))
Expand Down
Loading