-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Profile merge concatenates things that don't make sense #878
Comments
I think the right thing to do here is to make the |
This is an issue more general than just for {:aliases {"test" ["do" "test," "midje"]}} then having {:aliases {"test" "midje"}} will give you a type mismatch, and having {:aliases {"test" ["do" "test," "midje"]}} Will attempt to do Instead of solving this specific case, I think it would be nice with a solution that can handle multiple cases and minimizes work if/when another case pops up. |
Oy; quite right. This is tricky; it seems that We could enumerate all the non-merge keys in Leiningen itself, but plugins could introduce their own keys that need special handling too. I can't think of a simple way to do this; it may get messy. |
In the mean time I think |
Oh... the problem with doing this in the |
|
FWIW, in pallet we use https://github.com/pallet/pallet-map-merge to do key specific merging. |
Gotta love how quote silently discards metadata wooooo.
I'm starting to think the best thing to do here is to default to |
what is the status of this? any updates? |
I have some notes on this, but this won't be done before 3.0.0 to keep backwards compatibility. |
Is this a benign / ignorable error or does it actually cause problems? |
Regardless of whether the types match, I'd expect the last value to win / override.
Really? You'd expect adding :dependencies in your :dev profile to wipe out the :dependencies from the top-level defproject map? This is not how most users expect it to work.
Is this a benign / ignorable error or does it actually cause problems?
When people expect it to work one way, and it works differently, there's always plenty of potential for problems.
|
To clarify: I only meant when there's a type conflict. Always merge when possible. Fallback to replace when not. |
Always merge when possible. Fallback to replace when not.
I see--this issue is actually about the fact that "always merge when possible" is not always the correct choice--merging two lists like `(require 'xyz)` and `(println "abc")` gives you `(require 'xyz println "abc")`, which is wrong. You can see some examples of this in the comments above.
|
Indeed, still a problem. I found it when using a combination of See the specific whidbey source and leiningen issue #1997
$ lein with-profile +whidbey/repl,+repl cprint :repl-options
{:init (do
(clojure.core/require (quote whidbey.repl))
(whidbey.repl/init! nil)
go),
:nrepl-context {:interactive-eval {:renderer whidbey.repl/render-str}},
:nrepl-middleware [clojure.tools.nrepl.middleware.render-values/render-values],
$ lein with-profile +repl cprint :repl-options
{:init (go), :timeout 600000}
$ lein with-profile +whidbey/repl cprint :repl-options
{:init (do (clojure.core/require (quote whidbey.repl)) (whidbey.repl/init! nil)),
:nrepl-context {:interactive-eval {:renderer whidbey.repl/render-str}},
:nrepl-middleware [clojure.tools.nrepl.middleware.render-values/render-values]} Notice, though that the expanded form is not the same as the source. Also, the parens around $ lein with-profile +whidbey/repl,+repl cprint :repl-options
{:init (do
(clojure.core/require (quote whidbey.repl))
(whidbey.repl/init! nil)
(go)),
:nrepl-context {:interactive-eval {:renderer whidbey.repl/render-str}},
:nrepl-middleware [clojure.tools.nrepl.middleware.render-values/render-values],
:timeout 600000} The only "workaround" is to remove my |
Leiningen doesn't know how to merge this option - see technomancy/leiningen#878. Instead, use ^:replace metadata to overwrite it completely with a form that evaluates any existing value before evaluating our init code.
Leiningen doesn't know how to merge this option - see technomancy/leiningen#878. Instead, use ^:replace metadata to overwrite it completely with a form that evaluates any existing value before evaluating our init code.
Leiningen doesn't know how to merge these - see technomancy/leiningen#878. Instead, use ^:replace metadata to overwrite it completely with a form that evaluates any existing value before evaluating our init code.
Leiningen doesn't know how to merge these - see technomancy/leiningen#878. Instead, use ^:replace metadata to overwrite it completely with a form that evaluates any existing value before evaluating our init code.
At this point any changes to this behavior are likely to be more disruptive than the problematic behavior itself. |
If I have this in
~/.lein/profiles.clj
and this in my
project.clj
:then the merged
:repl-options
end up being:because of meta-merge.
Ideally there would be something like a list of all the forms - is the full list of profile settings exposed anywhere that would allow the repl task to do something custom? I'm not sure what the right thing to do here would be. :(
The text was updated successfully, but these errors were encountered: