Macchina-CLI / libmacchina

A library providing access to all sorts of system information.
https://crates.io/crates/libmacchina
MIT License
68 stars 20 forks source link

Use 'local-ip-address' crate instead of 'if-addrs' #94

Closed FantasyTeddy closed 3 years ago

FantasyTeddy commented 3 years ago

This change fixes #88 but since the discussion there was not final, I would understand if the alternative method would be preferred.

The implementation in this pull request would also allow to not specify an interface. In this case the local-ip-address crate chooses some sensible defaults to determine the local IP address.

grtcdr commented 3 years ago

I don't think switching to a less customizable IP crate is a good idea, maybe temporarily for Windows, but not the other platforms.

FantasyTeddy commented 3 years ago

I don't quite understand how this crate is less customizable than if-addrs, it is still possible to choose an explicit interface. Could you elaborate what you mean?

grtcdr commented 3 years ago

Sorry.

The implementation in this pull request would also allow to not specify an interface.

I mistook this for an inability to specify an interface, but I get what you mean now.

grtcdr commented 3 years ago

I don't think this crate supports the BSDs, or at least it doesn't mention that. In the case that it doesn't, we should proceed by moving what's inside shared::interface() to Windows' own interface() method, and conditionally compile the local-ip-address for Windows.

FantasyTeddy commented 3 years ago

I don't think this crate supports the BSDs, or at least it doesn't mention that.

You are right, I didn't notice that. (Maybe we could add the BSDs to the GitHub Workflow to catch similar oversights?)

I tried to only use local-ip-address on Windows. Did I do that right?

grtcdr commented 3 years ago

Thank you for helping out @FantasyTeddy :heart:

FantasyTeddy commented 3 years ago

Thank you for your patience with me 😅

grtcdr commented 3 years ago

My pleasure :heart: