babashka / neil

A CLI to add common aliases and features to deps.edn-based projects
MIT License
377 stars 27 forks source link

`neil add kaocha` adds `:kaocha` alias with irregular indent #215

Closed teodorlu closed 5 months ago

teodorlu commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

babashka.neil/add-alias appears to be the function to understand: https://github.com/babashka/neil/blob/9a795828e4c201a47c5851157868c06f2ca37448/src/babashka/neil.clj#L131-L166

teodorlu commented 5 months ago

@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 commented 5 months ago

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

teodorlu commented 5 months ago

Sounds good.

👍

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

I will 👍