use record.V1 more, housekeeping

spelling
error handling improvements
logging
ci

.github/workflows/release.yaml

@@ -18,7 +18,7 @@ jobs:
           fetch-depth: 0
 
       - name: Get Go version
-        run: echo "GO_VERSION=$(grep '^go ' go.mod | cut -d ' ' -f 2)" >> $GITHUB_ENV
+        run: echo "GO_VERSION=$(grep '^go ' go.mod | cut -d ' ' -f 2)" >> $GITHUB_ENV && cat $GITHUB_ENV
 
       - name: Set up Go
         uses: actions/setup-go@v3

cmd/install-utils/main.go

@@ -44,7 +44,7 @@ func main() {
 	case "rollback":
 		rollback()
 	case "migrate-config":
-		migrateConfig()
+		migrateConfig(out)
 	case "migrate-history":
 		migrateHistory(out)
 	case "setup-device":

cmd/install-utils/migrate.go

@@ -10,22 +10,19 @@ import (
 	"github.com/curusarn/resh/internal/futil"
 	"github.com/curusarn/resh/internal/output"
 	"github.com/curusarn/resh/internal/recio"
-	"github.com/curusarn/resh/record"
 )
 
-func migrateConfig() {
+func migrateConfig(out *output.Output) {
 	err := cfg.Touch()
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "ERROR: Failed to touch config file: %v\n", err)
-		os.Exit(1)
+		out.Fatal("ERROR: Failed to touch config file", err)
 	}
 	changes, err := cfg.Migrate()
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "ERROR: Failed to update config file: %v\n", err)
-		os.Exit(1)
+		out.Fatal("ERROR: Failed to update config file", err)
 	}
 	if changes {
-		fmt.Printf("RESH config file format has changed since last update - your config was updated to reflect the changes.\n")
+		out.Info("RESH config file format has changed since last update - your config was updated to reflect the changes.")
 	}
 }
 
@@ -34,6 +31,8 @@ func migrateHistory(out *output.Output) {
 	migrateHistoryFormat(out)
 }
 
