Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

scratch template does not use :name #32

Closed
borkdude opened this issue Jul 31, 2022 · 18 comments
Closed

scratch template does not use :name #32

borkdude opened this issue Jul 31, 2022 · 18 comments

Comments

@borkdude
Copy link
Contributor

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

@seancorfield
Copy link
Owner

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

@borkdude
Copy link
Contributor Author

borkdude commented Aug 1, 2022

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
Copy link
Owner

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
Copy link
Contributor Author

borkdude commented Aug 1, 2022

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

  • empty deps.edn
  • src/app_name/core.clj with one function (same as scratch probably)
  • test/app_name/core_test.clj with one test

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:
borkdude@76259a2
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
Copy link
Owner

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
Copy link
Contributor Author

borkdude commented Aug 1, 2022

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Contributor Author

borkdude commented Aug 1, 2022

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 added a commit that referenced this issue Aug 1, 2022
@seancorfield
Copy link
Owner

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
Copy link
Contributor Author

borkdude commented Aug 1, 2022

Excellent, I'll use that :)

@seancorfield
Copy link
Owner

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

@seancorfield
Copy link
Owner

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

@seancorfield
Copy link
Owner

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
Copy link
Contributor Author

borkdude commented Aug 2, 2022

Here's the PR to make deps-new bb compatible: #33
Thanks!

@borkdude
Copy link
Contributor Author

borkdude commented Aug 2, 2022

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

@seancorfield
Copy link
Owner

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

@borkdude
Copy link
Contributor Author

borkdude commented Aug 2, 2022

All good!

@borkdude borkdude closed this as completed Aug 2, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants