refac: cleanup and comment code

main.go

@@ -14,17 +14,20 @@ import (
 )
 
 var (
-	app        = kingpin.New("clipman", "A clipboard manager for Wayland")
+	app      = kingpin.New("clipman", "A clipboard manager for Wayland")
-	histpath   = app.Flag("histpath", "Path of history file").Default("~/.local/share/clipman.json").String()
+	histpath = app.Flag("histpath", "Path of history file").Default("~/.local/share/clipman.json").String()
-	storer     = app.Command("store", "Run from `wl-paste --watch` to record clipboard events")
+
-	picker     = app.Command("pick", "Pick an item from clipboard history")
+	storer    = app.Command("store", "Record clipboard events (run as argument to `wl-paste --watch`)")
-	clearer    = app.Command("clear", "Remove an item from history")
+	maxDemon  = storer.Flag("max-items", "history size").Default("15").Int()
-	noPersist  = storer.Flag("no-persist", "Don't persist a copy buffer after a program exits").Short('P').Default("false").Bool()
+	noPersist = storer.Flag("no-persist", "Don't persist a copy buffer after a program exits").Short('P').Default("false").Bool()
-	maxDemon   = storer.Flag("max-items", "history size").Default("15").Int()
+
-	maxPicker  = picker.Flag("max-items", "scrollview length").Default("15").Int()
+	picker    = app.Command("pick", "Pick an item from clipboard history")
-	pickTool   = picker.Flag("selector", "Which selector to use: dmenu/rofi/-").Default("dmenu").String()
+	maxPicker = picker.Flag("max-items", "scrollview length").Default("15").Int()
-	clearTool  = clearer.Flag("selector", "Which selector to use: dmenu/rofi/-").Default("dmenu").String()
+	pickTool  = picker.Flag("selector", "Which selector to use: dmenu/rofi/-").Default("dmenu").String()
+
+	clearer    = app.Command("clear", "Remove item(s) from history")
 	maxClearer = clearer.Flag("max-items", "scrollview length").Default("15").Int()
+	clearTool  = clearer.Flag("selector", "Which selector to use: dmenu/rofi/-").Default("dmenu").String()
 	clearAll   = clearer.Flag("all", "Remove all items").Short('a').Default("false").Bool()
 )
 
