Merge pull request #92 from transifex/two_langs_in_file_filter

Support two <lang>s in file filters

Author: Konstantinos Bairaktaris

internal/txlib/add.go

@@ -46,13 +46,17 @@ type AddCommandArguments struct {
 }
 
 func validateFileFilter(input string) error {
-	res := strings.Count(input, "<lang>")
-	if res != 1 {
-		return errors.New("you need one <lang> in your File Filter")
-	}
 	if len(filepath.Ext(input)) <= 1 {
 		return errors.New("you need to add an extension to your file")
 	}
+	input = normaliseFileFilter(input)
+	for _, part := range strings.Split(input, string(os.PathSeparator)) {
+		if strings.Count(part, "<lang>") > 1 {
+			return errors.New(
+				"<lang> cannot appear more than once in the same part of the path",
+			)
+		}
+	}
 	return nil
 }
 

internal/txlib/file_filter_test.go

@@ -11,27 +11,12 @@ import (
 )
 
 func beforeFileFilterTest(t *testing.T) func() {
-	curDir, err := os.Getwd()
-	if err != nil {
-		t.Error(err)
-	}
-	tempDir, err := os.MkdirTemp("", "")
-	if err != nil {
-		t.Error(err)
-	}
-	err = os.Chdir(tempDir)
-	if err != nil {
-		t.Error(err)
-	}
+	curDir, _ := os.Getwd()
+	tempDir, _ := os.MkdirTemp("", "")
+	_ = os.Chdir(tempDir)
 	return func() {
-		err = os.Chdir(curDir)
-		if err != nil {
-			t.Error(err)
-		}
-		err := os.RemoveAll(tempDir)
-		if err != nil {
-			t.Error(err)
-		}
+		_ = os.Chdir(curDir)
+		_ = os.RemoveAll(tempDir)
 	}
 }
 
@@ -125,6 +110,53 @@ func TestSearchFileFilterDirs(t *testing.T) {
 	}
 }
 