+// find first existing history and use it
+// don't bother with merging of history in multiple locations - it could get messy and it shouldn't be necessary
 func migrateHistoryLocation(out *output.Output) {
 	dataDir, err := datadir.MakePath()
 	if err != nil {
@@ -92,9 +91,9 @@ func migrateHistoryFormat(out *output.Output) {
 	}
 	if !exists {
 		out.ErrorWOErr("There is no history file - this is normal if you are installing RESH for the first time on this device")
-		err = futil.CreateFile(historyPath)
+		err = futil.TouchFile(historyPath)
 		if err != nil {
-			out.Fatal("ERROR: Failed to create history file", err)
+			out.Fatal("ERROR: Failed to touch history file", err)
 		}
 		os.Exit(0)
 	}
@@ -110,12 +109,7 @@ func migrateHistoryFormat(out *output.Output) {
 	if err != nil {
 		out.Fatal("ERROR: Could not load history file", err)
 	}
-	// TODO: get rid of this conversion
-	var recsV1 []record.V1
-	for _, rec := range recs {
-		recsV1 = append(recsV1, rec.Rec)
-	}
-	err = rio.OverwriteFile(historyPath, recsV1)
+	err = rio.OverwriteFile(historyPath, recs)
 	if err != nil {
 		out.Error("ERROR: Could not update format of history file", err)
 

internal/cfg/cfg.go

@@ -46,8 +46,8 @@ type Config struct {
 	Debug bool
 	// SessionWatchPeriodSeconds is how often should daemon check if terminal
 	// sessions are still alive
-	// There is not much need to adjust the value both memory overhead of watched sessions
-	// and the CPU overhead of chacking them are relatively low
+	// There is not much need to adjust the value because both memory overhead of watched sessions
+	// and the CPU overhead of checking them are quite low
 	SessionWatchPeriodSeconds uint
 	// ReshHistoryMinSize is how large resh history needs to be for
 	// daemon to ignore standard shell history files
@@ -72,11 +72,11 @@ const headerComment = `##
 ## RESH config (v1) ##
 ######################
 ## Here you can find info about RESH configuration options.
-## You can uncomment the options and custimize them.
+## You can uncomment the options and customize them.
 
 ## Required.
 ## The config format can change in future versions.
-## ConfigVersion helps us seemlessly upgrade to the new formats.
+## ConfigVersion helps us seamlessly upgrade to the new formats.
 # ConfigVersion = "v1"
 
 ## Port used by RESH daemon and rest of the components to communicate.
@@ -88,7 +88,7 @@ const headerComment = `##
 ## Options: "debug", "info", "warn", "error", "fatal"
 # LogLevel = "info"
 
-## When BindControlR is "true" RESH search app is bound to CTRL+R on terminal startuA
+## When BindControlR is "true" RESH search app is bound to CTRL+R on terminal startup
 # BindControlR = true
 
 ## When Debug is "true" the RESH search app runs in debug mode.
@@ -101,7 +101,7 @@ const headerComment = `##
 # SessionWatchPeriodSeconds = 600
 
 ## When RESH is first installed there is no RESH history so there is nothing to search.
-## As a temporary woraround, RESH daemon parses bash/zsh shell history and searches it.
+## As a temporary workaround, RESH daemon parses bash/zsh shell history and searches it.
 ## Once RESH history is big enough RESH stops using bash/zsh history.
 ## ReshHistoryMinSize controls how big RESH history needs to be before this happens.
 ## You can increase this this to e.g. 10000 to get RESH to use bash/zsh history longer.

internal/cfg/migrate.go

@@ -5,21 +5,18 @@ import (
 	"os"
 
 	"github.com/BurntSushi/toml"
+	"github.com/curusarn/resh/internal/futil"
 )
 
 // Touch config file
 func Touch() error {
-	path, err := getConfigPath()
+	fpath, err := getConfigPath()
 	if err != nil {
 		return fmt.Errorf("could not get config file path: %w", err)
 	}
-	file, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666)
+	err = futil.TouchFile(fpath)
 	if err != nil {
-		return fmt.Errorf("could not open/create config file: %w", err)
-	}
-	err = file.Close()
-	if err != nil {
-		return fmt.Errorf("could not close config file: %w", err)
+		return fmt.Errorf("could not touch config file: %w", err)
 	}
 	return nil
 }
@@ -27,11 +24,11 @@ func Touch() error {
 // Migrate old config versions to current config version
 // returns true if any changes were made to the config
 func Migrate() (bool, error) {
-	path, err := getConfigPath()
+	fpath, err := getConfigPath()
 	if err != nil {
 		return false, fmt.Errorf("could not get config file path: %w", err)
 	}
-	configF, err := readConfig(path)
+	configF, err := readConfig(fpath)
 	if err != nil {
 		return false, fmt.Errorf("could not read config: %w", err)
 	}
@@ -50,7 +47,7 @@ func Migrate() (bool, error) {
 	if *configF.ConfigVersion != current {
 		return false, fmt.Errorf("unrecognized config version: '%s'", *configF.ConfigVersion)
 	}
-	err = writeConfig(configF, path)
+	err = writeConfig(configF, fpath)
 	if err != nil {
 		return true, fmt.Errorf("could not write migrated config: %w", err)
 	}

internal/device/device.go

@@ -1,3 +1,4 @@
+// device implements helpers that get/set device config files
 package device
 
 import (

internal/futil/futil.go

@@ -1,3 +1,4 @@
+// futil implements common file-related utilities
 package futil
 
 import (
@@ -41,14 +42,14 @@ func FileExists(fpath string) (bool, error) {
 	return false, fmt.Errorf("could not stat file: %w", err)
 }
 
-func CreateFile(fpath string) error {
-	ff, err := os.Create(fpath)
+func TouchFile(fpath string) error {
+	file, err := os.OpenFile(fpath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666)
 	if err != nil {
-		return err
+		return fmt.Errorf("could not open/create file: %w", err)
 	}
-	err = ff.Close()
+	err = file.Close()
 	if err != nil {
-		return err
+		return fmt.Errorf("could not close file: %w", err)
 	}
 	return nil
 }

internal/histcli/histcli.go

@@ -2,22 +2,26 @@ package histcli
 
 import (
 	"github.com/curusarn/resh/internal/recordint"
+	"github.com/curusarn/resh/record"
+	"go.uber.org/zap"
 )
 
 // Histcli is a dump of history preprocessed for resh cli purposes
 type Histcli struct {
 	// list of records
 	List []recordint.SearchApp
+
+	sugar *zap.SugaredLogger
 }
 
 // New Histcli
-func New() Histcli {
+func New(sugar *zap.SugaredLogger) Histcli {
 	return Histcli{}
 }
 
 // AddRecord to the histcli
-func (h *Histcli) AddRecord(rec *recordint.Indexed) {
-	cli := recordint.NewSearchApp(rec)
+func (h *Histcli) AddRecord(rec *record.V1) {
+	cli := recordint.NewSearchApp(h.sugar, rec)
 
 	h.List = append(h.List, cli)
 }

internal/histfile/histfile.go

@@ -35,7 +35,7 @@ type Histfile struct {
 	rio *recio.RecIO
 }
 
-// New creates new histfile and runs its gorutines
+// New creates new histfile and runs its goroutines
 func New(sugar *zap.SugaredLogger, input chan recordint.Collect, sessionsToDrop chan string,
 	reshHistoryPath string, bashHistoryPath string, zshHistoryPath string,
 	maxInitHistSize int, minInitHistSizeKB int,
@@ -48,7 +48,7 @@ func New(sugar *zap.SugaredLogger, input chan recordint.Collect, sessionsToDrop
 		historyPath:  reshHistoryPath,
 		bashCmdLines: histlist.New(sugar),
 		zshCmdLines:  histlist.New(sugar),
-		cliRecords:   histcli.New(),
+		cliRecords:   histcli.New(sugar),
 		rio:          &rio,
 	}
 	go hf.loadHistory(bashHistoryPath, zshHistoryPath, maxInitHistSize, minInitHistSizeKB)
@@ -58,7 +58,7 @@ func New(sugar *zap.SugaredLogger, input chan recordint.Collect, sessionsToDrop
 }
 
 // load records from resh history, reverse, enrich and save
-func (h *Histfile) loadCliRecords(recs []recordint.Indexed) {
+func (h *Histfile) loadCliRecords(recs []record.V1) {
 	for _, cmdline := range h.bashCmdLines.List {
 		h.cliRecords.AddCmdLine(cmdline)
 	}
@@ -218,17 +218,15 @@ func (h *Histfile) mergeAndWriteRecord(sugar *zap.SugaredLogger, part1 recordint
 		return
 	}
 
+	recV1 := record.V1(rec)
 	func() {
 		cmdLine := rec.CmdLine
 		h.bashCmdLines.AddCmdLine(cmdLine)
 		h.zshCmdLines.AddCmdLine(cmdLine)
-		h.cliRecords.AddRecord(&recordint.Indexed{
-			// TODO: is this what we want?
-			Rec: rec,
-		})
+		h.cliRecords.AddRecord(&recV1)
 	}()
 
-	h.rio.AppendToFile(h.historyPath, []record.V1{rec})
+	h.rio.AppendToFile(h.historyPath, []record.V1{recV1})
 }
 
 // TODO: use errors in RecIO
@@ -261,13 +259,13 @@ func (h *Histfile) DumpCliRecords() histcli.Histcli {
 	return h.cliRecords
 }
 
-func loadCmdLines(sugar *zap.SugaredLogger, recs []recordint.Indexed) histlist.Histlist {
+func loadCmdLines(sugar *zap.SugaredLogger, recs []record.V1) histlist.Histlist {
 	hl := histlist.New(sugar)
 	// go from bottom and deduplicate
 	var cmdLines []string
 	cmdLinesSet := map[string]bool{}
 	for i := len(recs) - 1; i >= 0; i-- {
-		cmdLine := recs[i].Rec.CmdLine
+		cmdLine := recs[i].CmdLine
 		if cmdLinesSet[cmdLine] {
 			continue
 		}

internal/histio/file.go

@@ -6,7 +6,7 @@ import (
 	"sync"
 
 	"github.com/curusarn/resh/internal/recio"
-	"github.com/curusarn/resh/internal/recordint"
+	"github.com/curusarn/resh/record"
 	"go.uber.org/zap"
 )
 
@@ -16,7 +16,7 @@ type histfile struct {
 	path string
 
 	mu       sync.RWMutex
-	data     []recordint.Indexed
+	data     []record.V1
 	fileinfo os.FileInfo
 }
 

internal/histio/histio.go

@@ -3,7 +3,6 @@ package histio
 import (
 	"path"
 
-	"github.com/curusarn/resh/internal/recordint"
 	"github.com/curusarn/resh/record"
 	"go.uber.org/zap"
 )
@@ -18,7 +17,7 @@ type Histio struct {
 	// moreHistories map[string]*histfile
 
 	recordsToAppend chan record.V1
-	recordsToFlag   chan recordint.Flag
+	// recordsToFlag   chan recordint.Flag
 }
 
 func New(sugar *zap.SugaredLogger, dataDir, deviceID string) *Histio {

internal/recio/read.go

@@ -10,86 +10,88 @@ import (
 
 	"github.com/curusarn/resh/internal/futil"
 	"github.com/curusarn/resh/internal/recconv"
-	"github.com/curusarn/resh/internal/recordint"
 	"github.com/curusarn/resh/record"
 	"go.uber.org/zap"
 )
 
-func (r *RecIO) ReadAndFixFile(fpath string, maxErrors int) ([]recordint.Indexed, error) {
-	recs, numErrs, err := r.ReadFile(fpath)
+func (r *RecIO) ReadAndFixFile(fpath string, maxErrors int) ([]record.V1, error) {
+	recs, err, decodeErrs := r.ReadFile(fpath)
 	if err != nil {
 		return nil, err
 	}
+	numErrs := len(decodeErrs)
 	if numErrs > maxErrors {
+		r.sugar.Errorw("Encountered too many decoding errors",
+			"corruptedRecords", numErrs,
+		)
 		return nil, fmt.Errorf("encountered too many decoding errors")
 	}
 	if numErrs == 0 {
 		return recs, nil
 	}
 
-	// TODO: check there error messages
+	// TODO: check the error messages
 	r.sugar.Warnw("Some history records could not be decoded - fixing resh history file by dropping them",
 		"corruptedRecords", numErrs,
 	)
 	fpathBak := fpath + ".bak"
 	r.sugar.Infow("Backing up current corrupted history file",
-		"backupFilename", fpathBak,
+		"historyFileBackup", fpathBak,
 	)
-	// TODO: maybe use upstream copy function
 	err = futil.CopyFile(fpath, fpathBak)
 	if err != nil {
 		r.sugar.Errorw("Failed to create a backup history file - aborting fixing history file",
-			"backupFilename", fpathBak,
+			"historyFileBackup", fpathBak,
 			zap.Error(err),
 		)
 		return recs, nil
 	}
 	r.sugar.Info("Writing resh history file without errors ...")
-	var recsV1 []record.V1
-	for _, rec := range recs {
-		recsV1 = append(recsV1, rec.Rec)
-	}
-	err = r.OverwriteFile(fpath, recsV1)
+	err = r.OverwriteFile(fpath, recs)
 	if err != nil {
-		r.sugar.Errorw("Failed write fixed history file - aborting fixing history file",
-			"filename", fpath,
+		r.sugar.Errorw("Failed write fixed history file - restoring history file from backup",
+			"historyFile", fpath,
 			zap.Error(err),
 		)
+
+		err = futil.CopyFile(fpathBak, fpath)
+		if err != nil {
+			r.sugar.Errorw("Failed restore history file from backup",
+				"historyFile", fpath,
+				"HistoryFileBackup", fpathBak,
+				zap.Error(err),
+			)
+		}
 	}
 	return recs, nil
 }
 
-func (r *RecIO) ReadFile(fpath string) ([]recordint.Indexed, int, error) {
-	var recs []recordint.Indexed
+func (r *RecIO) ReadFile(fpath string) ([]record.V1, error, []error) {
+	var recs []record.V1
 	file, err := os.Open(fpath)
 	if err != nil {
-		return nil, 0, fmt.Errorf("failed to open history file: %w", err)
+		return nil, fmt.Errorf("failed to open history file: %w", err), nil
 	}
 	defer file.Close()
 
 	reader := bufio.NewReader(file)
-	numErrs := 0
-	var idx int
+	decodeErrs := []error{}
 	for {
 		var line string
 		line, err = reader.ReadString('\n')
 		if err != nil {
 			break
 		}
-		idx++
 		rec, err := r.decodeLine(line)
 		if err != nil {
-			numErrs++
+			r.sugar.Errorw("Error while decoding line", zap.Error(err),
+				"filePath", fpath,
+				"line", line,
+			)
+			decodeErrs = append(decodeErrs, err)
 			continue
 		}
-		recidx := recordint.Indexed{
-			Rec: *rec,
-			// TODO: Is line index actually enough?
-			// 	 Don't we want to count bytes because we will scan by number of bytes?
-			// 	 hint: https://benjamincongdon.me/blog/2018/04/10/Counting-Scanned-Bytes-in-Go/
-			Idx: idx,
-		}
-		recs = append(recs, recidx)
+		recs = append(recs, *rec)
 	}
 	if err != io.EOF {
 		r.sugar.Error("Error while loading file", zap.Error(err))
@@ -97,7 +99,7 @@ func (r *RecIO) ReadFile(fpath string) ([]recordint.Indexed, int, error) {
 	r.sugar.Infow("Loaded resh history records",
 		"recordCount", len(recs),
 	)
-	return recs, numErrs, nil
+	return recs, nil, decodeErrs
 }
 
 func (r *RecIO) decodeLine(line string) (*record.V1, error) {

internal/recio/write.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 
-	"github.com/curusarn/resh/internal/recordint"
 	"github.com/curusarn/resh/record"
 )
 
@@ -29,16 +28,6 @@ func (r *RecIO) AppendToFile(fpath string, recs []record.V1) error {
 	return writeRecords(file, recs)
 }
 
-// TODO: better errors
-// TODO: rethink this
-func (r *RecIO) EditRecordFlagsInFile(fpath string, idx int, rec recordint.Flag) error {
-	// FIXME: implement
-	// open file "not as append"
-	// scan to the correct line
-	r.sugar.Error("not implemented yet (FIXME)")
-	return nil
-}
-
 func writeRecords(file *os.File, recs []record.V1) error {
 	for _, rec := range recs {
 		jsn, err := encodeV1Record(rec)

internal/recordint/flag.go

@@ -1,9 +0,0 @@
-package recordint
-
-type Flag struct {
-	deviceID string
-	recordID string
-
-	flagDeleted   bool
-	flagFavourite bool
-}

internal/recordint/indexed.go

@@ -1,11 +0,0 @@
-package recordint
-
-import "github.com/curusarn/resh/record"
-
-// TODO: rethink this
-
-// Indexed record allows us to find records in history file in order to edit them
-type Indexed struct {
-	Rec record.V1
-	Idx int
-}

internal/recordint/searchapp.go

@@ -5,7 +5,9 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/curusarn/resh/record"
 	giturls "github.com/whilp/git-urls"
+	"go.uber.org/zap"
 )
 
 // SearchApp record used for sending records to RESH-CLI
@@ -27,7 +29,6 @@ type SearchApp struct {
 	Idx int
 }
 
-// NewCliRecordFromCmdLine
 func NewSearchAppFromCmdLine(cmdLine string) SearchApp {
 	return SearchApp{
 		IsRaw:   true,
@@ -35,36 +36,36 @@ func NewSearchAppFromCmdLine(cmdLine string) SearchApp {
 	}
 }
 
-// NewCliRecord from EnrichedRecord
-func NewSearchApp(r *Indexed) SearchApp {
-	// TODO: we used to validate records with recutil.Validate()
-	// TODO: handle this error
-	time, _ := strconv.ParseFloat(r.Rec.Time, 64)
+// The error handling here could be better
+func NewSearchApp(sugar *zap.SugaredLogger, r *record.V1) SearchApp {
+	time, err := strconv.ParseFloat(r.Time, 64)
+	if err != nil {
+		sugar.Errorw("Error while parsing time as float", zap.Error(err),
+			"time", time)
+	}
 	return SearchApp{
 		IsRaw:     false,
-		SessionID: r.Rec.SessionID,
-		CmdLine:   r.Rec.CmdLine,
-		Host:      r.Rec.Device,
-		Pwd:       r.Rec.Pwd,
-		Home:      r.Rec.Home,
+		SessionID: r.SessionID,
+		CmdLine:   r.CmdLine,
+		Host:      r.Device,
+		Pwd:       r.Pwd,
+		Home:      r.Home,
 		// TODO: is this the right place to normalize the git remote
-		GitOriginRemote: normalizeGitRemote(r.Rec.GitOriginRemote),
-		ExitCode:        r.Rec.ExitCode,
+		GitOriginRemote: normalizeGitRemote(sugar, r.GitOriginRemote),
+		ExitCode:        r.ExitCode,
 		Time:            time,
-
-		Idx: r.Idx,
 	}
 }
 
 // TODO: maybe move this to a more appropriate place
 // normalizeGitRemote helper
-func normalizeGitRemote(gitRemote string) string {
-	if strings.HasSuffix(gitRemote, ".git") {
-		gitRemote = gitRemote[:len(gitRemote)-4]
-	}
+func normalizeGitRemote(sugar *zap.SugaredLogger, gitRemote string) string {
+	gitRemote = strings.TrimSuffix(gitRemote, ".git")
 	parsedURL, err := giturls.Parse(gitRemote)
 	if err != nil {
-		// TODO: log this error
+		sugar.Errorw("Failed to parse git remote", zap.Error(err),
+			"gitRemote", gitRemote,
+		)
 		return gitRemote
 	}
 	if parsedURL.User == nil || parsedURL.User.Username() == "" {

internal/searchapp/test.go

@@ -12,12 +12,12 @@ func LoadHistoryFromFile(sugar *zap.SugaredLogger, historyPath string, numLines
 	rio := recio.New(sugar)
 	recs, _, err := rio.ReadFile(historyPath)
 	if err != nil {
-		sugar.Panicf("failed to read hisotry file: %w", err)
+		sugar.Panicf("failed to read history file: %w", err)
 	}
 	if numLines != 0 && numLines < len(recs) {
 		recs = recs[:numLines]
 	}
-	cliRecords := histcli.New()
+	cliRecords := histcli.New(sugar)
 	for i := len(recs) - 1; i >= 0; i-- {
 		rec := recs[i]
 		cliRecords.AddRecord(&rec)

record/v1.go

@@ -54,6 +54,9 @@ type V1 struct {
 
 	// these look like internal stuff
 
+	// TODO: rethink
+	// I don't really like this :/
+	// Maybe just one field 'NotMerged' with 'partOne' and 'partTwo' as values and empty string for merged
 	// records come in two parts (collect and postcollect)
 	PartOne        bool `json:"partOne,omitempty"` // false => part two
 	PartsNotMerged bool `json:"partsNotMerged,omitempty"`