seancorfield / deps-new

Create new projects for the Clojure CLI / deps.edn
Eclipse Public License 1.0
347 stars 19 forks source link

scratch template does not use :name #32

Closed borkdude closed 2 years ago

borkdude commented 2 years ago

When creating a scratch project, it does not use the :name option except for the directory

seancorfield commented 2 years ago

True. What would you expect it to affect? It's creating a bare minimum "project" with just a scratch.clj file in it.

borkdude commented 2 years ago

I think it would be nice if the name option affected the namespace name, like with the other templates. But I could create my own "minimal" template for this if you think scratch should behave the way it does now. minimal could also have a test directory with one example test.

seancorfield commented 2 years ago

I'd be happy to take a PR to add a new minimal template that is somewhere between scratch and app -- with an additional README section describing it.

borkdude commented 2 years ago

Cool. What I think would be reasonable for a minimal template:

Would you agree with this? My use case for this minimal template would be to make it the default in neil's new command. The idea of neil is to take an existing project or start with a minimal setup and then incrementally add things to it: neil add test adds cognitect's test runner, neil add build adds a build.clj, etc. Starting with neil new minimal myproject would be the use case for the above template.

Another minor thing: Currently neil runs with a fork of deps-new to make it compatible with babashka. To commit to make it babashka-compatible was this one: https://github.com/borkdude/deps-new/commit/76259a27c57dba4530671a3d18c610ed904297d7 I replaced the usage of Calendar with Date: babashka doesn't ship with the Calender class since it has adopted java.time.* as the preferred time library and adding Calender would mean adding more space to the binary while it's regarded as something that's replaced by java.time. But because Date is so ubiquitous, of course it has support for that. I would understand if you wouldn't accept this change and would be happy to continue using my minor fork, but if you don't mind, I could include this change too with the above mentioned minimal template.

seancorfield commented 2 years ago

So an empty deps.edn but also a test file -- that couldn't be run without neil add test? That seems a bit confusing.

Other than that quibble, yes, everything else sounds good -- including dropping use of Calendar (that was pure paranoia on my part instead of just using the code in your commit).

borkdude commented 2 years ago

Yes, that's a good point. We could add the first test as part of neil add test I guess. So let's say we leave that out. Then the only change compared to scratch would be that the generated namespace respects --name. Wouldn't you agree that we could just make that change to scratch, or would you consider that a breaking change?

seancorfield commented 2 years ago

There is a way to make it possible to rename scratch.clj -- update the template.edn to contain :scratch/file "scratch" and add a mapping to copy src/scratch.clj to src/{{scratch/file}}.clj and then you could invoke it specifying :scratch/file as an option behind the scenes. Would that work for you?

seancorfield commented 2 years ago

Like this:

seanc@Sean-win-11-laptop:~/oss/deps-new$ clojure -X org.corfield.new/scratch :name play :scratch/file ground
Creating project from org.corfield.new/scratch in play
seanc@Sean-win-11-laptop:~/oss/deps-new$ tree play
play
├── deps.edn
└── src
    └── ground.clj

1 directory, 2 files

seanc@Sean-win-11-laptop:~/oss/deps-new$ tree resources/org/corfield/new/scratch
resources/org/corfield/new/scratch
├── root
│   └── deps.edn
├── src
│   └── scratch.clj
└── template.edn

2 directories, 3 files
seanc@Sean-win-11-laptop:~/oss/deps-new$ cat resources/org/corfield/new/scratch/template.edn
{:scratch/file "scratch"
 :transform
 [["src" "src"
   {"scratch.clj" "{{scratch/file}}.clj"}]]}
borkdude commented 2 years ago

Almost... If it does also support a two-level directory, like foo/bar.clj then it would be sufficient. Single segment namespace aren't a great start imo.

seancorfield commented 2 years ago

Specify a qualified :scratch/file:

seanc@Sean-win-11-laptop:~/oss/deps-new$ clojure -X org.corfield.new/scratch :name play :scratch/file play/ground
Creating project from org.corfield.new/scratch in play
seanc@Sean-win-11-laptop:~/oss/deps-new$ tree play
play
├── deps.edn
└── src
    └── play
        └── ground.clj

2 directories, 2 files
borkdude commented 2 years ago

Excellent, I'll use that :)

seancorfield commented 2 years ago

Ah, that doesn't update the ns ... just a second ...

seancorfield commented 2 years ago

OK, try the latest commit. It accepts :scratch which generates :scratch/file and :scratch/ns so qualified names work properly.

seancorfield commented 2 years ago

Feel free to send a PR to fix bb compatibility -- I noticed it already applied to @rads fork of deps-new and let me know if you need anything else changed. Once that's all in, I'll tag a new version.

borkdude commented 2 years ago

Here's the PR to make deps-new bb compatible: https://github.com/seancorfield/deps-new/pull/33 Thanks!

borkdude commented 2 years ago

@rads Has tried the latest commits and everything looks good. After the release we can move back to the original deps-new repo.

seancorfield commented 2 years ago

Merged. Is anything else needed at this point or shall I go ahead and tag a new release?

borkdude commented 2 years ago

All good!