immoh / nsorg

Clojure library for organizing ns form
https://immoh.github.io/nsorg
Eclipse Public License 1.0
13 stars 1 forks source link

Doesn't work on babashka scripts #3

Open fuadsaud opened 2 years ago

fuadsaud commented 2 years ago

Steps to reproduce

Create hello.clj file with the following contents:

#!/usr/bin/env bb

(require '[clojure.java.shell :as sh])

(sh/sh "echo" "Hello")

Run clojure -Sdeps '{:deps {nsorg-cli {:mvn/version "0.3.1"}}}' -m nsorg.cli hello.clj

Actual result

Checking following paths:
hello.clj

Failed to check path hello.clj:
clojure.lang.ExceptionInfo: Invalid token: !/usr/bin/env
{:type :reader-exception}
...

Expected Result

Since babashka scripts share the same file extension with regular Clojure files, a project that uses both clojure and babashka might have problems using extension-based glob patterns for nsorg. Since babashka scripts also have namespace forms, they could benefit from nsorg as well. Would it make sense to somehow account for the hashbang at the beginning of these files as if it were a comment?

immoh commented 2 years ago

I think this error comes from rewrite-clj which is used by nsorg to analyze and rewrite Clojure code. In order to make nsorg work with Babashka scripts, I think it would need to remove hashbang before passing the file to rewrite-clj and add it back after the work is done. This seems like something that could be in the scope of nsorg.

However, I should mention that nsorg operates on (ns ..) macro and it doesn't work with naked requires, like the one in your example. I am not super familiar with bb scripts but I would imagine that naked requires are more common than using full ns form. Making nsorg work with naked requires would require significant effort given the way it is currently implemented.

fuadsaud commented 2 years ago

Thanks for the prompt response!

I think it would need to remove hashbang before passing the file to rewrite-clj and add it back after the work is done. This seems like something that could be in the scope of nsorg.

Makes sense to me.

I am not super familiar with bb scripts but I would imagine that naked requires are more common than using full ns form.

I am too not super familiar with them as I've only started using them recently. I have the same impression as you, though. It makes more sense for scripts meant to run top to bottom to have scattered require forms (especially when considering dependencies that are optionally loaded depending on some branching logic).

Making nsorg work with naked requires would require significant effort given the way it is currently implemented.

Right. I wouldn't expect nsorg to work with standalone require calls, to be honest. But I'm guessing that, if nsorg gets past the hashbang, it will work with bb ns forms too.