[Fix #305] No newline after :as or :refer

This makes the ns form take up less vertical space and makes it easier on the eyes. This was never done intentionally but a side-effect of simply delegating the printing of regular libspecs to `clojure.core.pprint` which puts each element of a vector on its own line when the line gets long enough.
clojure-emacs · Jun 27, 2021 · fa5918d · fa5918d
1 parent 9fee2ed
commit fa5918d
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,12 +2,12 @@
 
 ## Unreleased
 
+* [#305](https://github.com/clojure-emacs/refactor-nrepl/issues/305): Don't put `:as` or `:refer` on their own lines in the ns form, when the libspec is so long it causes the line to wrap.
 * [clojure-emacs/clj-refactor.el#459](https://github.com/clojure-emacs/clj-refactor.el/issues/459): `clean-ns` should conform to the style guide: `(:require` in the ns form should be followed by a newline.
   * You can opt out via the new `:insert-newline-after-require` configuration option.
 * [#294](https://github.com/clojure-emacs/refactor-nrepl/pull/294): Properly skip uneval nodes when looking for the first/last sexp
 * From now on, if you set the `clojure.tools.namespace.repl/refresh-dirs`, files outside said `refresh-dirs` won't be analyzed, resulting in safer, more efficient analysis.
 
-
 ## 2.5.1 (2021-02-16)
 
 ### Bugs fixed

diff --git a/src/refactor_nrepl/ns/pprint.clj b/src/refactor_nrepl/ns/pprint.clj
@@ -14,7 +14,7 @@
   (vec (concat (remove sequential? libspecs)
                (filter sequential? libspecs))))
 
-(defn- pprint-prefix-form [[name & libspecs]]
+(defn- pprint-libspec-with-prefix-form [[name & libspecs]]
   (printf "[%s" name)
   (let [ordered-libspecs (libspec-vectors-last libspecs)]
     (dorun
@@ -35,11 +35,19 @@
                             (printf "%s " libspec))))))
                   ordered-libspecs))))
 
-(defn insert-clause-delimiter []
+(defn- insert-clause-delimiter []
   (if (:insert-newline-after-require *config*)
     (println)
     (print " ")))
 
+(defn- pprint-libspec [libspec]
+  ;; If a vector gets too long `pprint` will print one element per line.
+  ;; This puts `:as` and `:refer` on their own line, which causes the ns form
+  ;; to take up too much vertical space.
+  (printf (str/replace (with-out-str (pprint libspec))
+                       (re-pattern (str "(:as|refer)" (System/lineSeparator)))
+                       "$1")))
+
 (defn pprint-require-form
   [[_ & libspecs]]
   (print "(:require")
@@ -48,13 +56,15 @@
    (map-indexed
     (fn [idx libspec]
       (if (= idx (dec (count libspecs)))
-        (printf "%s)\n" (str/trim-newline
-                         (with-out-str (if (prefix-form? libspec)
-                                         (pprint-prefix-form libspec)
-                                         (pprint libspec)))))
+        (printf "%s)\n"
+                (str/trim-newline
+                 (with-out-str
+                   (if (prefix-form? libspec)
+                     (pprint-libspec-with-prefix-form libspec)
+                     (pprint libspec)))))
         (if (prefix-form? libspec)
-          (pprint-prefix-form libspec)
-          (pprint libspec))))
+          (pprint-libspec-with-prefix-form libspec)
+          (pprint-libspec libspec))))
     libspecs)))
 
 (defn- form-is? [form type]

diff --git a/test/refactor_nrepl/ns/clean_ns_test.clj b/test/refactor_nrepl/ns/clean_ns_test.clj
@@ -144,25 +144,29 @@
     (is (= clean-requires requires))
     (is (= clean-imports imports))))
 