@@ -32,9 +35,7 @@ func main() {
 	app.HelpFlag.Short('h')
 	switch kingpin.MustParse(app.Parse(os.Args[1:])) {
 	case "store":
-		persist := !*noPersist
+		histfile, history, err := getHistory(*histpath)
-
-		histfile, history, err := getHistory()
 		if err != nil {
 			log.Fatal(err)
 		}
@@ -50,9 +51,12 @@ func main() {
 		}
 		text := strings.Join(stdin, "\n")
 
-		store(text, history, histfile, *maxDemon, persist)
+		persist := !*noPersist
+		if err := store(text, history, histfile, *maxDemon, persist); err != nil {
+			log.Fatal(err)
+		}
 	case "pick":
-		_, history, err := getHistory()
+		_, history, err := getHistory(*histpath)
 		if err != nil {
 			log.Fatal(err)
 		}
@@ -65,11 +69,12 @@ func main() {
 		// serve selection to the OS
 		err = exec.Command("wl-copy", []string{"--", selection}...).Run()
 	case "clear":
-		histfile, history, err := getHistory()
+		histfile, history, err := getHistory(*histpath)
 		if err != nil {
 			log.Fatal(err)
 		}
 
+		// remove all history
 		if *clearAll {
 			if err := os.Remove(histfile); err != nil {
 				log.Fatal(err)
@@ -88,9 +93,9 @@ func main() {
 	}
 }
 
-func getHistory() (string, []string, error) {
+func getHistory(rawPath string) (string, []string, error) {
-	// set histfile
+	// set histfile; expand user home
-	histfile := *histpath
+	histfile := rawPath
 	if strings.HasPrefix(histfile, "~") {
 		home, err := os.UserHomeDir()
 		if err != nil {
@@ -99,16 +104,16 @@ func getHistory() (string, []string, error) {
 		histfile = strings.Replace(histfile, "~", home, 1)
 	}
 
-	// read existing history
+	// read history if it exists
 	var history []string
 	b, err := ioutil.ReadFile(histfile)
 	if err != nil {
 		if !os.IsNotExist(err) {
-			return "", nil, fmt.Errorf("Failure reading history file: %s", err)
+			return "", nil, fmt.Errorf("failure reading history file: %s", err)
 		}
 	} else {
 		if err := json.Unmarshal(b, &history); err != nil {
-			return "", nil, fmt.Errorf("Failure parsing history: %s", err)
+			return "", nil, fmt.Errorf("failure parsing history: %s", err)
 		}
 	}
 

selector.go

@@ -1,36 +1,22 @@
 package main
 
 import (
+	"errors"
 	"fmt"
-	"log"
 	"os"
 	"os/exec"
 	"strconv"
 	"strings"
 )
 
-func selector(history []string, max int, tool string) (string, error) {
+func selector(data []string, max int, tool string) (string, error) {
-	if len(history) == 0 {
+	if len(data) == 0 {
-		log.Fatal("No history available")
+		return "", errors.New("No history available")
 	}
 
-	// don't modify in-place!
+	// output to stdout and return
-	tmp := make([]string, len(history))
-	copy(tmp, history)
-
-	// reverse the history
-	for i, j := 0, len(history)-1; i < j; i, j = i+1, j-1 {
-		tmp[i], tmp[j] = tmp[j], tmp[i]
-	}
-
-	selected, err := dmenu(tmp, max, tool)
-
-	return selected, err
-}
-
-func dmenu(list []string, max int, tool string) (string, error) {
 	if tool == "-" {
-		escaped, _ := preprocessHistory(list, false)
+		escaped, _ := preprocessData(data, false)
 		os.Stdout.WriteString(strings.Join(escaped, "\n"))
 		return "", nil
 	}
@@ -53,50 +39,52 @@ func dmenu(list []string, max int, tool string) (string, error) {
 			"-lines",
 			strconv.Itoa(max)}
 	default:
-		return "", fmt.Errorf("Unsupported tool")
+		return "", fmt.Errorf("Unsupported tool: %s", tool)
 	}
 
-	escaped, guide := preprocessHistory(list, true)
+	processed, guide := preprocessData(data, true)
-	input := strings.NewReader(strings.Join(escaped, "\n"))
 
-	cmd := exec.Cmd{Path: bin, Args: args, Stdin: input}
+	cmd := exec.Cmd{Path: bin, Args: args, Stdin: strings.NewReader(strings.Join(processed, "\n"))}
-	selected, err := cmd.Output()
+	b, err := cmd.CombinedOutput()
 	if err != nil {
 		if err.Error() == "exit status 1" {
-			// dmenu exits with this error when no selection done
+			// dmenu/rofi exits with this error when no selection done
 			return "", nil
 		}
 		return "", err
 	}
-	trimmed := selected[:len(selected)-1] // drop newline added by dmenu
+	selected := string(b[:len(b)-1]) // drop newline added by dmenu/rofi
 
-	sel, ok := guide[string(trimmed)]
+	sel, ok := guide[selected]
 	if !ok {
-		return "", fmt.Errorf("couldn't recover original string; please report this bug along with a copy of your clipman.json")
+		return "", errors.New("couldn't recover original string")
 	}
 
 	return sel, nil
 }
 
-func preprocessHistory(list []string, cutting bool) ([]string, map[string]string) {
+// preprocessData:
-	// dmenu will break if items contain newlines, so we must pass them as literals.
+// - reverses the data
-	// however, when it sends them back, we need a way to restore them
+// - escapes special characters (like newlines) that would break external selectors;
+// - optionally it cuts items longer than 400 bytes (dmenu doesn't allow more than ~1200).
+// A guide is created to allow restoring the selected item.
+func preprocessData(data []string, cutting bool) ([]string, map[string]string) {
 	var escaped []string
 	guide := make(map[string]string)
 
-	for _, original := range list {
+	for i := len(data) - 1; i >= 0; i-- { // reverse slice
-		repr := fmt.Sprintf("%#v", original)
+		original := data[i]
-		max := len(repr) - 1 // drop right quote
 
-		// dmenu will split lines longer than 1200 something; we cut at 400 to spare memory
+		repr := fmt.Sprintf("%#v", original)
+		size := len(repr) - 1
 		if cutting {
-			maxChars := 400
+			const maxChars = 400
-			if max > maxChars {
+			if size > maxChars {
-				max = maxChars
+				size = maxChars
 			}
 		}
+		repr = repr[1:size] // drop left and right quotes
 
-		repr = repr[1:max] // drop left quote
 		guide[repr] = original
 		escaped = append(escaped, repr)
 	}

storer.go

@@ -2,24 +2,22 @@ package main
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"log"
 	"os/exec"
 )
 
-func store(text string, history []string, histfile string, max int, persist bool) {
+func store(text string, history []string, histfile string, max int, persist bool) error {
 	if text == "" {
-		return
+		return nil
 	}
 
 	l := len(history)
 	if l > 0 {
-		if history[l-1] == text {
+		// drop oldest items that exceed max list size
-			return
-		}
-
 		if l >= max {
-			// usually just one item, but more if we reduce our --max-items value
+			// usually just one item, but more if we suddenly reduce our --max-items
 			history = history[l-max+1:]
 		}
 
@@ -31,49 +29,38 @@ func store(text string, history []string, histfile string, max int, persist bool
 
 	// dump history to file so that other apps can query it
 	if err := write(history, histfile); err != nil {
-		log.Fatalf("Fatal error writing history: %s", err)
+		return fmt.Errorf("error writing history: %s", err)
 	}
 
+	// make the copy buffer available to all applications,
+	// even when the source has disappeared
 	if persist {
-		// make the copy buffer available to all applications,
-		// even when the source has disappeared
 		if err := exec.Command("wl-copy", []string{"--", text}...).Run(); err != nil {
-			log.Printf("Error running wl-copy: %s", err)
+			log.Printf("Error running wl-copy: %s", err) // don't abort, minor error
 		}
 	}
 
-	return
+	return nil
 }
 
-func filter(history []string, text string) []string {
+// filter removes all occurrences of text
-	var (
+func filter(slice []string, text string) []string {
-		found bool
+	var filtered []string
-		idx   int
+	for _, s := range slice {
-	)
+		if s != text {
-
+			filtered = append(filtered, s)
-	for i, el := range history {
-		if el == text {
-			found = true
-			idx = i
-			break
 		}
 	}
 
-	if found {
+	return filtered
-		// we know that idx can't be the last element, because
-		// we never get to call this function if that's the case
-		history = append(history[:idx], history[idx+1:]...)
-	}
-
-	return history
 }
 
+// write dumps history to json file
 func write(history []string, histfile string) error {
-	histlog, err := json.Marshal(history)
+	b, err := json.Marshal(history)
 	if err != nil {
 		return err
 	}
-	err = ioutil.WriteFile(histfile, histlog, 0644)
 
-	return err
+	return ioutil.WriteFile(histfile, b, 0644)
 }