search

RyanMcG / lein-npm

Manage Node dependencies for CLJS projects
295 stars 43 forks source link

Order packages by name #22

Open binarykitchen opened 9 years ago

binarykitchen commented 9 years ago

Currently when you do a direct npm install, it sorts the dependencies by name in the package.json. Do you think you could do the same here? That would be great. Thanks

RyanMcG commented 9 years ago

Do you mean to say that package.json is modified by npm install?

binarykitchen commented 9 years ago

Yes. Every npm install reorders the list alphabetically. Check it out yourself. Would be good to have the same behavior here for the project.clj :)

RyanMcG commented 9 years ago

Generating a package.json with the dependencies in order seems like a good idea (not sure if we do that already). However, I'd prefer not to modify a source file without making that action explicit. Leiningen does not, why should we?

binarykitchen commented 9 years ago

Well, having the package list sorted alphabetically is very nice for the developer's eyes. In our project we have over 40 npm packages. Scanning through these when not sorted is a pain.

If that list were sorted, we would save some seconds looking for packages. Makes it easier to visually parse the file.

In regards to modifying a source, I do not see a problem here because adding a new npm package requires a new line in the project.clj and that file has to be modified anyway.

RyanMcG commented 9 years ago

For all users who aren't adding a new dependency, this would modify their project.clj next time they run npm install. There are other ways to sort this list without adding unnecessary complexity to lein-npm.

On Thu, Feb 19, 2015 at 3:22 PM, Michael Heuberger <notifications@github.com

wrote:

Well, having the package list sorted alphabetically is very nice for the developer's eyes. In our project we have over 40 npm packages. Scanning through these when not sorted is a pain.

If that list were sorted, we would save some seconds looking for packages. Makes it easier to visually parse the file.

In regards to modifying a source, I do not see a problem here because adding a new npm package requires a new line in the project.clj and that file has to be modified anyway.

— Reply to this email directly or view it on GitHub https://github.com/RyanMcG/lein-npm/issues/22#issuecomment-75160478.

binarykitchen commented 9 years ago

Not entirely correct. package.json is only modified when you append the --save flag. We could do this here too. Modify project.clj only on lein npm install --save. IMO this is fine and a clear indication from the user to add a new dependency on the list and this implies that reordering the list alphabetically afterwards should be fine too.

RyanMcG commented 9 years ago

I'm OK with that. It was not clear to me from your description that an implicit flag was necessary for modifying the file.

On Fri, Feb 20, 2015 at 2:25 PM, Michael Heuberger notifications@github.com wrote:

Not entirely correct. package.json is only modified when you append the --save flag. We could do this here too. Modify project.clj only on lein npm install --save. IMO this is fine and a clear indication from the user to add a new dependency on the list and this implies that reordering the list alphabetically afterwards should be fine too.

Reply to this email directly or view it on GitHub: https://github.com/RyanMcG/lein-npm/issues/22#issuecomment-75330937

binarykitchen commented 9 years ago

Great. Sorry for not mentioning the --save flag earlier. Ok, looking forward to see this auto-sorting functionality in the next version ;)

RyanMcG commented 9 years ago

You are more than welcome to contribute a pull request. Unfortunately, I feel like this might be harder to implement than it is with npm. project.clj is not just data, it is a Clojure code. Perhaps something like rewrite-clj would be helpful, of course that means unnecessary dependencies.

On Fri, Feb 20, 2015 at 4:06 PM, Michael Heuberger <notifications@github.com

wrote:

Great. Sorry for not mentioning the --save flag earlier. Ok, looking forward to see this auto-sorting functionality in the next version ;)

— Reply to this email directly or view it on GitHub https://github.com/RyanMcG/lein-npm/issues/22#issuecomment-75342392.

binarykitchen commented 9 years ago

Thanks man. I am afraid I do not know any Clojure. Writing all the code in JavaScript here at work.

Take your time to implement this. There is no rush. I'd say, low priority ;)

RyanMcG commented 9 years ago

Well Michael, despite being OK with the interface, unless there is higher demand for this feature I'd rather not implement it.

On Fri, Feb 20, 2015 at 10:13 PM, Michael Heuberger notifications@github.com wrote:

Thanks man. I am afraid I do not know any Clojure. Writing all the code in JavaScript here at work.

Take your time to implement this. There is no rush. I'd say, low priority ;)

Reply to this email directly or view it on GitHub: https://github.com/RyanMcG/lein-npm/issues/22#issuecomment-75358204

binarykitchen commented 9 years ago

that's fine - as i said before: low priority for now.