refactor(forgejo-comment): restructure the way we handle env variables vs flags

apps/ci-os/packages/forgejo-comment/src/main.go

@@ -11,80 +11,112 @@ import (
 	"strconv"
 )
 
+func getEnv(key, defaultVal string) string {
+	if value, exists := os.LookupEnv(key); exists {
+		return value
+	}
+	return defaultVal
+}
+
 func main() {
-	// Define command-line flags
+	// Define command-line flags with environment variable defaults
 	var forgejoAPIURL string
+	flag.StringVar(&forgejoAPIURL, "forgejo-api-url", getEnv("FORGEJO_API_URL", "https://code.252.no/api/v1"), "Forgejo API URL (default: https://code.252.no/api/v1 or $FORGEJO_API_URL)")
+
 	var token string
+	flag.StringVar(&token, "token", os.Getenv("FORGEJO_TOKEN"), "Forgejo API token (required, or set $FORGEJO_TOKEN)")
+
 	var repoOwner string
+	flag.StringVar(&repoOwner, "repo-owner", os.Getenv("REPO_OWNER"), "Repository owner (required, or set $REPO_OWNER)")
+
 	var repoName string
+	flag.StringVar(&repoName, "repo-name", os.Getenv("REPO_NAME"), "Repository name (required, or set $REPO_NAME)")
+
 	var issueId int
-
+	issueIdStr := os.Getenv("ISSUE_ID")
-	flag.StringVar(&forgejoAPIURL, "forgejo-api-url", "https://code.252.no/api/v1", "Forgejo API URL")
+	if issueIdStr != "" {
-	flag.StringVar(&token, "token", "", "Forgejo API token")
-	flag.StringVar(&repoOwner, "repo-owner", "", "Repository owner")
-	flag.StringVar(&repoName, "repo-name", "", "Repository name")
-	flag.IntVar(&issueId, "issue-id", 0, "Issue id")
-	flag.Parse()
-
-	// Get the diff file from the positional arguments
-	args := flag.Args()
-	if len(args) < 1 {
-		fmt.Println("Usage: forgejo-comment <diff-file>")
-		os.Exit(1)
-	}
-	diffFile := args[0]
-
-	// Read environment variables if flags are not set
-	if forgejoAPIURL == "" {
-		forgejoAPIURL = os.Getenv("FORGEJO_API_URL")
-		if forgejoAPIURL == "" {
-			fmt.Println("Error: FORGEJO_API_URL is not set")
-			os.Exit(1)
-		}
-	}
-
-	if token == "" {
-		token = os.Getenv("FORGEJO_TOKEN")
-		if token == "" {
-			fmt.Println("Error: FORGEJO_TOKEN is not set")
-			os.Exit(1)
-		}
-	}
-
-	if repoOwner == "" {
-		repoOwner = os.Getenv("REPO_OWNER")
-		if repoOwner == "" {
-			fmt.Println("Error: REPO_OWNER is not set")
-			os.Exit(1)
-		}
-	}
-
-	if repoName == "" {
-		repoName = os.Getenv("REPO_NAME")
-		if repoName == "" {
-			fmt.Println("Error: REPO_NAME is not set")
-			os.Exit(1)
-		}
-	}
-
-	if issueId == 0 {
-		issueIdEnv := os.Getenv("ISSUE_ID")
-		if issueIdEnv == "" {
-			fmt.Println("Error: ISSUE_ID is not set")
-			os.Exit(1)
-		}
 		var err error
-		issueId, err = strconv.Atoi(issueIdEnv)
+		issueId, err = strconv.Atoi(issueIdStr)
 		if err != nil {
 			fmt.Println("Error: Invalid ISSUE_ID")
 			os.Exit(1)
 		}
 	}
+	flag.IntVar(&issueId, "issue-id", issueId, "Issue ID to comment on (cannot be used with --pull-id, or set $ISSUE_ID)")
+
+	var pullId int
+	pullIdStr := os.Getenv("PULL_ID")
+	if pullIdStr != "" {
+		var err error
+		pullId, err = strconv.Atoi(pullIdStr)
+		if err != nil {
+			fmt.Println("Error: Invalid PULL_ID")
+			os.Exit(1)
+		}
+	}
+	flag.IntVar(&pullId, "pull-id", pullId, "Pull Request ID to comment on (cannot be used with --issue-id, or set $PULL_ID)")
+
+	// Override the default usage message to be more helpful
+	flag.Usage = func() {
+		fmt.Fprintf(flag.CommandLine.Output(), "Usage: %s [options] <diff-file>\n", os.Args[0])
+		fmt.Fprintln(flag.CommandLine.Output(), "\nOptions:")
+		flag.PrintDefaults()
+		fmt.Fprintln(flag.CommandLine.Output(), "\nExample:")
+		fmt.Fprintf(flag.CommandLine.Output(), "  %s --pull-id=123 --token=YOUR_TOKEN --repo-owner=owner --repo-name=repo diff.txt\n", os.Args[0])
+	}
+
+	flag.Parse()
+
+	// Get the diff file from the positional arguments
+	args := flag.Args()
+	if len(args) < 1 {
+		fmt.Println("Error: Missing diff file.")
+		flag.Usage()
+		os.Exit(1)
+	}
+	diffFile := args[0]
+
+	// Check for required parameters
+	if token == "" {
+		fmt.Println("Error: Forgejo API token is not set. Use the --token flag or set FORGEJO_TOKEN environment variable.")
+		flag.Usage()
+		os.Exit(1)
+	}
+
+	if repoOwner == "" {
+		fmt.Println("Error: Repository owner is not set. Use the --repo-owner flag or set REPO_OWNER environment variable.")
+		flag.Usage()
+		os.Exit(1)
+	}
+
+	if repoName == "" {
+		fmt.Println("Error: Repository name is not set. Use the --repo-name flag or set REPO_NAME environment variable.")
+		flag.Usage()
+		os.Exit(1)
+	}
+
+	// Ensure that only one of issueId or pullId is set
+	if (issueId == 0 && pullId == 0) || (issueId != 0 && pullId != 0) {
+		fmt.Println("Error: You must specify either --issue-id or --pull-id, but not both.")
+		flag.Usage()
+		os.Exit(1)
+	}
+
+	// Determine which ID to use
+	var itemId int
+	var itemType string
+	if issueId != 0 {
+		itemId = issueId
+		itemType = "issue"
+	} else {
+		itemId = pullId
+		itemType = "pull"
+	}
 
 	// Read the diff file
 	diffContentBytes, err := os.ReadFile(diffFile)
 	if err != nil {
-		fmt.Printf("Error reading diff file: %v\n", err)
+		fmt.Printf("Error reading diff file '%s': %v\n", diffFile, err)
 		os.Exit(1)
 	}
 
@@ -103,7 +135,7 @@ func main() {
 	}
 
 	// Construct the API URL
-	apiURL := fmt.Sprintf("%s/repos/%s/%s/issues/%d/comments", forgejoAPIURL, repoOwner, repoName, issueId)
+	apiURL := fmt.Sprintf("%s/repos/%s/%s/issues/%d/comments", forgejoAPIURL, repoOwner, repoName, itemId)
 
 	// Create the HTTP request
 	req, err := http.NewRequest("POST", apiURL, bytes.NewBuffer(payloadBytes))
@@ -137,6 +169,6 @@ func main() {
 		fmt.Printf("Response body: %s\n", string(respBody))
 		os.Exit(1)
 	} else {
-		fmt.Printf("Posted diff to Forgejo issue #%d\n", issueId)
+		fmt.Printf("Posted diff to Forgejo %s #%d\n", itemType, itemId)
 	}
 }