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

Feature/20 monorepo support #24

Merged
merged 4 commits into from
Mar 7, 2023

Conversation

copdsUcn
Copy link
Contributor

Hi!
A few days ago, when I was trying to use a monorepo, I encountered the problem that in your repo was already mentioned and documented as issue #20.

I found the idea of recursively searching node_modules/.bin directories a bit wild. Forty's idea of allowing a list of commands albeit quite sexy.

I took the liberty of sending you a PR that implements this function. It looks like a lot of code, but it's just a bunch of helper functions and the proper remodeling of the add-node-modules-path interactive function.
Additionally a short paragraph in the README.md as well as ert tests.

Of course, the implementation is backwards compatible; it is still possible to use the default value of add-node-modules-path-command or override it with a single command.

Due to the fact that with several commands some commands can work well, but others don't, I had to adjust the reporting. Directories that could be added to exec-path are reported using (message ...). Commands that failed are reported as warnings in the Warnings buffer.

Maybe you are interested in the extension. otherwise just ignore :)

best regards from austria,
Max

@codesuki
Copy link
Owner

Great contribution! Thank you! Just a small nit: what about making the commands a list instead of a comma separated string?

@copdsUcn
Copy link
Contributor Author

copdsUcn commented Feb 28, 2023

This would definitely be the cleaner solution.
I will soon provide an update of the PR that is also backward compatible regarding the according defcustom :type

@copdsUcn copdsUcn force-pushed the feature/20-monorepo-support branch 2 times, most recently from de80724 to 7e05120 Compare March 3, 2023 08:31
…strings, so that multiple commands can be specified as list, not as comma-separated values in a string.
@copdsUcn copdsUcn force-pushed the feature/20-monorepo-support branch from 7e05120 to 006ed8a Compare March 3, 2023 08:41
@copdsUcn
Copy link
Contributor Author

copdsUcn commented Mar 3, 2023

Hi!
I updated the PR. Multiple commands are now specified as list instead of a comma-separated string.
The defcustom :type was changed from string to (repeat string). To ensure backward compatibility, a custom :set function was added as well to convert a string value to a single-element list of this value. This way, users with an already existing customization in their config are able to open the customization UI and see their string value properly converted and displayed.

@codesuki
Copy link
Owner

codesuki commented Mar 7, 2023

Very cool! Thanks a lot for working on this!

@codesuki codesuki merged commit 841e93d into codesuki:master Mar 7, 2023
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

Successfully merging this pull request may close these issues.

2 participants