netspeed: Show all IPv4 and IPv6 addresses in tooltip

[treysis]

Fixes #557.

[treysis]

Argh, how do I get rid of all the previous commits? Always when I try to rebase it will delete all my previous changes....I'm sorry, but I guess I have to leave this up to you. I'll never get around the way git works.

[vkareh]

Do you want only the last commit? Try doing this: git rebase -i HEAD~4 and follow the instructions to drop commits from that list. Save and exit, and you will then have only the remaining commit

[treysis]

I don't know. I thought I rebased to the current master branch, created a new branch for my fix, did my adjustments to the code, pushed it, opened pull request. No idea why it shows the other commits in this PR. I'll better leave it as it is now.

[vkareh]

let's fix it! what do you see when you do git remote -v?

[treysis]

origin  https://github.com/treysis/mate-applets (fetch)
origin  https://github.com/treysis/mate-applets (push)

[vkareh]

huh, how are you updating your master then? you should add the upstream as a remote: git remote add upstream git@github.com:mate-desktop/mate-applets.git then you can actually update:

$ git checkout master # checks out your local master
$ git fetch upstream # fetches all latest changes from the upstream remote
$ git pull upstream master # merges all changes from upstream into your local master
$ git push # pushes those changes to your origin branch on github

Now you can do git rebase master on your ipv6-tooltip-all branch. Once you rebase master, you'll need to git push --force your ipv6-tooltip-all so that it overrides all the previous junk

[treysis]

huh, how are you updating your master then?

I did that. But then I got confused about everything and decided to remove my local copy and clone it again. After cloning I just had forgotten to add the upstream repo again.

I'll try your suggestions. Thx!

EDIT: The git pull upstream master causes a merge, and thus 1 new commit.

EDIT2: It just doesn't make sense to me. It would take one or two more hours and a lot of trial and error to understand what I am doing. Without understanding it, it will be very unsatisfying so I'll leave it where it is now. I need to use my time for different tasks than to understand git. Sorry, and thanks again for your help.

[rbuj]

tip: before starting a new pr, first update your local and remote copy as @vkareh pointed.

git clone git@github.com:treysis/mate-applets.git
cd mate-applets
git remote add upstream https://github.com/mate-desktop/mate-applets.git
git fetch upstream
git checkout master
git merge upstream/master
git push

note: you only have to do the steps 1 and 3 once

[lukefromdc]

Since I have no landline and only connect by tethering I don't have a way to test an IPv6 address.

[treysis]

IPv6 works also with tethering, if your carrier supports it.

Anyways, for testing you can just rely on link-local addresses or you can just configure dummy addresses as static addresses and check it out.

[lukefromdc]

I can never call customer service as I am keeping my name from them(prepaid plans only). I don't know enough about networking to really experiment with this, didn't even have the Internet at home when I started playing with software.

[treysis]

I cleaned it up and rebased to latest master (as of now). Should I open a new PR @vkareh?

[treysis]

Still any objections?

[rbuj]

I don't think the best option is to set the show-all-addresses key to true by default, since as I mentioned above the ipv6 addressing is not the most used at the moment, moreover it doesn't provide any useful information when an interface is just ipv6 link-local, or loopback. For this reason I cannot approve this pull request. If we omit this objection the other changes seem correct.

[raveit65]

I agree to don't show them as default in tooltip, because it's not a big thing to enable it in preferences. Also , an admin or distro maintainer can always do a gsettings override, which is easy to do, when he think this would be a better default for his distro or let say 1000 machines he have to administrate. For fedora i have done this at several places ;) So, please change that to false and we can merge, it if there are no other concerns.

[treysis]

Ok, while I still disagree, I changed it.

[treysis]

Yes. Should it better read no IPv4? I left it this way as it is more in line with how someone unaware of IPv6 would expect and how it used to be in the past.

[rbuj]

The tooltip displayed that an interface "has no ip", when it uses only ipv6 addressing and the "show all ip addresses on tooltip" option is unchecked.

In the example below the interface has no ipv4 address but has two ipv6 addresses, however in this case it only considers ipv4 addresses.

[treysis]

I know. Do you suggest that in this case it should show the IPv6 addresses, even if the checkbox is unchecked? Then, I would suggest to change the checkbox to "Show IPv6 addresses" instead of "all addresses".

[rbuj]

The get_ip6_address_list function returns the ipv6 addresses sorted alphabetically, in this case you can display only one ipv6 address, perhaps the most important (https://en.wikipedia.org/wiki/IPv6_address#Special_addresses)

See https://github.com/mate-desktop/mate-applets/pull/578#issuecomment-755941019 and https://github.com/mate-desktop/mate-applets/pull/578#issuecomment-755946572

[treysis]

I guess it should be the GUA address with Privacy Extensions > Manual > Stable Privacy > EUI-64 (depending on what's available). But this is also somewhat user-dependent. And I am not so convinced that this is a meaningful approach in general. I would rather change the checkbox to read Show IPv6 addresses on tooltip, and then change the message from has no IP to no IPv4.

[rbuj]

At least all ipv6 addresses should be displayed if ipv4 address is not used, instead of showing has no ip.

[rbuj]

::/128 is equivalent to has no ip in ipv6, ! (strlen (ipv6_text) > 2) in the old code.

[treysis]

At least all ipv6 addresses should be displayed if ipv4 address is not used, instead of showing has no ip.

I went with this approach for now as it was quite easy to implement. IMO this should be merged to provide at least a very basic IPv6 support. I am open to refining this in the future!

[treysis]

it always displays ipv6 addresses from the second line of the tooltip

That is intentional. Since IPv6 addresses are usually longer, it would make the tooltip window even wider. And imo it looks nicer if all IPv6 addresses have the same indentation, i.e. beginning of the line.

[treysis]

Can we get this merged?

[treysis]

Rebased to current master....thx for teaching me git 😊

[raveit65]

So many changes with netspeed-applet in the last weeks, but showing the important ip address is missing. ;) The result of curl ifconfig.me Very important when you use a vpn connection.