internal/ui: nicer sorting and display of Mullvad nodes (#116)

* internal/tsutil: add `CompareLocations()`

* internal/ui: better sorting and display of Mullvad nodes

* internal/ui: preemptively protect against `nil` `*tailcfg.Location`s

* internal/ui: add some TODOs

* internal/tsutil: fix an incorrect comment

internal/tsutil/client.go

@@ -241,8 +241,8 @@ func (c *Client) DeleteWaitingFile(ctx context.Context, name string) error {
 	return localClient.DeleteWaitingFile(ctx, name)
 }
 
-// WaitingFiles polls for any pending incoming files. It blocks for an
-// extended period of time.
+// WaitingFiles polls for any pending incoming files. It returns
+// quickly if there are no files currently pending.
 func (c *Client) WaitingFiles(ctx context.Context) ([]apitype.WaitingFile, error) {
 	// TODO: https://github.com/tailscale/tailscale/issues/8911
 	return localClient.AwaitWaitingFiles(ctx, time.Second)

internal/tsutil/tsutil.go

@@ -1,11 +1,13 @@
 package tsutil
 
 import (
+	"cmp"
 	"fmt"
 	"strings"
 
 	"golang.org/x/net/idna"
 	"tailscale.com/ipn/ipnstate"
+	"tailscale.com/tailcfg"
 	"tailscale.com/util/dnsname"
 )
 
@@ -39,3 +41,12 @@ func IsMullvad(peer *ipnstate.PeerStatus) bool {
 func CanMullvad(peer *ipnstate.PeerStatus) bool {
 	return peer.CapMap.Contains("mullvad")
 }
+
+// CompareLocations alphabestically compares the countries and then,
+// if necessary, cities of two Locations.
+func CompareLocations(loc1, loc2 *tailcfg.Location) int {
+	return cmp.Or(
+		cmp.Compare(loc1.Country, loc2.Country),
+		cmp.Compare(loc1.City, loc2.City),
+	)
+}

internal/ui/mullvadpage.go

@@ -111,8 +111,11 @@ func (page *MullvadPage) Update(a *App, peer *ipnstate.PeerStatus, status tsutil
 			}
 		}
 	}
-	slices.SortFunc(nodes, func(p1 *ipnstate.PeerStatus, p2 *ipnstate.PeerStatus) int {
-		return cmp.Compare(p1.DNSName, p2.DNSName)
+	slices.SortFunc(nodes, func(p1, p2 *ipnstate.PeerStatus) int {
+		if (p1.Location == nil) || (p2.Location == nil) {
+			return cmp.Compare(p1.HostName, p2.HostName)
+		}
+		return tsutil.CompareLocations(p1.Location, p2.Location)
 	})
 
 	page.exitNodeRows.Update(nodes)
@@ -139,7 +142,16 @@ func (row *exitNodeRow) Widget() gtk.Widgetter {
 }
 
 func mullvadExitNodeName(peer *ipnstate.PeerStatus) string {
-	return fmt.Sprintf("%v %v, %v", countryCodeToFlag(peer.Location.CountryCode), peer.Location.Country, peer.Location.City)
+	if peer.Location == nil {
+		return peer.HostName
+	}
+
+	return fmt.Sprintf(
+		"%v %v, %v",
+		countryCodeToFlag(peer.Location.CountryCode),
+		peer.Location.City,
+		peer.Location.Country,
+	)
 }
 
 func countryCodeToFlag(code string) string {

internal/ui/page.go

@@ -22,6 +22,8 @@ type Page interface {
 	// values to their defaults and whatnot. It should not call Update
 	// unless doing so is idempotent, though even then it's better not
 	// to.
+	//
+	// TODO: Remove this and just do it in constructors instead.
 	Init(*App, *ipnstate.PeerStatus, tsutil.Status)
 
 	// Update performs an update of the UI to match new state.

internal/ui/rowmanager.go

@@ -6,6 +6,8 @@ import (
 	"github.com/diamondburned/gotk4/pkg/gtk/v4"
 )
 
+// TODO: Move this into a different package and remove the need to keep
+// track of the parent and other pieces of tight coupling to GTK.
 type rowManager[Data any] struct {
 	Parent rowManagerParent
 	New    func(Data) row[Data]