neil add kaocha adds :kaocha alias with irregular indent

[teodorlu]

What

When using Neil to add Kaocha (neil add kaocha), I get one fewer indent in front of the :kaocha alias than I'd expect.

Reproduction

$ git st
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
$ neil add kaocha
If you wish to create a `bin/kaocha` file, copy and run the following:

mkdir -p bin && \
echo '#!/usr/bin/env bash
clojure -M:kaocha "$@"' > bin/kaocha && \
chmod +x bin/kaocha
$ git diff
diff --git a/deps.edn b/deps.edn
index d7f9663..ce21c02 100644
--- a/deps.edn
+++ b/deps.edn
@@ -10,4 +10,8 @@
  :aliases
  {:dev
   {:extra-paths ["dev" "test"]
-   :extra-deps {io.github.nextjournal/clerk {:mvn/version "0.13.842"}}}}}
+   :extra-deps {io.github.nextjournal/clerk {:mvn/version "0.13.842"}}}
+
+ :kaocha ;; added by neil
+ {:extra-deps {lambdaisland/kaocha {:mvn/version "1.91.1392"}}
+  :main-opts ["-m" "kaocha.runner"]}}}
$ neil --version
neil 0.3.65
$ cat deps.edn 
{:paths ["src"]
 :deps {babashka/fs {:mvn/version "0.2.16"}
        babashka/process {:mvn/version "0.4.16"}
        borkdude/edamame {:mvn/version "1.4.25"}
        cheshire/cheshire {:mvn/version "5.11.0"}
        enlive/enlive {:mvn/version "1.1.6"}
        org.babashka/cli {:mvn/version "0.6.44"}
        org.clj-commons/hickory {:mvn/version "0.7.3"}
        org.clojure/data.xml {:mvn/version "0.0.8"}}
 :aliases
 {:dev
  {:extra-paths ["dev" "test"]
   :extra-deps {io.github.nextjournal/clerk {:mvn/version "0.13.842"}}}

 :kaocha ;; added by neil
 {:extra-deps {lambdaisland/kaocha {:mvn/version "1.91.1392"}}
  :main-opts ["-m" "kaocha.runner"]}}}

[teodorlu]

I get correct indents when I have zero aliases other than :kaocha, but wrong indents when I have other aliases.

Good behavior:

$ cat deps.edn 
{:deps {}}
$ neil-dev add kaocha
"adding kaocha"
If you wish to create a `bin/kaocha` file, copy and run the following:

mkdir -p bin && \
echo '#!/usr/bin/env bash
clojure -M:kaocha "$@"' > bin/kaocha && \
chmod +x bin/kaocha
$ cat deps.edn 
{:deps {}
 :aliases
 {:kaocha ;; added by neil
  {:extra-deps {lambdaisland/kaocha {:mvn/version "1.91.1392"}}
   :main-opts ["-m" "kaocha.runner"]}}}

Bad behavior:

$ cat deps.edn 
{:deps {}
 :aliases
 {:dev {}}}
$ neil-dev add kaocha
"adding kaocha"
If you wish to create a `bin/kaocha` file, copy and run the following:

mkdir -p bin && \
echo '#!/usr/bin/env bash
clojure -M:kaocha "$@"' > bin/kaocha && \
chmod +x bin/kaocha
$ cat deps.edn 
{:deps {}
 :aliases
 {:dev {}

 :kaocha ;; added by neil
 {:extra-deps {lambdaisland/kaocha {:mvn/version "1.91.1392"}}
  :main-opts ["-m" "kaocha.runner"]}}}

[teodorlu]

babashka.neil/add-alias appears to be the function to understand:

neil/src/babashka/neil.clj

Lines 131 to 166 in 9a79582

	(defn add-alias [opts alias-kw alias-contents]
	(ensure-deps-file opts)
	(let [edn-string (edn-string opts)
	edn-nodes (edn-nodes edn-string)
	edn (edn/read-string edn-string)
	alias (or (:alias opts)
	alias-kw)
	existing-aliases (get-in edn [:aliases])
	alias-node (r/parse-string
	(str (when (seq existing-aliases) "\n ")
	alias
	" ;; added by neil"))]
	(if-not (get existing-aliases alias)
	(let [s (-> (if-not (seq existing-aliases)
	; If there are no existing aliases, we assoc an empty map
	; before updating to prevent borkdude.rewrite-edn/update
	; from removing the newline preceding the :aliases key.
	(r/assoc edn-nodes :aliases {})
	edn-nodes)
	(r/update :aliases
	(fn [aliases]
	(let [s (rw/indent alias-contents 1)
	alias-nodes (r/parse-string s)
	aliases' (r/assoc aliases alias-node alias-nodes)]
	(if-not (seq existing-aliases)
	; If there are no existing aliases, add an
	; explicit newline after the :aliases key.
	(r/parse-string (str "\n" (rw/indent (str aliases') 1)))
	aliases'))))
	str)
	s (rw/clean-trailing-whitespace s)
	s (str s "\n")]
	(spit (:deps-file opts) s))
	(do (println (format "[neil] Project deps.edn already contains alias %s" (str alias ".")))
	::update))))

[teodorlu]

@borkdude I feel tempted to drop the previous logic that tried to compute the correct in favor of a dumb approach: repeatedly using rewrite-edn/assoc-in, and accepting the result. If we go this way, newline behavior will change.

Code before and after

Previous add-alias impl relies on a hard-coded 1 value for indention, which I think is causing problems:

https://github.com/teodorlu/neil/blob/f1ec6afac0f666e068b13ec7b4fd72e8793d98be/src/babashka/neil.clj#L131-L166

Proposal: instead, assume the user already has a deps.edn file they are okay with, and that we can add content from the new map by adding one key + value per line:

https://github.com/teodorlu/neil/blob/e3dd00618a174938194509794b64f5aa46303ef1/src/babashka/neil.clj#L130-L177

Behavior example

No existing deps.edn file.

Existing behavior:

$ neil add kaocha
If you wish to create a `bin/kaocha` file, copy and run the following:

mkdir -p bin && \
echo '#!/usr/bin/env bash
clojure -M:kaocha "$@"' > bin/kaocha && \
chmod +x bin/kaocha
$ cat deps.edn 
{:deps {}
 :aliases
 {:kaocha ;; added by neil
  {:extra-deps {lambdaisland/kaocha {:mvn/version "1.91.1392"}}
   :main-opts ["-m" "kaocha.runner"]}}}

Proposed behavior:

$ rm deps.edn 
$ neil-dev add kaocha
If you wish to create a `bin/kaocha` file, copy and run the following:

mkdir -p bin && \
echo '#!/usr/bin/env bash
clojure -M:kaocha "$@"' > bin/kaocha && \
chmod +x bin/kaocha
$ cat deps.edn       
{:deps {}
 :aliases {:kaocha {:extra-deps {lambdaisland/kaocha {:mvn/version "1.91.1392"}}
                    :main-opts ["-m" "kaocha.runner"]}}}

Thoughts?

Would you like to keep the current newline behavior?

[borkdude]

Sounds good. Can you also test this with an already existing deps.edn file?

[teodorlu]

Sounds good.

👍

Can you also test this with an already existing deps.edn file?

I will 👍