From 5189ce9b2073b34fe307a2c972d3069b47977572 Mon Sep 17 00:00:00 2001 From: p4v4n Date: Fri, 8 Sep 2023 04:02:22 +0530 Subject: [PATCH 1/4] Fix clojure-find-ns when preceded by other forms --- CHANGELOG.md | 2 ++ clojure-mode.el | 38 +++++++++++++++++++++------------- test/clojure-mode-sexp-test.el | 4 ++-- test/clojure-mode-util-test.el | 21 +++++++++++++++++++ 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a4b3eb7..581c7abd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs fixed +* [#656](https://github.com/clojure-emacs/clojure-mode/issues/656): Fix clojure-find-ns when ns form is preceded by other forms. + * [#593](https://github.com/clojure-emacs/clojure-mode/issues/593): Fix clojure-find-ns when ns form is preceded by whitespace or inside comment form. ## 5.16.2 (2023-08-23) diff --git a/clojure-mode.el b/clojure-mode.el index 43030a1d..30c46f61 100644 --- a/clojure-mode.el +++ b/clojure-mode.el @@ -57,7 +57,6 @@ ;;; Code: - (defvar calculate-lisp-indent-last-sexp) (defvar delete-pair-blink-delay) (defvar font-lock-beg) @@ -717,7 +716,7 @@ If JUSTIFY is non-nil, justify as well as fill the paragraph." (fill-prefix (clojure-adaptive-fill-function))) (do-auto-fill))))) - + ;;; #_ comments font-locking ;; Code heavily borrowed from Slime. ;; https://github.com/slime/slime/blob/master/contrib/slime-fontifying-fu.el#L186 @@ -780,7 +779,7 @@ and `(match-end 1)'." (scan-error (setq result 'retry)))) result)) - + ;;; General font-locking (defun clojure-match-next-def () "Scans the buffer backwards for the next \"top-level\" definition. @@ -1889,7 +1888,7 @@ work). To set it from Lisp code, use (go-loop 1) (thread 0)) - + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; @@ -1944,7 +1943,7 @@ nil." (delete-region begin (point)) result))) - + (defcustom clojure-cache-project-dir t "Whether to cache the results of `clojure-project-dir'." @@ -1988,7 +1987,7 @@ Return nil if not inside a project." "Denormalize PATH by making it relative to the project root." (file-relative-name path (clojure-project-dir))) - + ;;; ns manipulation (defun clojure-expected-ns (&optional path) "Return the namespace matching PATH. @@ -2125,7 +2124,7 @@ content) are considered part of the preceding sexp." (make-obsolete-variable 'clojure-namespace-name-regex 'clojure-namespace-regexp "5.12.0") (defconst clojure-namespace-regexp - (rx line-start (zero-or-more whitespace) "(" (? "clojure.core/") (or "in-ns" "ns" "ns+") symbol-end)) + (rx "(" (? "clojure.core/") (or "in-ns" "ns" "ns+") symbol-end)) (defcustom clojure-cache-ns nil "Whether to cache the results of `clojure-find-ns'. @@ -2148,12 +2147,13 @@ DIRECTION is `forward' or `backward'." #'search-backward-regexp))) (while (and (not candidate) (funcall fn clojure-namespace-regexp nil t)) - (let ((end (match-end 0))) + (let ((start (match-beginning 0)) + (end (match-end 0))) (save-excursion - (save-match-data - (goto-char end) - (clojure-forward-logical-sexp) - (unless (or (clojure--in-string-p) (clojure--in-comment-p) (clojure-top-level-form-p "comment")) + (when (clojure--is-top-level-form-p start) + (save-match-data + (goto-char end) + (clojure-forward-logical-sexp) (setq candidate (string-remove-prefix "'" (thing-at-point 'symbol)))))))) candidate)) @@ -2222,7 +2222,7 @@ Returns a list pair, e.g. (\"defn\" \"abc\") or (\"deftest\" \"some-test\")." (list (match-string-no-properties 1) (match-string-no-properties 2))))) - + ;;; Sexp navigation (defun clojure--looking-at-non-logical-sexp () @@ -2270,6 +2270,16 @@ This will skip over sexps that don't represent objects, so that ^hints and (backward-sexp 1)) (setq n (1- n)))))) +(defun clojure--is-top-level-form-p (&optional point) + "Return truthy if form at POINT is a top level form." + (save-excursion + (when point (goto-char point)) + (and (looking-at-p "(") + (= (point) + (progn (forward-char) + (beginning-of-defun-raw) + (point)))))) + (defun clojure-top-level-form-p (first-form) "Return truthy if the first form matches FIRST-FORM." (condition-case nil @@ -3173,7 +3183,7 @@ Assumes cursor is at beginning of function." (clojure--add-arity-reify-internal))) (indent-region beg end-marker)))) - + ;;; Toggle Ignore forms (defun clojure--toggle-ignore-next-sexp (&optional n) diff --git a/test/clojure-mode-sexp-test.el b/test/clojure-mode-sexp-test.el index aaeb798d..ad7c126b 100644 --- a/test/clojure-mode-sexp-test.el +++ b/test/clojure-mode-sexp-test.el @@ -164,9 +164,9 @@ (expect (equal "baz-quux" (clojure-find-ns)))) (let ((data '(("\"\n(ns foo-bar)\"\n" "(in-ns 'baz-quux)" "baz-quux") - (";(ns foo-bar)\n" "(in-ns 'baz-quux)" "baz-quux") + (";(ns foo-bar)\n" "(in-ns 'baz-quux2)" "baz-quux2") ("(ns foo-bar)\n" "\"\n(in-ns 'baz-quux)\"" "foo-bar") - ("(ns foo-bar)\n" ";(in-ns 'baz-quux)" "foo-bar")))) + ("(ns foo-bar2)\n" ";(in-ns 'baz-quux)" "foo-bar2")))) (pcase-dolist (`(,form1 ,form2 ,expected) data) (with-clojure-buffer form1 (save-excursion (insert form2)) diff --git a/test/clojure-mode-util-test.el b/test/clojure-mode-util-test.el index 3f6e2de7..565773d4 100644 --- a/test/clojure-mode-util-test.el +++ b/test/clojure-mode-util-test.el @@ -142,6 +142,27 @@ (with-clojure-buffer "(ns foo) (ns-unmap *ns* 'map) (ns.misleading 1 2 3)" + (expect (clojure-find-ns) :to-equal "foo"))) + (it "should skip leading garbage" + (with-clojure-buffer " (ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "1(ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "1 (ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "1 +(ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "[1] +(ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "[1] (ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "[1](ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "(ns)(ns foo)" + (expect (clojure-find-ns) :to-equal "foo")) + (with-clojure-buffer "(ns )(ns foo)" (expect (clojure-find-ns) :to-equal "foo")))) (describe "clojure-sort-ns" From ced0077fca199e395960c1c37f6f8356101c5480 Mon Sep 17 00:00:00 2001 From: p4v4n Date: Fri, 8 Sep 2023 07:16:22 +0530 Subject: [PATCH 2/4] Add back Ctrl-l chars --- clojure-mode.el | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/clojure-mode.el b/clojure-mode.el index 30c46f61..a7db2513 100644 --- a/clojure-mode.el +++ b/clojure-mode.el @@ -57,6 +57,7 @@ ;;; Code: + (defvar calculate-lisp-indent-last-sexp) (defvar delete-pair-blink-delay) (defvar font-lock-beg) @@ -716,7 +717,7 @@ If JUSTIFY is non-nil, justify as well as fill the paragraph." (fill-prefix (clojure-adaptive-fill-function))) (do-auto-fill))))) - + ;;; #_ comments font-locking ;; Code heavily borrowed from Slime. ;; https://github.com/slime/slime/blob/master/contrib/slime-fontifying-fu.el#L186 @@ -779,7 +780,7 @@ and `(match-end 1)'." (scan-error (setq result 'retry)))) result)) - + ;;; General font-locking (defun clojure-match-next-def () "Scans the buffer backwards for the next \"top-level\" definition. @@ -1888,7 +1889,7 @@ work). To set it from Lisp code, use (go-loop 1) (thread 0)) - + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; @@ -1943,7 +1944,7 @@ nil." (delete-region begin (point)) result))) - + (defcustom clojure-cache-project-dir t "Whether to cache the results of `clojure-project-dir'." @@ -1987,7 +1988,7 @@ Return nil if not inside a project." "Denormalize PATH by making it relative to the project root." (file-relative-name path (clojure-project-dir))) - + ;;; ns manipulation (defun clojure-expected-ns (&optional path) "Return the namespace matching PATH. @@ -2222,7 +2223,7 @@ Returns a list pair, e.g. (\"defn\" \"abc\") or (\"deftest\" \"some-test\")." (list (match-string-no-properties 1) (match-string-no-properties 2))))) - + ;;; Sexp navigation (defun clojure--looking-at-non-logical-sexp () @@ -3183,7 +3184,7 @@ Assumes cursor is at beginning of function." (clojure--add-arity-reify-internal))) (indent-region beg end-marker)))) - + ;;; Toggle Ignore forms (defun clojure--toggle-ignore-next-sexp (&optional n) From 41ae9d054f97cc379409d4c9302d9a1cdf0f4fb4 Mon Sep 17 00:00:00 2001 From: p4v4n Date: Fri, 8 Sep 2023 16:27:46 +0530 Subject: [PATCH 3/4] Add tests for clojure--is-top-level-form-p --- test/clojure-mode-sexp-test.el | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/clojure-mode-sexp-test.el b/test/clojure-mode-sexp-test.el index ad7c126b..f5bb3148 100644 --- a/test/clojure-mode-sexp-test.el +++ b/test/clojure-mode-sexp-test.el @@ -41,7 +41,26 @@ (wrong))" (expect (let ((beginning-of-defun-function nil)) (clojure-top-level-form-p "comment")))))) - +(describe "clojure--is-top-level-form-p" + (it "should return nil when point is inside a top level form" + (with-clojure-buffer-point + "(comment + |(ns foo))" + (expect (clojure--is-top-level-form-p) :to-equal nil)) + (with-clojure-buffer-point + "\"|(ns foo)\"" + (expect (clojure--is-top-level-form-p) :to-equal nil)) + (with-clojure-buffer-point + "^{:fake-ns |(ns foo)}" + (expect (clojure--is-top-level-form-p) :to-equal nil))) + (it "should return true when point is looking at a top level form" + (with-clojure-buffer-point + "(comment + |(ns foo))" + (expect (clojure--is-top-level-form-p (point-min)) :to-equal t)) + (with-clojure-buffer-point + "|(ns foo)" + (expect (clojure--is-top-level-form-p) :to-equal t)))) (describe "clojure-beginning-of-defun-function" (it "should go to top level form" (with-clojure-buffer-point From e1ae06d62f8ab204341139082061e491d9b7069f Mon Sep 17 00:00:00 2001 From: p4v4n Date: Fri, 8 Sep 2023 17:05:00 +0530 Subject: [PATCH 4/4] Rename fn to clojure--looking-at-top-level-form --- clojure-mode.el | 4 ++-- test/clojure-mode-sexp-test.el | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clojure-mode.el b/clojure-mode.el index a7db2513..151bd3be 100644 --- a/clojure-mode.el +++ b/clojure-mode.el @@ -2151,7 +2151,7 @@ DIRECTION is `forward' or `backward'." (let ((start (match-beginning 0)) (end (match-end 0))) (save-excursion - (when (clojure--is-top-level-form-p start) + (when (clojure--looking-at-top-level-form start) (save-match-data (goto-char end) (clojure-forward-logical-sexp) @@ -2271,7 +2271,7 @@ This will skip over sexps that don't represent objects, so that ^hints and (backward-sexp 1)) (setq n (1- n)))))) -(defun clojure--is-top-level-form-p (&optional point) +(defun clojure--looking-at-top-level-form (&optional point) "Return truthy if form at POINT is a top level form." (save-excursion (when point (goto-char point)) diff --git a/test/clojure-mode-sexp-test.el b/test/clojure-mode-sexp-test.el index f5bb3148..11bf519f 100644 --- a/test/clojure-mode-sexp-test.el +++ b/test/clojure-mode-sexp-test.el @@ -41,26 +41,26 @@ (wrong))" (expect (let ((beginning-of-defun-function nil)) (clojure-top-level-form-p "comment")))))) -(describe "clojure--is-top-level-form-p" +(describe "clojure--looking-at-top-level-form" (it "should return nil when point is inside a top level form" (with-clojure-buffer-point "(comment |(ns foo))" - (expect (clojure--is-top-level-form-p) :to-equal nil)) + (expect (clojure--looking-at-top-level-form) :to-equal nil)) (with-clojure-buffer-point "\"|(ns foo)\"" - (expect (clojure--is-top-level-form-p) :to-equal nil)) + (expect (clojure--looking-at-top-level-form) :to-equal nil)) (with-clojure-buffer-point "^{:fake-ns |(ns foo)}" - (expect (clojure--is-top-level-form-p) :to-equal nil))) + (expect (clojure--looking-at-top-level-form) :to-equal nil))) (it "should return true when point is looking at a top level form" (with-clojure-buffer-point "(comment |(ns foo))" - (expect (clojure--is-top-level-form-p (point-min)) :to-equal t)) + (expect (clojure--looking-at-top-level-form (point-min)) :to-equal t)) (with-clojure-buffer-point "|(ns foo)" - (expect (clojure--is-top-level-form-p) :to-equal t)))) + (expect (clojure--looking-at-top-level-form) :to-equal t)))) (describe "clojure-beginning-of-defun-function" (it "should go to top level form" (with-clojure-buffer-point