use regex replace instead of parsing urls and fix replacing username

Author: karolzwolak

pkg/gui/controllers/remotes_controller.go

@@ -2,7 +2,7 @@ package controllers
 
 import (
 	"fmt"
-	"net/url"
+	"regexp"
 	"strings"
 
 	"github.com/jesseduffield/gocui"
@@ -186,79 +186,37 @@ func (self *RemotesController) add() error {
 	return nil
 }
 
-// replaceForkUsername replaces the "owner" part of a git remote URL with forkUsername,
-// preserving the repo name (last path segment) and everything else (host, scheme, port, .git suffix).
-// Supported forms:
-//   - SSH scp-like:   git@host:owner[/subgroups]/repo(.git)
-//   - HTTPS/HTTP:     https://host/owner[/subgroups]/repo(.git)
-//
-// Rules:
-//   - If there are fewer than 2 path segments (i.e., no clear owner+repo), return an error.
-//   - For multi-segment paths (e.g., group/subgroup/repo), the entire prefix is replaced by forkUsername.
+var (
+	// 1. SCP-like SSH: git@host:owner[/subgroups]/repo(.git)
+	sshScpRegex = regexp.MustCompile(`^(git@[^:]+:)([^/]+(?:/[^/]+)*)/([^/]+?)(\.git)?$`)
+
+	// 2. SSH URL style: ssh://user@host[:port]/owner[/subgroups]/repo(.git)
+	sshUrlRegex = regexp.MustCompile(`^(ssh://[^/]+/)([^/]+(?:/[^/]+)*)/([^/]+?)(\.git)?$`)
+
+	// 3. HTTPS: https://host/owner[/subgroups]/repo(.git)
+	httpRegex = regexp.MustCompile(`^(https?://[^/]+/)([^/]+(?:/[^/]+)*)/([^/]+?)(\.git)?$`)
+)
+
+// replaceForkUsername rewrites a Git remote URL to use the given fork username,
+// keeping the repo name and host intact. Supports SCP-like SSH, SSH URL style, and HTTPS.
 func replaceForkUsername(remoteUrl, forkUsername string) (string, error) {
 	if forkUsername == "" {
-		return "", fmt.Errorf("Fork username cannot be empty")
+		return "", fmt.Errorf("fork username cannot be empty")
 	}
 	if remoteUrl == "" {
-		return "", fmt.Errorf("Remote url cannot be empty")
-	}
-
-	// SSH scp-like (most common): git@host:path
-	if isScpLikeSSH(remoteUrl) {
-		colon := strings.IndexByte(remoteUrl, ':')
-		if colon == -1 {
-			return "", fmt.Errorf("Invalid SSH remote URL (missing ':'): %s", remoteUrl)
-		}
-		path := remoteUrl[colon+1:] // e.g. owner/repo(.git) or group/sub/repo(.git)
-		segments := splitNonEmpty(path, "/")
-		if len(segments) < 2 {
-			return "", fmt.Errorf("Remote URL must include owner and repo: %s", remoteUrl)
-		}
-		last := segments[len(segments)-1] // repo(.git)
-		newPath := forkUsername + "/" + last
-		return remoteUrl[:colon+1] + newPath, nil
-	}
-
-	// Try URL parsing for http(s) (and reject anything else).
-	u, err := url.Parse(remoteUrl)
-	if err != nil {
-		return "", fmt.Errorf("Invalid remote URL: %w", err)
-	}
-	if u.Scheme != "https" && u.Scheme != "http" {
-		return "", fmt.Errorf("Unsupported remote URL scheme: %s", u.Scheme)
+		return "", fmt.Errorf("remote URL cannot be empty")
 	}
 
-	// u.Path like "/owner[/subgroups]/repo(.git)" or "" or "/"
-	path := strings.Trim(u.Path, "/")
-	segments := splitNonEmpty(path, "/")
-	if len(segments) < 2 {
-		return "", fmt.Errorf("Remote URL must include owner and repo: %s", remoteUrl)
+	switch {
+	case sshScpRegex.MatchString(remoteUrl):
+		return sshScpRegex.ReplaceAllString(remoteUrl, "${1}"+forkUsername+"/$3$4"), nil
+	case sshUrlRegex.MatchString(remoteUrl):
+		return sshUrlRegex.ReplaceAllString(remoteUrl, "${1}"+forkUsername+"/$3$4"), nil
+	case httpRegex.MatchString(remoteUrl):
+		return httpRegex.ReplaceAllString(remoteUrl, "${1}"+forkUsername+"/$3$4"), nil
+	default:
+		return "", fmt.Errorf("unsupported or invalid remote URL: %s", remoteUrl)
 	}
-
-	last := segments[len(segments)-1] // repo(.git)
-	u.Path = "/" + forkUsername + "/" + last
-
-	// Preserve trailing slash only if it existed and wasn't empty
-	// (remotes rarely care, but we'll avoid adding one)
-	return u.String(), nil
-}
-
-func isScpLikeSSH(s string) bool {
-	// Minimal heuristic: "<user>@<host>:<path>"
-	at := strings.IndexByte(s, '@')
-	colon := strings.IndexByte(s, ':')
-	return at > 0 && colon > at
-}
-
-func splitNonEmpty(s, sep string) []string {
-	raw := strings.Split(s, sep)
-	out := make([]string, 0, len(raw))
-	for _, p := range raw {
-		if p != "" {
-			out = append(out, p)
-		}
-	}
-	return out
 }
 
 func (self *RemotesController) addFork(baseRemote *models.Remote) error {
@@ -269,19 +227,13 @@ func (self *RemotesController) addFork(baseRemote *models.Remote) error {
 				Title:          self.c.Tr.NewRemoteName,
 				InitialContent: forkUsername,
 				HandleConfirm: func(remoteName string) error {
-					if forkUsername == "" {
-						return fmt.Errorf("Fork username cannot be empty")
-					}
 					if len(baseRemote.Urls) == 0 {
-						return fmt.Errorf("Base remote must have url")
+						return fmt.Errorf("base remote must have url")
 					}
-					url := baseRemote.Urls[0]
-					if url == "" {
-						return fmt.Errorf("Base remote url cannot be empty")
-					}
-					remoteUrl, err := replaceForkUsername(url, forkUsername)
+					baseUrl := baseRemote.Urls[0]
+					remoteUrl, err := replaceForkUsername(baseUrl, forkUsername)
 					if err != nil {
-						return fmt.Errorf("Failed to replace fork username in remote URL: `%w`, make sure it's a valid url", err)
+						return err
 					}
 
 					return self.addRemoteHelper(remoteName, remoteUrl)

pkg/gui/controllers/remotes_controller_test.go

@@ -12,23 +12,41 @@ func TestReplaceForkUsername_SSH_OK(t *testing.T) {
 		expected string
 	}{
 		{
-			name:     "github ssh basic",
+			name:     "github ssh scp-like basic",
 			in:       "[email protected]:old/repo.git",
 			forkUser: "new",
 			expected: "[email protected]:new/repo.git",
 		},
 		{
-			name:     "ssh no .git",
+			name:     "ssh scp-like no .git",
 			in:       "[email protected]:old/repo",
 			forkUser: "new",
 			expected: "[email protected]:new/repo",
 		},
 		{
-			name:     "gitlab subgroup ssh",
+			name:     "gitlab subgroup ssh scp-like",
 			in:       "[email protected]:group/sub/repo.git",
 			forkUser: "alice",
 			expected: "[email protected]:alice/repo.git",
 		},
+		{
+			name:     "ssh url style basic",
+			in:       "ssh://[email protected]/old/repo.git",
+			forkUser: "new",
+			expected: "ssh://[email protected]/new/repo.git",
+		},
+		{
+			name:     "ssh url style with port",
+			in:       "ssh://[email protected]:2222/old/repo.git",
+			forkUser: "bob",
+			expected: "ssh://[email protected]:2222/bob/repo.git",
+		},
+		{
+			name:     "ssh url style multi subgroup",
+			in:       "ssh://[email protected]/group/sub/repo.git",
+			forkUser: "alice",
+			expected: "ssh://[email protected]/alice/repo.git",
+		},
 	}
 
 	for _, c := range cases {
@@ -128,7 +146,7 @@ func TestReplaceForkUsername_Errors(t *testing.T) {
 		},
 		{
 			name:     "unsupported scheme",
-			in:       "ssh://git@github.com/old/repo.git", // explicit ssh:// not supported here
+			in:       "ftp://github.com/old/repo.git",
 			forkUser: "x",
 		},
 	}