-
-
Notifications
You must be signed in to change notification settings - Fork 65
Add exclude
option
#53
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
Conversation
@sindresorhus let me know if I need to change anything here. Lovely tool you have in this repo! |
I think it should be an option instead. Having exclude as a separate method makes it not compose well with |
aaa , so something like this ( current update ) |
Yes, but it shouldn't even run |
Ok I think I got it , let me know |
@sindresorhus , let me know of anything else needing to be done here, thanks! |
You haven't actually done what was asked: #53 (comment) |
Not sure I follow, isn't that what this statement is for? |
@mastrzyz I'm not entirely sure what you're asking. Please just read the comment I linked to:
|
Closing this PR , don't have the cycles currently to continue work on this. |
I have cycles again and will start work here. |
@sindresorhus I have updated all unresolved comments to the best of my knowledge. |
I would recommend looking over your own diff to ensure there are no more typos, inconsistencies or things that could be improved: https://github.com/sindresorhus/get-port/pull/53/files |
We have a problem where we need to get a port for a local but we can't use specific ports due to them being blocked.
In our case, Chromium blocks them -> https://chromium.googlesource.com/chromium/src.git/+/refs/heads/main/net/base/port_util.cc
Here I added logic for a
exclude
generator in the same way we have the makeRange generator.For example :
getPort.exclude([1024,1027])
will give me a generator containing all ports from the valid TCP range excluding 1024 and 1027
[1025,1026,1028,1029,...,65535]