Closed triole closed 6 months ago
Hello @triole, I appreciate your effort in submitting this PR, and your kind words mean a lot. It's great to hear that you find cidr
useful! I have a few comments on the pull request and would like to discuss them with you.
cidr
is to be agnostic to both IPv4 and IPv6, offering features that naturally support both, as per the existing CLI features.Let's discuss and brainstorm on these points.
Hi,
thanks for your ideas and comments. I am glad that we can discuss here to help each other understand a little better. Let me try to explain why I find this feature useful.
But first let me say that regarding the implementation you are absolutely right. It wasn't a good idea to store the addresses inside a list and print them later. Something that I also noticed a little later but at this point the pull request was already made. I guess sometimes it's better to sleep over things. Anyhow I updated this and hope that it is solved better now.
Concerning your other point that an address list of bigger cidr ranges can grow quite large I must admit that I do not see this as a big problem. I suppose the most use cases are just checking smaller ranges and have a look at which addresses belong to these. But of course you could also try larger ones. From my point of view this is a feature that I would not decide to keep back from users just because it might end up in a print orgy that by the way can always be aborted pressing ctrl+c.. I would leave the decision about the usefulness of the list feature to the users. If someone wants to have a look at thousands of ip addresses, why not? Furthermore there might be occasions where people just need these kind of large lists to process them further with shell tools like grep, tail, head or anything else. You could also think of piping lists into files and continue working with them from there.
So anyway the point is that I think I get what you mean, but I would not decide to not offer this feature. There might be people who have use cases for it. So let the user decide what to do with it.
I hope the above makes senses and helps a little.
Cheers.
Hi @triole,
It's great to hear that you've revisited the implementation and made updates to address the concerns regarding storing addresses in a list. Taking a step back and reconsidering the approach can indeed lead to more robust solutions.
My primary concern is around the scalability of the feature. As CIDR ranges with small prefix lengths can indeed be more manageable, the situation might become less favorable as the prefix length increases. Handling thousands or even millions of IP addresses could lead to performance issues and might not align with the intended user experience.
Considering these factors, I wonder if there could be a middle ground that allows users to explore smaller CIDR ranges more interactively while also providing warnings or limitations for larger ranges. This approach could strike a balance between user experience and cidr
's efficiency.
I'm not fully convinced yet of the ideal solution, and I believe it's a complex matter that requires careful consideration. It's great that you're open to feedback @triole , and I look forward to seeing how this feature develops - what are your thoughts on this @DanielRieske, @mmourick and @mvanholsteijn?
Hi :wave: @bschaatsbergen @triole
Aside from scalability concerns that I absolutely agree with, I do have some questions regarding usability. I am wondering how useful this feature would be for larger CIDR ranges, while I won't dispute that it has its merits for smaller CIDR ranges I would look into options to make more of a summary of a given CIDR range.
Example, given CIDR range 172.16.0.0/24
you can print out a summary with the following details
IP Address | Network Address | Usable Host IP Range | Total Number of Hosts |
---|---|---|---|
172.16.0.0/24 | 172.16.0.0 | 172.16.0.1 - 172.16.0.254 | 256 |
This approach can also be used to calculate IPV6 addresses, therefore we keep supporting both.
Curious what you guys think of this approach
Hi 👋 @bschaatsbergen @triole
Aside from scalability concerns that I absolutely agree with, I do have some questions regarding usability. I am wondering how useful this feature would be for larger CIDR ranges, while I won't dispute that it has its merits for smaller CIDR ranges I would look into options to make more of a summary of a given CIDR range.
Example, given CIDR range
172.16.0.0/24
you can print out a summary with the following detailsIP Address Network Address Usable Host IP Range Total Number of Hosts 172.16.0.0/24 172.16.0.0 172.16.0.1 - 172.16.0.254 256 This approach can also be used to calculate IPV6 addresses, therefore we keep supporting both.
Curious what you guys think of this approach
Appreciate you sharing your view on this, @DanielRieske! Your input aligns with my current thinking, and I'm actively incorporating it into #75 as I'm typing this.
Hi @bschaatsbergen and @DanielRieske ,
I appreciate your feedback and I totally see your point regarding the scalability of the list feature. Nonetheless I do tend to leave the question of usability up to each individual who considers using the feature. Maybe a warning before printing too many addresses would be a reasonable middle ground that we all could settle with. Although of course immediately the question arises, how many actually are too many.
The explain
feature definitely has its use and I'm excitedly looking forward to see it implemented. I think it'll be very handy but list
is a different thing on purpose. Sometimes I just need stupid plain lists of IP addresses to be generated. As said I wouldn't limit the possibilities of list
but of course I can live with the fact that you see this differently.
Thanks for the constructive discussion. I really enjoy it.
Cheers.
Hi @bschaatsbergen and @DanielRieske ,
I appreciate your feedback and I totally see your point regarding the scalability of the list feature. Nonetheless I do tend to leave the question of usability up to each individual who considers using the feature. Maybe a warning before printing too many addresses would be a reasonable middle ground that we all could settle with. Although of course immediately the question arises, how many actually are too many.
The
explain
feature definitely has its use and I'm excitedly looking forward to see it implemented. I think it'll be very handy butlist
is a different thing on purpose. Sometimes I just need stupid plain lists of IP addresses to be generated. As said I wouldn't limit the possibilities oflist
but of course I can live with the fact that you see this differently.Thanks for the constructive discussion. I really enjoy it.
Cheers.
Hey @triole,
I've been doing some tinkering about this and please allow me to get back on this in a couple days :)
Meawhile, what's your view on the explain
subcommand we've added?
Hi, no worries. It is absolutely fine not to answer or expect an answer on the same day. I know how it is to be busy all the time and therefore being forced to postpone things over and over again. It's all fine.
But to answer your question: I like the explain
command and find it quite useful to get some information about network ranges, netmask, the broadcast address and so on. As I said earlier this is a nice feature that definitely has its purpose. Nonetheless the use case is different from the list
feature because if you need a stupid plain list of IP addresses to further process them somewhere else list
would be the way to go.
I guess you know my standpoint. I'd like to see both features implemented. But I totally respect if you decide otherwise.
Cheers.
Thank you for your understanding, and we appreciate your input on the explain
command and list
feature. At this time, we're not planning to implement the list
feature, but your feedback is valuable. If there's increased community support or demand for this particular feature, we'll certainly reconsider. @triole
Hi there,
thanks for this useful little tool. I thought it would be nice to just have a simple
list
command that prints a list of the addresses in a given cidr range. I am not sure if you find it useful as well but if so, please feel free to merge this pull request.Cheers.