Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Make minimum number of map entries lift-ns? requires configurable #325

Open
eval-on-point opened this issue Jul 2, 2024 · 2 comments
Open

Comments

@eval-on-point
Copy link

From the changelog:

{:map {:lift-ns? false}} -- the default for :map :lift-ns? was changed to false from true, largely because formatting deps.edn files looks ugly when the :mvn is lifted out. An alternative would have been to only lift a namespace out of a map if there were more than one key. Please submit an issue if you feel strongly about this one way or the other and we can discuss it.

I was discussing the lift-ns? option with my team this morning and we all (N=3) agreed that we would prefer namespaces to be lifted if there were more than one key in a map, but not to lift it if there were only one. We thought it would be slick if lift-ns? could optionally take in integer argument instead of true. This would set the minimum number of map elements required before zprint performs a namespace lift in a backwards compatible way.

@kkinnear
Copy link
Owner

kkinnear commented Jul 8, 2024

Many thanks for your suggestion! I think it is a reasonable approach, though it hurts a bit to put something in addition to true and false an option for a key ending in ?. Still, I don't really think we need yet another key-value pair here, so I'll see what I can to do to make this work. I'll let you know when I've figured it out and finished it.

@eval-on-point
Copy link
Author

That is an interesting philosophical point and is possibly evidence that ? should be reserved only for boolean-valued predicates. Check out bbatsov/clojure-style-guide#182 (comment) for an interesting perspective on this. I am not exactly sure where I land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants