Rewrite PO headers parsing and handling. Implement correct GNU gettext headers format. Fix tests. Fixes #10

Author: leonelquinteros

gotext_test.go

@@ -3,6 +3,7 @@ package gotext
 import (
 	"os"
 	"path"
+	"sync"
 	"testing"
 )
 
@@ -32,10 +33,12 @@ func TestGettersSetters(t *testing.T) {
 func TestPackageFunctions(t *testing.T) {
 	// Set PO content
 	str := `
-# msgid ""
-# msgstr ""
+msgid   ""
+msgstr  "Project-Id-Version: %s\n"
+        "Report-Msgid-Bugs-To: %s\n"
+        
 # Initial comment
-# Headers below
+# More Headers below
 "Language: en\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
@@ -55,14 +58,6 @@ msgstr[0] "This one is the singular: %s"
 msgstr[1] "This one is the plural: %s"
 msgstr[2] "And this is the second plural form: %s"
 
-msgid "This one has invalid syntax translations"
-msgid_plural "Plural index"
-msgstr[abc] "Wrong index"
-msgstr[1 "Forgot to close brackets"
-msgstr[0] "Badly formatted string'
-
-msgid "Invalid formatted id[] with no translations
-
 msgctxt "Ctx"
 msgid "One with var: %s"
 msgid_plural "Several with vars: %s"
@@ -149,8 +144,8 @@ msgstr[1] ""
 func TestUntranslated(t *testing.T) {
 	// Set PO content
 	str := `
-# msgid ""
-# msgstr ""
+msgid ""
+msgstr ""
 # Initial comment
 # Headers below
 "Language: en\n"
@@ -258,22 +253,29 @@ msgstr[2] "And this is the second plural form: %s"
 		t.Fatalf("Can't write to test file: %s", err.Error())
 	}
 
-	// Init sync channels
-	c1 := make(chan bool)
-	c2 := make(chan bool)
+	var wg sync.WaitGroup
 
 	for i := 0; i < 100; i++ {
+		wg.Add(1)
 		// Test translations
-		go func(done chan bool) {
+		go func() {
+			defer wg.Done()
+
 			Get("My text")
-			done <- true
-		}(c1)
+			GetN("One with var: %s", "Several with vars: %s", 0, "test")
+		}()
+
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
 
-		go func(done chan bool) {
 			Get("My text")
-			done <- true
-		}(c2)
+			GetN("One with var: %s", "Several with vars: %s", 1, "test")
+		}()
 
 		Get("My text")
+		GetN("One with var: %s", "Several with vars: %s", 2, "test")
 	}
+
+	wg.Wait()
 }

locale_test.go

@@ -9,8 +9,8 @@ import (
 func TestLocale(t *testing.T) {
 	// Set PO content
 	str := `
-# msgid ""
-# msgstr ""
+msgid ""
+msgstr ""
 # Initial comment
 # Headers below
 "Language: en\n"
@@ -38,8 +38,6 @@ msgstr[abc] "Wrong index"
 msgstr[1 "Forgot to close brackets"
 msgstr[0] "Badly formatted string'
 
-msgid "Invalid formatted id[] with no translations
-
 msgctxt "Ctx"
 msgid "One with var: %s"
 msgid_plural "Several with vars: %s"

po.go

@@ -79,8 +79,8 @@ Example:
 
 */
 type Po struct {
-	// Headers
-	RawHeaders string
+	// Headers storage
+	Headers textproto.MIMEHeader
 
 	// Language header
 	Language string
@@ -140,7 +140,6 @@ func (po *Po) ParseFile(f string) {
 func (po *Po) Parse(str string) {
 	// Lock while parsing
 	po.Lock()
-	defer po.Unlock()
 
 	// Init storage
 	if po.translations == nil {
@@ -195,38 +194,42 @@ func (po *Po) Parse(str string) {
 
 		// Multi line strings and headers
 		if strings.HasPrefix(l, "\"") && strings.HasSuffix(l, "\"") {
-			state = po.parseString(l, state)
+			po.parseString(l, state)
 			continue
 		}
 	}
 
 	// Save last translation buffer.
 	po.saveBuffer()
 
+	// Unlock to parse headers
+	po.Unlock()
+
 	// Parse headers
 	po.parseHeaders()
 }
 
 // saveBuffer takes the context and translation buffers
 // and saves it on the translations collection
 func (po *Po) saveBuffer() {
-	// If we have something to save...
-	if po.trBuffer.id != "" {
-		// With no context...
-		if po.ctxBuffer == "" {
-			po.translations[po.trBuffer.id] = po.trBuffer
-		} else {
-			// With context...
-			if _, ok := po.contexts[po.ctxBuffer]; !ok {
-				po.contexts[po.ctxBuffer] = make(map[string]*translation)
-			}
-			po.contexts[po.ctxBuffer][po.trBuffer.id] = po.trBuffer
+	// With no context...
+	if po.ctxBuffer == "" {
+		po.translations[po.trBuffer.id] = po.trBuffer
+	} else {
+		// With context...
+		if _, ok := po.contexts[po.ctxBuffer]; !ok {
+			po.contexts[po.ctxBuffer] = make(map[string]*translation)
 		}
+		po.contexts[po.ctxBuffer][po.trBuffer.id] = po.trBuffer
 
-		// Flush buffer
-		po.trBuffer = newTranslation()
-		po.ctxBuffer = ""
+		// Cleanup current context buffer if needed
+		if po.trBuffer.id != "" {
+			po.ctxBuffer = ""
+		}
 	}
+
+	// Flush translation buffer
+	po.trBuffer = newTranslation()
 }
 
 // parseContext takes a line starting with "msgctxt",
@@ -286,70 +289,72 @@ func (po *Po) parseMessage(l string) {
 
 // parseString takes a well formatted string without prefix
 // and creates headers or attach multi-line strings when corresponding
-func (po *Po) parseString(l string, state parseState) parseState {
+func (po *Po) parseString(l string, state parseState) {
+	clean, _ := strconv.Unquote(l)
+
 	switch state {
 	case msgStr:
-		// Check for multiline from previously set msgid
-		if po.trBuffer.id != "" {
-			// Append to last translation found
-			uq, _ := strconv.Unquote(l)
-			po.trBuffer.trs[len(po.trBuffer.trs)-1] += uq
+		// Append to last translation found
+		po.trBuffer.trs[len(po.trBuffer.trs)-1] += clean
 
-		}
 	case msgID:
 		// Multiline msgid - Append to current id
-		uq, _ := strconv.Unquote(l)
-		po.trBuffer.id += uq
+		po.trBuffer.id += clean
+
 	case msgIDPlural:
 		// Multiline msgid - Append to current id
-		uq, _ := strconv.Unquote(l)
-		po.trBuffer.pluralID += uq
+		po.trBuffer.pluralID += clean
+
 	case msgCtxt:
 		// Multiline context - Append to current context
-		ctxt, _ := strconv.Unquote(l)
-		po.ctxBuffer += ctxt
-	default:
-		// Otherwise is a header
-		h, _ := strconv.Unquote(strings.TrimSpace(l))
-		po.RawHeaders += h
-		return head
-	}
+		po.ctxBuffer += clean
 
-	return state
+	}
 }
 
 // isValidLine checks for line prefixes to detect valid syntax.
 func (po *Po) isValidLine(l string) bool {
-	// Skip empty lines
-	if l == "" {
-		return false
+	// Check prefix
+	valid := []string{
+		"\"",
+		"msgctxt",
+		"msgid",
+		"msgid_plural",
+		"msgstr",
 	}
 
-	// Check prefix
-	if !strings.HasPrefix(l, "\"") && !strings.HasPrefix(l, "msgctxt") && !strings.HasPrefix(l, "msgid") && !strings.HasPrefix(l, "msgid_plural") && !strings.HasPrefix(l, "msgstr") {
-		return false
+	for _, v := range valid {
+		if strings.HasPrefix(l, v) {
+			return true
+		}
 	}
 
-	return true
+	return false
 }
 
 // parseHeaders retrieves data from previously parsed headers
 func (po *Po) parseHeaders() {
 	// Make sure we end with 2 carriage returns.
-	po.RawHeaders += "\n\n"
+	raw := po.Get("") + "\n\n"
 
 	// Read
-	reader := bufio.NewReader(strings.NewReader(po.RawHeaders))
+	reader := bufio.NewReader(strings.NewReader(raw))
 	tp := textproto.NewReader(reader)
 
-	mimeHeader, err := tp.ReadMIMEHeader()
+	var err error
+
+	// Sync Headers write.
+	po.Lock()
+	defer po.Unlock()
+
+	po.Headers, err = tp.ReadMIMEHeader()
 	if err != nil {
 		return
 	}
 
 	// Get/save needed headers
-	po.Language = mimeHeader.Get("Language")
-	po.PluralForms = mimeHeader.Get("Plural-Forms")
+	po.Language = po.Headers.Get("Language")
+	po.PluralForms = po.Headers.Get("Plural-Forms")
 
 	// Parse Plural-Forms formula
 	if po.PluralForms == "" {
@@ -422,12 +427,12 @@ func (po *Po) Get(str string, vars ...interface{}) string {
 
 	if po.translations != nil {
 		if _, ok := po.translations[str]; ok {
-			return fmt.Sprintf(po.translations[str].get(), vars...)
+			return po.printf(po.translations[str].get(), vars...)
 		}
 	}
 
 	// Return the same we received by default
-	return fmt.Sprintf(str, vars...)
+	return po.printf(str, vars...)
 }
 
 // GetN retrieves the (N)th plural form of translation for the given string.
@@ -439,15 +444,14 @@ func (po *Po) GetN(str, plural string, n int, vars ...interface{}) string {
 
 	if po.translations != nil {
 		if _, ok := po.translations[str]; ok {
-			return fmt.Sprintf(po.translations[str].getN(po.pluralForm(n)), vars...)
+			return po.printf(po.translations[str].getN(po.pluralForm(n)), vars...)
 		}
 	}
 
 	if n == 1 {
-		return fmt.Sprintf(str, vars...)
+		return po.printf(str, vars...)
 	}
-
-	return fmt.Sprintf(plural, vars...)
+	return po.printf(plural, vars...)
 }
 
 // GetC retrieves the corresponding translation for a given string in the given context.
@@ -461,14 +465,14 @@ func (po *Po) GetC(str, ctx string, vars ...interface{}) string {
 		if _, ok := po.contexts[ctx]; ok {
 			if po.contexts[ctx] != nil {
 				if _, ok := po.contexts[ctx][str]; ok {
-					return fmt.Sprintf(po.contexts[ctx][str].get(), vars...)
+					return po.printf(po.contexts[ctx][str].get(), vars...)
 				}
 			}
 		}
 	}
 
 	// Return the string we received by default
-	return fmt.Sprintf(str, vars...)
+	return po.printf(str, vars...)
 }
 
 // GetNC retrieves the (N)th plural form of translation for the given string in the given context.
@@ -482,14 +486,23 @@ func (po *Po) GetNC(str, plural string, n int, ctx string, vars ...interface{})
 		if _, ok := po.contexts[ctx]; ok {
 			if po.contexts[ctx] != nil {
 				if _, ok := po.contexts[ctx][str]; ok {
-					return fmt.Sprintf(po.contexts[ctx][str].getN(po.pluralForm(n)), vars...)
+					return po.printf(po.contexts[ctx][str].getN(po.pluralForm(n)), vars...)
 				}
 			}
 		}
 	}
 
 	if n == 1 {
+		return po.printf(str, vars...)
+	}
+	return po.printf(plural, vars...)
+}
+
+// printf applies text formatting only when needed to parse variables.
+func (po *Po) printf(str string, vars ...interface{}) string {
+	if len(vars) > 0 {
 		return fmt.Sprintf(str, vars...)
 	}
-	return fmt.Sprintf(plural, vars...)
+
+	return str
 }