refactor(clipper/options): Enhance version handling and remove pointe…

…rs for improved usability (#37)

* feat(version): implement dynamic versioning with build metadata

This commit refactors the version management in Clipper:

- Introduces package-level variables `Version` and `BuildMetadata` in `options.go` with "dev" and "git/source" defaults for development builds.
- Adds the `GetVersion()` function to format the version string for display, incorporating build metadata when available.
- Updates the Makefile to inject the correct version and build metadata using `-ldflags` during compilation.
- Modifies `main.go` to use `GetVersion()` for consistent version display.

Now, Clipper displays "dev git/source" when run from source and a tagged version with OS/ARCH information (e.g., "v1.5.0 linux/amd64") when run from a built binary. This improves clarity for users and maintainers alike.

Issue #36

* refactor(options): remove pointers from Config struct

This commit refactors the `options.Config` struct by changing the `DirectText` and `ShowVersion` fields from pointers (`*string` and `*bool`) to their corresponding value types (`string` and `bool`).

The use of pointers was initially introduced to accommodate the return type of `flag.String` and `flag.Bool` in the `ParseFlags` function. However, it resulted in unnecessary complexity and the potential for nil pointer dereferences.

By removing the pointers, the code becomes simpler, more readable, and less error-prone. It also eliminates the need to constantly dereference pointers when accessing the values of these fields.

Issue #30: Refactor options.Config to Remove Pointer for DirectText (Improving Usability and Testing).

cli/clipper/clipper.go

@@ -21,13 +21,9 @@ func (c DefaultClipboardWriter) Write(content string) error {
 	return clipboard.WriteAll(content)
 }
 
-// Run executes the clipper tool logic based on the provided configuration.
+// Run executes the core logic of the Clipper tool.
 func Run(config *options.Config, writer ClipboardWriter) (string, error) {
-	if *config.ShowVersion {
-		return options.Version, nil
-	}
-
-	readers := reader.GetReaders(config.Args)
+	readers := reader.GetReaders(config.Args...)
 
 	// Aggregate the content from the provided sources.
 	content, err := reader.ParseContent(config.DirectText, readers...)
@@ -40,5 +36,5 @@ func Run(config *options.Config, writer ClipboardWriter) (string, error) {
 		return "", fmt.Errorf("copying content to clipboard: %w", err)
 	}
 
-	return "updated clipboard successfully. Ready to paste!", nil
+	return "Updated clipboard successfully. Ready to paste!", nil
 }

cli/main.go

@@ -9,17 +9,20 @@ import (
 )
 
 func main() {
-	config := options.ParseFlags()
-	writer := clipper.DefaultClipboardWriter{}
+	config := options.ParseFlags() // Parse command-line flags
 
-	msg, err := clipper.Run(config, writer)
+	if config.ShowVersion {
+		fmt.Printf("Clipper %s\n", options.GetVersion())
+		os.Exit(0)
+	}
+
+	writer := clipper.DefaultClipboardWriter{} // Create the default clipboard writer
+
+	msg, err := clipper.Run(config, writer) // Run the main Clipper logic
 	if err != nil {
 		fmt.Printf("Error %s\n", err)
-		os.Exit(1)
+		os.Exit(1) // Exit with an error code
 	}
 
-	if msg != "" {
-		fmt.Printf("Clipper %s\n", msg)
-		os.Exit(0)
-	}
+	fmt.Println(msg)
 }

cli/options/options.go

@@ -3,15 +3,29 @@ package options
 import (
 	"flag"
 	"fmt"
+	"strings"
 )
 
 type Config struct {
-	DirectText  *string
-	ShowVersion *bool
+	DirectText  string
+	ShowVersion bool
 	Args        []string
 }
 
-const Version = "1.5.0"
+// Package-level variables for version information (set at build time or default)
+var (
+	Version       = "dev"        // Default for development builds
+	BuildMetadata = "git/source" // Default for development builds
+)
+
+// GetVersion formats the version string for display
+func GetVersion() string {
+	if BuildMetadata != "" {
+		return strings.TrimSpace(Version) + " " + strings.TrimSpace(BuildMetadata)
+	} else {
+		return strings.TrimSpace(Version)
+	}
+}
 
 // ParseFlags parses the command-line flags and arguments.
 func ParseFlags() *Config {
@@ -31,8 +45,8 @@ func ParseFlags() *Config {
 	flag.Parse()
 
 	return &Config{
-		DirectText:  directText,
-		ShowVersion: showVersion,
+		DirectText:  *directText,
+		ShowVersion: *showVersion,
 		Args:        flag.Args(),
 	}
 }

cli/reader/reader.go

@@ -43,7 +43,7 @@ func (s StdinContentReader) Read() (string, error) {
 
 // ReadContentConcurrently reads content from multiple readers concurrently and returns the results.
 // This function utilizes goroutines to perform concurrent reads, improving performance for multiple files.
-func ReadContentConcurrently(readers []ContentReader) ([]string, error) {
+func ReadContentConcurrently(readers ...ContentReader) ([]string, error) {
 	var wg sync.WaitGroup
 	var mu sync.Mutex
 	errChan := make(chan error, len(readers)) // Channel to capture errors
@@ -79,7 +79,7 @@ func ReadContentConcurrently(readers []ContentReader) ([]string, error) {
 
 // AggregateContent aggregates the content from the provided results and returns it as a single string.
 // It combines the content of all readers into a single string with newline separators.
-func AggregateContent(results []string) string {
+func AggregateContent(results ...string) string {
 	var sb strings.Builder
 	for _, content := range results {
 		if content != "" { // Ensure non-empty content is aggregated
@@ -91,26 +91,26 @@ func AggregateContent(results []string) string {
 
 // ParseContent aggregates content from the provided readers, or returns the direct text if provided.
 // This function first checks for direct text input, then reads from the provided readers concurrently.
-func ParseContent(directText *string, readers ...ContentReader) (string, error) {
-	if directText != nil && *directText != "" {
-		return *directText, nil // Return direct text if provided
+func ParseContent(directText string, readers ...ContentReader) (string, error) {
+	if directText != "" {
+		return directText, nil // Return direct text if provided
 	}
 
 	if len(readers) == 0 {
 		return "", fmt.Errorf("no content readers provided")
 	}
 
-	results, err := ReadContentConcurrently(readers) // Read content concurrently
+	results, err := ReadContentConcurrently(readers...) // Read content concurrently
 	if err != nil {
 		return "", err
 	}
 
-	return AggregateContent(results), nil // Aggregate and return the content
+	return AggregateContent(results...), nil // Aggregate and return the content
 }
 
 // GetReaders constructs the appropriate ContentReaders based on the provided file paths or lack thereof.
 // If no targets are provided, it defaults to using StdinContentReader.
-func GetReaders(targets []string) []ContentReader {
+func GetReaders(targets ...string) []ContentReader {
 	if len(targets) == 0 {
 		// If no file paths are provided, use StdinContentReader to read from stdin.
 		return []ContentReader{StdinContentReader{}}

makefile

@@ -1,46 +1,49 @@
+# Makefile for building Clipper
+
 # Define the output directory for the binaries
 OUT_DIR := bin
 
-# Retrieve the version from git tags
-VERSION := $(shell git describe --tags --always)
+# Define version info (set dynamically during build)
+VERSION ?= $(shell git describe --tags --always)  # Get latest tag or commit hash as version
 
-# Define the names for the binaries with version
-WINDOWS_BIN := clipper_windows_amd64_$(VERSION).exe
-LINUX_BIN := clipper_linux_amd64_$(VERSION)
-LINUX_ARM_BIN := clipper_linux_arm_$(VERSION)
-LINUX_ARM64_BIN := clipper_linux_arm64_$(VERSION)
-DARWIN_BIN := clipper_darwin_amd64_$(VERSION)
-DARWIN_ARM64_BIN := clipper_darwin_arm64_$(VERSION)
+REPO_URL := github.com/supitsdu/clipper
 
 # Define the build targets for each platform
 .PHONY: all windows linux linux_arm linux_arm64 darwin darwin_arm64 clean checksums test help
 
 # Default target: build binaries for all platforms
 all: windows linux linux_arm linux_arm64 darwin darwin_arm64
 
-# Build binary for Windows
+# Generic build function with simplified binary name and embedded metadata
+define build
+GOOS=$(1) GOARCH=$(2) go build \
+	-ldflags="-X '$(REPO_URL)/cli/options.Version=$(VERSION)' -X '$(REPO_URL)/cli/options.BuildMetadata=$(1)/$(2)'" \
+	-o $(OUT_DIR)/clipper_$(1)_$(2)_$(VERSION) ./cli
+endef
+
+# Build binaries for each platform, calling the generic build function with appropriate arguments
 windows: $(OUT_DIR)
-	GOOS=windows GOARCH=amd64 go build -ldflags="-X main.version=$(VERSION)" -o $(OUT_DIR)/$(WINDOWS_BIN) ./cli
+	$(call build,windows,amd64,windows)
 
 # Build binary for Linux (amd64)
 linux: $(OUT_DIR)
-	GOOS=linux GOARCH=amd64 go build -ldflags="-X main.version=$(VERSION)" -o $(OUT_DIR)/$(LINUX_BIN) ./cli
+	$(call build,linux,amd64,linux)
 
 # Build binary for Linux (arm)
 linux_arm: $(OUT_DIR)
-	GOOS=linux GOARCH=arm go build -ldflags="-X main.version=$(VERSION)" -o $(OUT_DIR)/$(LINUX_ARM_BIN) ./cli
+	$(call build,linux,arm,linux)
 
 # Build binary for Linux (arm64)
 linux_arm64: $(OUT_DIR)
-	GOOS=linux GOARCH=arm64 go build -ldflags="-X main.version=$(VERSION)" -o $(OUT_DIR)/$(LINUX_ARM64_BIN) ./cli
+	$(call build,linux,arm64,linux)
 
 # Build binary for macOS (amd64)
 darwin: $(OUT_DIR)
-	GOOS=darwin GOARCH=amd64 go build -ldflags="-X main.version=$(VERSION)" -o $(OUT_DIR)/$(DARWIN_BIN) ./cli
+	$(call build,darwin,amd64,darwin)
 
 # Build binary for macOS (arm64)
 darwin_arm64: $(OUT_DIR)
-	GOOS=darwin GOARCH=arm64 go build -ldflags="-X main.version=$(VERSION)" -o $(OUT_DIR)/$(DARWIN_ARM64_BIN) ./cli
+	$(call build,darwin,arm64,darwin)
 
 # Generate SHA256 checksums for each binary
 checksums: $(OUT_DIR)

tests/reader/reader_test.go

@@ -85,7 +85,7 @@ func TestParseContent(t *testing.T) {
 		reader2 := reader.FileContentReader{FilePath: file2.Name()}
 
 		// Execute the function under test
-		actual, err := reader.ParseContent(nil, reader1, reader2)
+		actual, err := reader.ParseContent("", reader1, reader2)
 		if err != nil {
 			t.Fatalf("Expected no error, got %v", err)
 		}
@@ -111,7 +111,7 @@ func TestParseContent(t *testing.T) {
 		testReader := reader.FileContentReader{FilePath: emptyFile.Name()}
 
 		// Execute the function under test with the empty file
-		actual, err := reader.ParseContent(nil, testReader)
+		actual, err := reader.ParseContent("", testReader)
 		if err != nil {
 			t.Fatalf("Expected no error, got: %v", err)
 		}
@@ -127,7 +127,7 @@ func TestParseContent(t *testing.T) {
 
 	t.Run("InvalidNilInput", func(t *testing.T) {
 		// Execute the function under test with no input arguments
-		_, err := reader.ParseContent(nil)
+		_, err := reader.ParseContent("")
 		if err == nil {
 			t.Fatalf("Expected error, got %v", err)
 		}
@@ -141,7 +141,7 @@ func TestParseContent(t *testing.T) {
 		testReader := reader.FileContentReader{FilePath: invalidPath}
 
 		// Execute the function under test with the invalid file path
-		_, err := reader.ParseContent(nil, testReader)
+		_, err := reader.ParseContent("", testReader)
 		if err == nil {
 			t.Fatalf("Expected an error, but got none")
 		}
@@ -162,7 +162,7 @@ func TestParseContent(t *testing.T) {
 		testReader := reader.FileContentReader{FilePath: largeFile.Name()}
 
 		// Execute the function under test with the large file
-		actual, err := reader.ParseContent(nil, testReader)
+		actual, err := reader.ParseContent("", testReader)
 		if err != nil {
 			t.Fatalf("Expected no error, got %v", err)
 		}
@@ -176,7 +176,7 @@ func TestParseContent(t *testing.T) {
 	t.Run("DirectText", func(t *testing.T) {
 		// Test with direct text
 		directText := tests.SampleText_32
-		content, err := reader.ParseContent(&directText)
+		content, err := reader.ParseContent(directText)
 		if err != nil {
 			t.Errorf("Error parsing content: %v", err)
 		}
@@ -194,7 +194,7 @@ func TestParseContent(t *testing.T) {
 
 		testReader := reader.FileContentReader{FilePath: tmpFile.Name()}
 
-		content, err := reader.ParseContent(nil, testReader)
+		content, err := reader.ParseContent("", testReader)
 		if err != nil {
 			t.Errorf("Error parsing content: %v", err)
 		}