+func TestSearchFileWithTwoLangs(t *testing.T) {
+	afterTest := beforeFileFilterTest(t)
+	defer afterTest()
+
+	// <curDir>/
+	//   + en/
+	//   | + foo/
+	//   |   + en.txt
+	//   |   + fr.txt
+	err := os.Mkdir("en", os.ModeDir|0755)
+	if err != nil {
+		t.Error(err)
+	}
+	err = os.Mkdir(filepath.Join("en", "foo"), os.ModeDir|0755)
+	if err != nil {
+		t.Error(err)
+	}
+	file1, err := os.OpenFile(
+		filepath.Join("en", "foo", "en.txt"),
+		os.O_RDWR|os.O_CREATE|os.O_TRUNC,
+		0755,
+	)
+	if err != nil {
+		t.Error(err)
+	}
+	defer file1.Close()
+	file2, err := os.OpenFile(
+		filepath.Join("en", "foo", "fr.txt"),
+		os.O_RDWR|os.O_CREATE|os.O_TRUNC,
+		0755,
+	)
+	if err != nil {
+		t.Error(err)
+	}
+	defer file2.Close()
+
+	curDir, err := os.Getwd()
+	if err != nil {
+		t.Error(err)
+	}
+	actual := searchFileFilter(curDir, filepath.Join("<lang>", "foo", "<lang>.txt"))
+	expected := map[string]string{"en": filepath.Join(curDir, "en", "foo", "en.txt")}
+	if !reflect.DeepEqual(actual, expected) {
+		t.Errorf("Got '%+v', expected '%+v'", actual, expected)
+	}
+}
+
 func TestNormaliseFileFilterLinuxBased(t *testing.T) {
 	result := normaliseFileFilter("en/text.txt")
 	expected := filepath.Join("en", "text.txt")

internal/txlib/filefilter.go

@@ -1,32 +1,29 @@
 package txlib
 
 import (
-	"io/ioutil"
 	"os"
 	"strings"
 )
 
 const PathSeparator = string(os.PathSeparator)
 
 /*
-Recursively search under the directory 'root' for files that match the
-'fileFilter'. The original file filter must have exactly one instance of
-"<lang>" in it.
+Recursively search under the directory 'root' for files that match the 'fileFilter'.
 
 If nothing is found, an empty map will be returned.
 
-If 'fileFilter' is empty, which means that we are in the last step of the
-recursion, 'root' is returned if it exists in the filesystem and is a file.
+If 'fileFilter' is empty, which means that we are in the last step of the recursion,
+'root' is returned if it exists in the filesystem and is a file.
 
-If, in the current iteration, the file filter does not have "<lang>" (which
-means that the "<lang>" part is now in the 'root'), then the matching file, if
-found, will be returned under the "" key ({"": "/path/to/file.txt"}).
+If, in the current iteration, the file filter does not have "<lang>" (which means that
+the "<lang>" part is now in the 'root'), then the matching file, if found, will be
+returned under the "" key ({"": "/path/to/file.txt"}).
 
-If the first item in 'fileFilter' does have a "<lang>" in it, then the contents
-of the 'root' directory (it must be a directory) will be matched against the
-pattern, the function will be called recursively for a new root that contains
-the matched path and the result of the recursive function will be added to the
-result of the current function with the matched language as the key.
+If the first item in 'fileFilter' does have a "<lang>" in it, then the contents of the
+'root' directory (it must be a directory) will be matched against the pattern, the
+function will be called recursively for a new root that contains the matched path and
+the result of the recursive function will be added to the result of the current function
+with the matched language as the key.
 
 Examples:
 
@@ -35,19 +32,19 @@ Examples:
       |
       + file.txt
 
-Then the invocation of:
+  Then the invocation of:
 
     searchFileFilter("/path/to/root/file.txt", "")
 
-will check that 'root' does exist and is not a directory and return:
+  will check that 'root' does exist and is not a directory and return:
 
     map[string]string{"": "/path/to/root/file.txt"}
 
-The invocation of:
+  The invocation of:
 
     searchFileFilter("/path/to/root", "file.txt")
 
-will recursively call the previous invocation and return its result:
+  will recursively call the previous invocation and return its result:
 
     map[string]string{"": "/path/to/root/file.txt"}
 
@@ -59,26 +56,26 @@ will recursively call the previous invocation and return its result:
       |
       + fr.txt
 
-The invocation of:
+  The invocation of:
 
     searchFileFilter("/path/to/root/en.txt", "")
 
-as before, will return:
+  as before, will return:
 
     map[string]string{"": "/path/to/root/en.txt"}
 
-But, the invocation of:
+  But, the invocation of:
 
     searchFileFilter("/path/to/root", "<lang>.txt")
 
-will inspect the contents of 'root', match the 2 files against the pattern,
-make 2 invocations similar to the first (one with "en" in 'root' and one with
-"fr") and return their results using the matched language codes as keys. So:
+  will inspect the contents of 'root', match the 2 files against the pattern, make 2
+  invocations similar to the first (one with "en" in 'root' and one with "fr") and
+  return their results using the matched language codes as keys. So:
 
     map[string]string{"en": "/path/to/root/en.txt",
                       "fr": "/path/to/root/fr.txt"}
 
-3. Finally, assuming the filesystem looks like this:
+3. Assuming the filesystem looks like this:
 
     /path/to/root/
       |
@@ -90,7 +87,7 @@ make 2 invocations similar to the first (one with "en" in 'root' and one with
           |
           + file.txt
 
-The following calls and results will happen:
+  The following calls and results will happen:
 
     searchFileFilter("/path/to/root/en/file.txt", "")
     // map[string]string{"": "/path/to/root/en/file.txt"}
@@ -102,12 +99,12 @@ The following calls and results will happen:
     // map[string]string{"en": "/path/to/root/en/file.txt",
                          "fr": "/path/to/root/fr/file.txt"}
 */
-func searchFileFilter(root string, fileFilter string) map[string]string {
+
+func searchFileFilter(root, fileFilter string) map[string]string {
 	result := make(map[string]string)
 
 	fileFilter = normaliseFileFilter(fileFilter)
 
-	fileFilterSlice := strings.Split(fileFilter, PathSeparator)
 	if len(fileFilter) == 0 {
 		fileInfo, err := os.Stat(root)
 		if err != nil || fileInfo.IsDir() {
@@ -116,21 +113,18 @@ func searchFileFilter(root string, fileFilter string) map[string]string {
 		result[""] = root
 		return result
 	}
-
+	fileFilterSlice := strings.Split(fileFilter, PathSeparator)
 	if !strings.Contains(fileFilterSlice[0], "<lang>") {
 		// Recursively go deeper
-		newRoot := strings.Join([]string{root, fileFilterSlice[0]},
-			PathSeparator)
+		newRoot := strings.Join([]string{root, fileFilterSlice[0]}, PathSeparator)
 		newFileFilter := strings.Join(fileFilterSlice[1:], PathSeparator)
 		return searchFileFilter(newRoot, newFileFilter)
 	} else {
-		// Sometime before we checked that the original 'fileFilterSlice' had
-		// exactly one "<lang>" in it, so 'parts' is guaranteed to be of size 2
 		parts := strings.Split(fileFilterSlice[0], "<lang>")
 		left := parts[0]
 		right := parts[1]
 
-		fileInfos, err := ioutil.ReadDir(root)
+		fileInfos, err := os.ReadDir(root)
 		if err != nil {
 			return result
 		}
@@ -149,6 +143,8 @@ func searchFileFilter(root string, fileFilter string) map[string]string {
 
 			newRoot := strings.Join([]string{root, name}, PathSeparator)
 			newFileFilter := strings.Join(fileFilterSlice[1:], PathSeparator)
+			// IT doesn't make sense to capture 'en/fr' with '<lang>/<lang>'
+			newFileFilter = strings.ReplaceAll(newFileFilter, "<lang>", languageCode)
 			answer := searchFileFilter(newRoot, newFileFilter)
 
 			path, exists := answer[""]

internal/txlib/pull.go

@@ -114,7 +114,6 @@ func (task *ResourcePullTask) Run(send func(string), abort func()) {
 	filePullTaskChannel := task.filePullTaskChannel
 	cfg := task.cfg
 
-
 	sendMessage := func(body string) {
 		send(fmt.Sprintf(
 			"%s.%s - %s",

internal/txlib/utils.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/gosimple/slug"
@@ -145,12 +144,7 @@ func handleThrottling(do func() error, initialMsg string, send func(string)) err
 func checkFileFilter(fileFilter string) error {
 	if fileFilter == "" {
 		return errors.New("file filter is empty")
-	} else if strings.Count(fileFilter, "<lang>") != 1 {
-		return fmt.Errorf(
-			"file filter '%s' should have exactly one occurrence of '<lang>'",
-			fileFilter,
-		)
 	} else {
-		return nil
+		return validateFileFilter(fileFilter)
 	}
 }