search

jlesquembre / clj-nix

Nix helpers for Clojure projects
https://jlesquembre.github.io/clj-nix/
Eclipse Public License 2.0
139 stars 20 forks source link

Ignore alias option #54

Closed bendlas closed 1 year ago

bendlas commented 1 year ago

Locking all aliases may not be desirable for every project, e.g. for development-only aliases with local paths, and such.

This PR adds the ability to ignore aliases, similar to ignoring deps files.

For some use cases, the existing ignore-deps capability might be used to achive the same result as with ignored aliases, but with some tools, that's inconvenient. E.g. shadow-cljs doesn't seem to implement support for multiple overlaid deps files.

I'm still working out how to test this and add some error handling to the options parser, but wanted to get some feedback in the meantime about the options interface and if you want this PR at all.

jlesquembre commented 1 year ago

Thanks for this PR, that's the direction I want to go.

My plan was to re-implement the CLI to be more granular about dependencies. My idea is to have the following flags:

Without include/exclude options, we include everything.

I'd like to hear your toughs about the flags. Anything to add or change?

I also thought about adding a library like tools.cli or babashka.cli, but maybe the way you are implementing it with loop-case is enough, what do you think?

I'm answering your other questions separately.

My question is how you want to proceed. As an initial implementation, it's good, but I want to add more functionality. For me, it's ok if you want to work on it. Depending on how much time you want to invest, you could implement it and get it merged to main, or we could merge a subset of the functionality into a branch, and I can take it from there.

bendlas commented 1 year ago

My question is how you want to proceed. As an initial implementation, it's good, but I want to add more functionality. For me, it's ok if you want to work on it. Depending on how much time you want to invest, you could implement it and get it merged to main, or we could merge a subset of the functionality into a branch, and I can take it from there.

Thanks to how flakes do, I'm already putting this to work for my project, and so I've got no problem if you feel like taking over. I'm also happy to help with implementing after we nail down the details.

  • To include/exclude *.edn files: --deps-include and --deps-exclude. Those should support glob patterns.
  • To include/exclude aliases: --alias-include and --alias-exclude (in both cases, exclude options takes preference over include)
  • --bb: Include bb.edn files, Probably in the end it will be an alias for --deps-include **/bb.edn
  • --lein: Include lein files (already implemented)

Without include/exclude options, we include everything.

I'd like to hear your toughs about the flags. Anything to add or change?

I'm wondering about the ergonomics of wildcards: Usually, wildcards would be expanded by the shell, so in order to pass them to deps-lock, they'd need to be quoted like --deps-include '**/bb.edn' and if user forgot to quote the wildcard, results would probably be confusing.

OTOH, if the interface was designed around shell wildcard expansion, it would also result in non-standard interface: --deps-include bb1.edn bb2.edn

Though does clj-nix really need wildcard support? What use cases are there, given that the deps-lock invokation could easily be generated by nix into a command for the flake using clj-nix?

I also thought about adding a library like tools.cli or babashka.cli, but maybe the way you are implementing it with loop-case is enough, what do you think?

Using a library seems like a good idea to me. Standard interface with generated help and all ...

jlesquembre commented 1 year ago

Ok, you convinced me about wildcard support, let's leave it out. Anyways projects don't have that many deps files, so probably it's going to cause trouble for almost no benefit.

Using a library seems like a good idea to me. Standard interface with generated help and all ...

Any suggestion? I'm not familiar with any clojure CLI library, I like tools.cli or babashka.cli because don't have any dependencies, but I'm open to suggestions.

bendlas commented 1 year ago

Any suggestion? I'm not familiar with any clojure CLI library, I like tools.cli or babashka.cli because don't have any dependencies, but I'm open to suggestions.

I haven't formed an opinion on CLI libs so far either. Maybe go with tools.cli by default? Try with both and compare outcomes?

jlesquembre commented 1 year ago

I'm thinking about merging this PR, and continuing working on the CLI on another PR, any objections?

Regarding CLI options, we can have:

No wildcards, deps-include / deps-exclude and alias-include / alias-exclude are mutually exclusive. Without any options, all deps.edn / aliases are included. Makes sense to you? Any suggestions about the option names?

Something I forgot to mention, in the current version we also have some other flags, like --jar or --patch-git-sha. I don't like to expose those flags, I'll move those somewhere else.

About the CLI libraries, I played a bit with both, no strong opinions, but I'm liking babashka.cli a bit more, probably I'll go with that one.

bendlas commented 1 year ago

I'm thinking about merging this PR, and continuing working on the CLI on another PR, any objections?

No objections.

I'm liking babashka.cli a bit more, probably I'll go with that one.

LGTM

No wildcards, deps-include / deps-exclude and alias-include / alias-exclude are mutually exclusive. Without any options, all deps.edn / aliases are included. Makes sense to you?

"all deps.edn" as in the current behavior? All files named deps.edn in subdirectories?

Any suggestions about the option names?

Your proposals seem fine to me.

jlesquembre commented 1 year ago

"all deps.edn" as in the current behavior? All files named deps.edn in subdirectories?

Yes, as in the current behavior, all deps.edn in all subdirectories