address outstanding TODOs

ayoisaiah · Oct 31, 2024 · 24e0567 · 24e0567
1 parent 3fc848b
commit 24e0567
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 77 deletions.
diff --git a/app/flag.go b/app/flag.go
@@ -158,7 +158,6 @@ var (
 		Ignores the file extension when searching for matches.`,
 	}
 
-	// TODO: JSON output should also be used for errors?
 	flagJSON = &cli.BoolFlag{
 		Name: "json",
 		Usage: `

diff --git a/find/find.go b/find/find.go
@@ -250,7 +250,6 @@ func searchPaths(conf *config.Config) (file.Changes, error) {
 
 				entryIsDir := entry.IsDir()
 
-				// FIXME:: Pairing should not affect how files are matched
 				if conf.IgnoreExt && !entryIsDir {
 					fileName = pathutil.StripExtension(fileName)
 				}

diff --git a/internal/sortfiles/sortfiles.go b/internal/sortfiles/sortfiles.go
@@ -242,7 +242,7 @@ func Changes(
 	changes file.Changes,
 	conf *config.Config,
 ) {
-	// TODO: EnforceHierarchicalOrder should be the default sort
+	// TODO: EnforceHierarchicalOrder should be the default sort?
 	if conf.SortPerDir {
 		EnforceHierarchicalOrder(changes)
 	}

diff --git a/replace/replace.go b/replace/replace.go
@@ -5,7 +5,6 @@ package replace
 
 import (
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/ayoisaiah/f2/internal/config"
@@ -16,59 +15,10 @@ import (
 	"github.com/ayoisaiah/f2/replace/variables"
 )
 
-// regexReplace replaces matched substrings in the input with the replacement.
-// It respects the specified replacement limit. A negative limit indicates that
-// replacement should start from the end of the fileName.
-func regexReplace(
-	regex *regexp.Regexp,
-	input, replacement string,
-	replaceLimit int,
-) string {
-	var output string
-
-	switch limit := replaceLimit; {
-	case limit > 0:
-		counter := 0
-		output = regex.ReplaceAllStringFunc(
-			input,
-			func(val string) string {
-				if counter == replaceLimit {
-					return val
-				}
-
-				counter++
-
-				return regex.ReplaceAllString(val, replacement)
-			},
-		)
-	case limit < 0:
-		matches := regex.FindAllString(input, -1)
-
-		l := len(matches) + limit
-		counter := 0
-		output = regex.ReplaceAllStringFunc(
-			input,
-			func(val string) string {
-				if counter >= l {
-					return regex.ReplaceAllString(val, replacement)
-				}
-
-				counter++
-
-				return val
-			},
-		)
-	default:
-		output = regex.ReplaceAllString(input, replacement)
-	}
-
-	return output
-}
-
 // replaceString replaces all matches in the filename
 // with the replacement string.
 func replaceString(conf *config.Config, originalName string) string {
-	return regexReplace(
+	return variables.RegexReplace(
 		conf.Search.Regex,
 		originalName,
 		conf.Replacement,
@@ -234,8 +184,7 @@ func Replace(
 		return nil, err
 	}
 
-	// TODO: This should also apply for CSV renaming
-	if conf.IncludeDir {
+	if (conf.IncludeDir || conf.CSVFilename != "") && conf.Exec {
 		sortfiles.ForRenamingAndUndo(changes, conf.Revert)
 	}
 

diff --git a/replace/replace_test/replace_test.go b/replace/replace_test/replace_test.go
@@ -145,10 +145,10 @@ func TestReplace(t *testing.T) {
 				},
 			},
 			Want: []string{
+				"music/2013/overgrown",
 				"music/Overgrown (2013)/01-overgrown.flac",
 				"music/Overgrown (2013)/02-i-am-sold.flac",
 				"music/Overgrown (2013)/cover.jpg",
-				"music/2013/overgrown",
 			},
 			Args: []string{
 				"-f",

diff --git a/replace/replace_test/variables_test.go b/replace/replace_test/variables_test.go
@@ -268,7 +268,7 @@ func TestVariables(t *testing.T) {
 			},
 			SetupFunc: createDateFile,
 		},
-		// TODO: Seem to be flaky
+		// FIXME: Seem to be flaky
 		// {
 		// 	Name: "use file birth and change times",
 		// 	Changes: file.Changes{

diff --git a/replace/variables/variables.go b/replace/variables/variables.go
@@ -95,9 +95,7 @@ func greatestCommonDivisor(a, b int) int {
 }
 
 // replaceSlashes replaces forward and backward slashes in the input with an
-// underscore character
-// TODO: Make replacement character configurable? Also possible to opt out per
-// variable?
+// underscore character.
 func replaceSlashes(input string) string {
 	r := strings.NewReplacer("/", "_", "\\", "_")
 	return r.Replace(input)
@@ -142,11 +140,10 @@ func integerToRoman(integer int) string {
 	return roman.String()
 }
 
-// TODO: move this
-// regexReplace replaces matched substrings in the input with the replacement.
+// RegexReplace replaces matched substrings in the input with the replacement.
 // It respects the specified replacement limit. A negative limit indicates that
 // replacement should start from the end of the fileName.
-func regexReplace(
+func RegexReplace(
 	regex *regexp.Regexp,
 	input, replacement string,
 	replaceLimit int,
@@ -239,7 +236,7 @@ func replaceFileHashVars(
 
 		hashValue = transformString(hashValue, current.transformToken)
 
-		target = regexReplace(current.regex, target, hashValue, 0)
+		target = RegexReplace(current.regex, target, hashValue, 0)
 	}
 
 	return target, nil
@@ -291,7 +288,7 @@ func replaceDateVars(
 
 		timeStr = transformString(timeStr, current.transformToken)
 
-		target = regexReplace(regex, target, timeStr, 0)
+		target = RegexReplace(regex, target, timeStr, 0)
 	}
 
 	return target, nil
@@ -395,7 +392,7 @@ func replaceID3Variables(
 
 		id3Tag = transformString(replaceSlashes(id3Tag), current.transformToken)
 
-		target = regexReplace(current.regex, target, id3Tag, 0)
+		target = RegexReplace(current.regex, target, id3Tag, 0)
 	}
 
 	return target, nil
@@ -616,7 +613,7 @@ func replaceExifVars(
 			current.transformToken,
 		)
 
-		target = regexReplace(regex, target, exifTag, 0)
+		target = RegexReplace(regex, target, exifTag, 0)
 	}
 
 	return target, nil
@@ -687,7 +684,7 @@ func replaceExifToolVars(
 
 		value = transformString(value, current.transformToken)
 
-		target = regexReplace(current.regex, target, value, 0)
+		target = RegexReplace(current.regex, target, value, 0)
 	}
 
 	return target, nil
@@ -787,14 +784,14 @@ func transformString(source, token string) string {
 		c := cases.Title(language.English)
 		return c.String(strings.ToLower(source))
 	case "win":
-		return regexReplace(
+		return RegexReplace(
 			osutil.CompleteWindowsForbiddenCharRegex,
 			source,
 			"",
 			0,
 		)
 	case "mac":
-		return regexReplace(osutil.MacForbiddenCharRegex, source, "", 0)
+		return RegexReplace(osutil.MacForbiddenCharRegex, source, "", 0)
 	case "di":
 		t := transform.Chain(
 			norm.NFD,
@@ -855,7 +852,7 @@ func replaceTransformVars(
 		// if capture variables aren't being used, transform the find matches
 		if match == "" {
 			for _, v := range matches {
-				target = regexReplace(
+				target = RegexReplace(
 					regex,
 					target,
 					transformString(v, current.token),
@@ -866,7 +863,7 @@ func replaceTransformVars(
 			continue
 		}
 
-		target = regexReplace(
+		target = RegexReplace(
 			regex,
 			target,
 			transformString(match, current.token),
@@ -893,7 +890,7 @@ func replaceCSVVars(target string, csvRow []string, cv csvVars) string {
 
 		value = transformString(value, current.transformToken)
 
-		target = regexReplace(current.regex, target, value, 0)
+		target = RegexReplace(current.regex, target, value, 0)
 	}
 
 	return target
@@ -932,7 +929,7 @@ func replaceParentDirVars(
 
 		source := transformString(parentDir, current.transformToken)
 
-		target = regexReplace(current.regex, target, source, 0)
+		target = RegexReplace(current.regex, target, source, 0)
 	}
 
 	return target
@@ -947,7 +944,7 @@ func replaceFilenameVars(
 
 		source := transformString(sourceName, current.transformToken)
 
-		target = regexReplace(current.regex, target, source, 0)
+		target = RegexReplace(current.regex, target, source, 0)
 	}
 
 	return target
@@ -976,7 +973,7 @@ func replaceExtVars(change *file.Change, ev extVars) (target string) {
 
 		source := transformString(fileExt, current.transformToken)
 
-		target = regexReplace(current.regex, change.Target, source, 0)
+		target = RegexReplace(current.regex, change.Target, source, 0)
 	}
 
 	return target