-(def artifact-ns '(ns refactor-nrepl.artifacts
-                    (:require [clojure
-                               [edn :as edn]
-                               [string :as str]]
-                              [clojure.data.json :as json]
-                              [clojure.java.io :as io]
-                              [nrepl
-                               [middleware :refer [set-descriptor!]]
-                               [misc :refer [response-for]]
-                               [transport :as transport]]
-                              [org.httpkit.client :as http]
-                              [refactor-nrepl.externs :refer [add-dependencies]])
-                    (:import java.util.Date)))
+(def artifact-ns
+  '(ns refactor-nrepl.artifacts
+     (:require
+      [clojure
+       [edn :as edn]
+       [string :as str]]
+      [clojure.data.json :as json]
+      [clojure.java.io :as io]
+      [nrepl
+       [middleware :refer [set-descriptor!]]
+       [misc :refer [response-for]]
+       [transport :as transport]]
+      [org.httpkit.client
+       :as very-very-very-very-long-alias-causing-line-wrap]
+      [refactor-nrepl.externs :refer [add-dependencies]])
+     (:import java.util.Date)))
 
 (deftest test-pprint-artifact-ns
-  (are [setting filename] (let [actual (config/with-config {:insert-newline-after-require setting}
-                                         (pprint-ns (with-meta artifact-ns nil)))
-                                expected (-> filename File. .getAbsolutePath slurp)]
-                            (= expected actual))
+  (are [setting filename]
+       (let [actual (config/with-config {:insert-newline-after-require setting}
+                      (pprint-ns (with-meta artifact-ns nil)))
+             expected (-> filename File. .getAbsolutePath slurp)]
+         (= expected actual))
     true  "test/resources/artifacts_pprinted"
     false "test/resources/artifacts_pprinted_traditional_newline"))
 
@@ -200,13 +204,10 @@
 (deftest does-not-remove-ns-with-rename
   (is (= (nthrest ns-with-rename-cleaned 2) (nthrest (clean-ns ns-with-rename) 2))))
 
-;; Order of stuff in maps aren't stable across versions which messes
-;; with pretty-printing
-(when (= (clojure-version) "1.7.0")
-  (deftest test-pprint
-    (let [ns-str (pprint-ns (clean-ns ns1))
-          ns1-str (slurp (.getAbsolutePath (File. "test/resources/ns1_cleaned_and_pprinted")))]
-      (is (= ns1-str ns-str)))))
+(deftest test-pprint
+  (let [ns-str (pprint-ns (clean-ns ns1))
+        ns1-str (slurp (.getAbsolutePath (File. "test/resources/ns1_cleaned_and_pprinted")))]
+    (is (= ns1-str ns-str))))
 
 (deftest preserves-shorthand-meta
   (let [cleaned (pprint-ns (clean-ns ns-with-shorthand-meta))]

diff --git a/test/resources/artifacts_pprinted b/test/resources/artifacts_pprinted
@@ -9,7 +9,8 @@
     [middleware :refer [set-descriptor!]]
     [misc :refer [response-for]]
     [transport :as transport]]
-   [org.httpkit.client :as http]
+   [org.httpkit.client
+    :as very-very-very-very-long-alias-causing-line-wrap]
    [refactor-nrepl.externs :refer [add-dependencies]])
   (:import
    java.util.Date))
diff --git a/test/resources/artifacts_pprinted_traditional_newline b/test/resources/artifacts_pprinted_traditional_newline
@@ -8,6 +8,7 @@
              [middleware :refer [set-descriptor!]]
              [misc :refer [response-for]]
              [transport :as transport]]
-            [org.httpkit.client :as http]
+            [org.httpkit.client
+             :as very-very-very-very-long-alias-causing-line-wrap]
             [refactor-nrepl.externs :refer [add-dependencies]])
   (:import java.util.Date))
diff --git a/test/resources/ns1_cleaned_and_pprinted b/test/resources/ns1_cleaned_and_pprinted
@@ -10,9 +10,13 @@
    :methods [[binomial [int int] double]])
   (:require
    [clojure data edn xml
+    [instant :as inst :reload true]
     [pprint :refer [cl-format formatter get-pretty-writer]]
+    [string :refer :all :reload-all true]
+    [test :refer :all]
     [walk :refer [postwalk prewalk]]]
    clojure.test.junit)
-  (:import [java.io Closeable FilenameFilter PushbackReader]
-           [java.util Calendar Date Random]
-           [refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo]))
+  (:import
+   [java.io Closeable FilenameFilter PushbackReader]
+   [java.util Calendar Date Random]
+   [refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo]))