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

Introduce completable-for-cljr-slash? #493

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
6 changes: 4 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ default: &default-steps
- run: make elpa
- run: emacs --version
- run: cask --version
- run: make unit
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 a target like unit-tests would be better here. make test should probably run all tests and there should be a separate make target for the integration tests.

- run: make test
# Make sure to run test-checks before test-bytecomp, as test-bytecomp autogenerates
# files which won't pass test-checks.
- run: make test-checks
- run: make test-bytecomp
# disabled - see https://github.com/clojure-emacs/clj-refactor.el/issues/491
#- run: make test-checks
#- run: make test-bytecomp

# Enumerated list of Emacs versions
jobs:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- [#483](https://github.com/clojure-emacs/clj-refactor.el/issues/483): Improve performance of cljr-slash when typing fraction literals.
- [#482](https://github.com/clojure-emacs/clj-refactor.el/issues/482): Add missing defgroup form.
- [#470](https://github.com/clojure-emacs/clj-refactor.el/issues/470): Choose `deps.edn` over `pom.xml` as project file.
- [#492](https://github.com/clojure-emacs/clj-refactor.el/issues/492): Speed up `cljr-slash` for predictably non-completable inputs.
- Introduce `defcustom cljr-insert-newline-after-require` option.

## 2.5.1 (2021-02-16)
Expand Down
3 changes: 2 additions & 1 deletion Cask
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@

(development
(depends-on "ecukes")
(depends-on "espuds"))
(depends-on "espuds")
(depends-on "buttercup"))
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ clean:
test: $(PKGDIR)
$(CASK) exec ecukes --no-win

unit:
$(CASK) exec buttercup -L .

test-checks:
$(CASK) exec $(EMACS) --no-site-file --no-site-lisp --batch \
-l test/test-checks.el ./
Expand Down
54 changes: 30 additions & 24 deletions clj-refactor.el
Original file line number Diff line number Diff line change
Expand Up @@ -1983,37 +1983,43 @@ the alias in the project."
(backward-sexp 1)
(looking-at-p "[-+0-9]")))

(defun completable-for-cljr-slash? (sym)
Copy link
Member

Choose a reason for hiding this comment

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

All definitions should have a docstring, especially the "public" ones.

(when sym
(not (null (string-match-p "^\\^?\\(::\\)?\\([a-zA-Z]+[a-zA-Z0-9\\-]*\\)+\\(\\.?[a-zA-Z]+[a-zA-Z0-9\\-]*\\)*$" sym)))))

;;;###autoload
(defun cljr-slash ()
"Inserts / as normal, but also checks for common namespace shorthands to require.
If `cljr-magic-requires' is non-nil, executing this command after one of the aliases
listed in `cljr-magic-require-namespaces', or any alias used elsewhere in the project,
will add the corresponding require statement to the ns form."
(interactive)
(insert "/")
(when-let (aliases (and cljr-magic-requires
(not (cljr--in-map-destructuring?))
(not (cljr--in-ns-above-point-p))
(not (cljr--in-reader-literal-p))
(not (cider-in-comment-p))
(not (cider-in-string-p))
(not (cljr--in-keyword-sans-alias-p))
(not (cljr--in-number-p))
(clojure-find-ns)
(cljr--magic-requires-lookup-alias)))
(let ((short (cl-first aliases)))
(when-let (long (cljr--prompt-user-for "Require " (cl-second aliases)))
(when (and (not (cljr--in-namespace-declaration-p (concat ":as " short "\b")))
(or (not (eq :prompt cljr-magic-requires))
(not (> (length (cl-second aliases)) 1)) ; already prompted
(yes-or-no-p (format "Add %s :as %s to requires?" long short))))
(save-excursion
(cljr--insert-in-ns ":require")
(let ((libspec (format "[%s :as %s]" long short)))
(insert libspec)
(ignore-errors (cljr--maybe-eval-ns-form))
(cljr--indent-defun)
(cljr--post-command-message "Required %s" libspec))))))))
(let ((original-input (thing-at-point 'symbol)))
(insert "/")
vemv marked this conversation as resolved.
Show resolved Hide resolved
(when-let (aliases (and cljr-magic-requires
(completable-for-cljr-slash? original-input)
(not (cljr--in-map-destructuring?))
(not (cljr--in-ns-above-point-p))
(not (cljr--in-reader-literal-p))
(not (cider-in-comment-p))
(not (cider-in-string-p))
(not (cljr--in-keyword-sans-alias-p))
(not (cljr--in-number-p))
(clojure-find-ns)
(cljr--magic-requires-lookup-alias)))
vemv marked this conversation as resolved.
Show resolved Hide resolved
(let ((short (cl-first aliases)))
(when-let (long (cljr--prompt-user-for "Require " (cl-second aliases)))
(when (and (not (cljr--in-namespace-declaration-p (concat ":as " short "\b")))
(or (not (eq :prompt cljr-magic-requires))
(not (> (length (cl-second aliases)) 1)) ; already prompted
(yes-or-no-p (format "Add %s :as %s to requires?" long short))))
(save-excursion
(cljr--insert-in-ns ":require")
(let ((libspec (format "[%s :as %s]" long short)))
(insert libspec)
(ignore-errors (cljr--maybe-eval-ns-form))
(cljr--indent-defun)
(cljr--post-command-message "Required %s" libspec)))))))))

(defun cljr--in-namespace-declaration-p (s)
(save-excursion
Expand Down
1 change: 1 addition & 0 deletions feature/feature.el
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(provide 'feature)
23 changes: 12 additions & 11 deletions test/test-checks.el
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@
(setq checkdoc-arguments-in-order-flag nil)
(setq checkdoc-verb-check-experimental-flag nil)

(let ((files (directory-files default-directory t
"\\`[^.].*\\.el\\'" t)))
(when nil ;; See https://github.com/clojure-emacs/clj-refactor.el/issues/491
(let ((files (directory-files default-directory t
"\\`[^.].*\\.el\\'" t)))

;; `checkdoc-file' was introduced in Emacs 25
(when (fboundp 'checkdoc-file)
(dolist (file files)
(checkdoc-file file))
(when (get-buffer "*Warnings*")
(message "Failing due to checkdoc warnings...")
(kill-emacs 1)))
;; `checkdoc-file' was introduced in Emacs 25
(when (fboundp 'checkdoc-file)
(dolist (file files)
(checkdoc-file file))
(when (get-buffer "*Warnings*")
(message "Failing due to checkdoc warnings...")
(kill-emacs 1)))

(when (apply #'check-declare-files files)
(kill-emacs 1)))
(when (apply #'check-declare-files files)
(kill-emacs 1))))
30 changes: 30 additions & 0 deletions tests/unit-test.el
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
(require 'paredit)
(require 'clj-refactor)

(describe "completable-for-cljr-slash?"
(it "Returns `t' for tokens that can represent alias-prefixed named things (symbols, keywords, metadata)"
(expect (completable-for-cljr-slash? nil) :to-be nil)
(dolist (prefix '("" "^" "^::"))
(expect (completable-for-cljr-slash? (concat prefix "a")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a-")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a-2")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2-")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2.a")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2.a-")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2.a2")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2.a-2")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2.a2-")) :to-be t)
(expect (completable-for-cljr-slash? (concat prefix "a2.a.")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "a2.2")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "a2.2a")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "a2.-")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "a2.-a")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "-")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix ".")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "2")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "+")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "+")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "a/")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "a/a")) :to-be nil)
(expect (completable-for-cljr-slash? (concat prefix "a/a/")) :to-be nil))))