Add option to exclude IPs

[ilyes-ced]

fixing old PR

solves issue #283
adds possibility to exclude an ip address x.x.x.x or an ip address with a port x.x.x.x:xxxx
can also exclude multiple ips at the same time with bandwitch -e x.x.x.x -e y.y.y.y --excluded-ipv4-port z.z.z.z:zzzz

[cyqsimon]

I'm probably going to be pretty critical in this review, so I would like to first clarify that your effort is greatly appreciated. Out of all our users, maybe 10% are willing to report bugs; maybe 5‰ are willing to submit PRs. So thank you. Added to that, Rust is not the easiest language to learn; in fact I dare say that most people (>80%) who talk about Rust have never written a single line. So I applaud you for your willingness to try something unfamiliar.

With that out of the way, let's get straight to it. Hopefully it's not too discouraging!

[cyqsimon]

This is pretty rigid and non-extensible. For example, it's pretty reasonable for a user to want to exclude by hostname. Or by CIDR. Do we need an option for each? And then double that if we want to support IPv6 (which we definitely do; it's not 2003).

Much better would be to declare a struct/enum HostFilter (or something like that) and implement FromStr and Display for it so that it can be used as a clap option parameter. Then all these different kinds of possible filters can be united under a single option.

[cyqsimon]

I don't think it makes sense to do this here. IMO there are two ways to achieve this logic and this is neither:

1. Apply the filtering in the sniffer threads, so that the filtered traffic is never accounted.

• This is actually not my preferred option. Sniffers are not "hostname-aware", and therefore cannot apply hostname filters if we want to support those.

2. Apply the filtering while rendering. But then this logic belongs in UIState::update instead.

[cyqsimon]

I can see why you did this. Probably got yelled at by the borrow checker (cannot borrow as mutable because it is also borrowed as immutable)? If so, this is because you tried to remove from the HashMap (there's your mutable borrow) while holding a reference to one of its items (there's your immutable borrow).

The API that will allow you to do this without cloning is HashMap::entry.

[cyqsimon]

All this being said, if you are doing this filtering in UIState::update (as previously mentioned), you likely won't need any of this. On a high level, this is because you are already copying data (from memory to the screen), so all you need is to tell it what to copy (i.e. a filter). Then there's no mutability issues involved at all.

Personally I think reducing mutability as much as possible is the best practise regardless of language.

[ilyes-ced]

that was not discouraging, thank you for taking the time to write this review so i can know where i am wrong
i will try to fix the mistakes

[ilyes-ced]

i made it the way you asked
ipV4 / ipV6 / socketV4 / socketV6 are filtred correctly at the start of ui_state::update (filtered out of network_utilization)
but i am yet to implement hostname filtering because i don't know where it is

[ilyes-ced]

added hostname filtering as well but it only filters ipV4 and not V6 , if you have any clue about how i can include V6 as well please tell me
the way it does it is by reading ip_to_host in ui::update_state and adding the ip of the excluded hostname to the UIState.excluded_ips