Closed deafmute1 closed 2 years ago
@moritzheiber any concerns here?
No, assuming that it's been tested I don't see any reason for not merging the PR
Nice, I'd love to see this merged! :pray:
@jc00ke we're just looking for someone to test and verify it works. Can you do that?
Sure, what's the best way to test this specific change? Manually edit the script? I have wireguard set up already, happy to poke at it now.
Great! Yeah since it's not packaged you'll need to download and update the file. So assuming you already have i3xrocks-nm-vpm
installed, make a back up of /usr/share/i3xrocks/nm-vpn
, copy the change, refresh your desktop session and see if the wireguard status info works as expected. Once you're done you can copy your backup back to /usr/share/i3xrocks/nm-vpn
.
I installed and then manually edited the file since it was only a few lines.
However...
~
❯ LC_ALL=C nmcli -t connection show --active
home:756b8315-2de5-4810-a889-eb79e88ad448:802-11-wireless:wlp2s0
igor:0be2c5f2-d122-4826-93ed-f86a4b4c4573:wireguard:igor
tailscale0:065e0392-4498-4c8f-90d5-b9e66e3dc083:tun:tailscale0
~
❯ LC_ALL=C nmcli -t connection show --active | awk -F ':' '
{ if($3 == "vpn") {
vpn_name=$1
} else if ($3 == "tun"){
tun_name=$1
} else if ($3 == "tap"){
tun_name=$1
} else if ($3 == "wireguard"){
tun_name=$1
}
}
END{if (vpn_name) {printf("%s", vpn_name)} else if(tun_name) {printf("%s", tun_name)}}'
tailscale0⏎
So I'd say it works, but it's also not what I expected. Because I have Tailscale installed, and it alphabetically is after my wireguard connection, the block will always return tailscale0
, even if igor
is active. But that could be a separate issue.
To make sure the wireguard
part worked, I replaced tun
with wireguard
and when my wireguard connection was active, it showed up. When I deactivated it, it disappeared. I should have been explicit in confirming that earlier.
Thanks for doing the test @jc00ke . I'm not really familiar with the use case here. What is the ideal experience (provides best experience for everyone)? Since it produced something you didn't expect, is there a way to update the script to make it better?
Good question. If I didn't have Tailscale installed as a tun
then I think just showing the connected tunnel name would be fine. But now I'm wondering if a better experience for multiple enabled devices might be a ":shield: x N" when N > 1 is enabled & a click would pop up a window (like the CPU block) or a rofi window with more info. Another one.
Hmm, is it common for people to use more than one VPN connection concurrently? I use a VPN for my day job, but just the one. Another option would be to just print all active VPN connections to the bar. There is nothing preventing that to my knowedge.. The Rofi scripts look interesting as well.
Ah, OK, so I think this is a strange aspect of Tailscale. nmcli
reports it as active even though the tunnel is down. I'll open an issue over there.
I doubt people generally have more than 1 VPN connection active at a time. You'd have to route traffic and it seems like an edge case not worth supporting here.
@jc00ke sorry I've lost track of this PR. Can you LMK if this looks good to merge for testing or is blocking on something else?
@kgilmer It's good to merge as is!
Hi @deafmute1 , thanks for your contribution! Before we merge it in, can you give some brief detail about what testing you've done?