Skip to content

Commit b548d88

Browse files
authored
Merge pull request #22 from mmonterroca/fix/pr20-code-review
fix: address PR #20 code review comments
2 parents 19e5da3 + 613cefb commit b548d88

File tree

6 files changed

+37
-30
lines changed

6 files changed

+37
-30
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func main() {
173173

174174
### More Examples
175175

176-
See the [`examples/`](examples/) directory for 14 comprehensive examples:
176+
See the [`examples/`](examples/) directory for 15 comprehensive examples:
177177

178178
- **[01_basic](examples/01_basic/)** - Simple document with builder pattern
179179
- **[02_intermediate](examples/02_intermediate/)** - Professional product catalog

RELEASE_NOTES_v2.3.0.md

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,65 +70,67 @@ import (
7070

7171
doc := docx.NewDocument()
7272
para, _ := doc.AddParagraph()
73-
para.AddRun().SetText("Hello {{name}}, welcome to {{company}}!")
73+
run, _ := para.AddRun()
74+
run.SetText("Hello {{name}}, welcome to {{company}}!")
7475

75-
data := map[string]string{
76+
data := template.MergeData{
7677
"name": "John",
7778
"company": "Acme Corp",
7879
}
7980

80-
result, _ := template.MergeTemplate(doc, data, nil)
81-
result.SaveAs("output.docx")
81+
err := template.MergeTemplate(doc, data)
82+
// MergeTemplate modifies the document in place
83+
doc.SaveAs("output.docx")
8284
```
8385

8486
### External Word Template with MERGEFIELDs
8587

8688
```go
8789
doc, _ := docx.OpenDocument("template.docx")
8890

89-
opts := &template.MergeOptions{
91+
opts := template.MergeOptions{
9092
OpenDelimiter: "«",
9193
CloseDelimiter: "»",
9294
}
9395

94-
data := map[string]string{
96+
data := template.MergeData{
9597
"Contact_FullName": "Jane Doe",
9698
"Account_Name": "Acme Corp",
9799
}
98100

99-
result, _ := template.MergeTemplate(doc, data, opts)
100-
result.SaveAs("merged_output.docx")
101+
err := template.MergeTemplate(doc, data, opts)
102+
// MergeTemplate modifies the document in place
103+
doc.SaveAs("merged_output.docx")
101104
```
102105

103106
### Batch Merge
104107

105108
```go
106-
doc, _ := docx.OpenDocument("invoice_template.docx")
107-
108-
records := []map[string]string{
109+
records := []template.MergeData{
109110
{"name": "John Smith", "amount": "$1,500.00"},
110111
{"name": "Alice Johnson", "amount": "$2,300.00"},
111112
}
112113

113-
results, _ := template.BatchMerge("invoice_template.docx", records, nil)
114-
for i, result := range results {
115-
result.SaveAs(fmt.Sprintf("invoice_%d.docx", i+1))
114+
for i, data := range records {
115+
doc, _ := docx.OpenDocument("invoice_template.docx")
116+
template.MergeTemplate(doc, data)
117+
doc.SaveAs(fmt.Sprintf("invoice_%d.docx", i+1))
116118
}
117119
```
118120

119121
## Files Added
120122

121-
- `pkg/template/engine.go`Main template engine
123+
- `pkg/template/doc.go`Package documentation
122124
- `pkg/template/merge.go` — Core merge logic
123125
- `pkg/template/merge_test.go` — Merge tests
124126
- `pkg/template/placeholder.go` — Placeholder detection
125127
- `pkg/template/placeholder_test.go` — Placeholder tests
126128
- `pkg/template/consolidate.go` — Run consolidation
127129
- `pkg/template/consolidate_test.go` — Consolidation tests
128-
- `pkg/template/validate.go`Template validation
129-
- `pkg/template/validate_test.go`Validation tests
130+
- `pkg/template/format.go`Run formatting comparison helper
131+
- `pkg/template/options.go`MergeData, MergeOptions, ValidationError types
130132
- `pkg/template/walk.go` — Document traversal
131-
- `pkg/template/batch.go`Batch merge support
133+
- `pkg/template/integration_test.go`Integration tests
132134
- `examples/14_mail_merge/main.go` — Programmatic template example
133135
- `examples/15_external_template/main.go` — External Word template example
134136

docs/V2_DESIGN.md

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,9 +1350,5 @@ See [CREDITS.md](../CREDITS.md) for complete project history.
13501350
- ✅ Advanced tables: Complete (merging, nesting, 8 styles, proper border serialization)
13511351
- ✅ Theme system: Complete (7 preset themes, custom theme support)
13521352
- ✅ Document reading: Complete (round-trip with style/hyperlink/image preservation)
1353-
- ✅ Documentation: Comprehensive (13 working examples, full API guide)
1353+
- ✅ Documentation: Comprehensive (15 working examples, full API guide)
13541354
- ✅ All 12 phases complete
1355-
1356-
```
1357-
1358-
```

examples/15_external_template/main.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,22 @@ func main() {
4343
// -------------------------------------------------------------------------
4444
// Step 2: Configure custom delimiters for Word-style merge fields
4545
// -------------------------------------------------------------------------
46+
fmt.Println("Step 2: Configuring custom delimiters for Word-style merge fields...")
4647
// Microsoft Word uses « » (guillemet) delimiters for mail merge fields.
4748
// Our template engine supports custom delimiters through MergeOptions.
4849
opts := template.MergeOptions{
4950
OpenDelimiter: "«",
5051
CloseDelimiter: "»",
5152
StrictMode: true, // require all placeholders to have data
5253
}
54+
fmt.Println(" Delimiters: « »")
55+
fmt.Println(" Strict mode: enabled")
56+
fmt.Println()
5357

5458
// -------------------------------------------------------------------------
5559
// Step 3: Inspect the template — discover all placeholders
5660
// -------------------------------------------------------------------------
57-
fmt.Println("Step 2: Inspecting template placeholders...")
61+
fmt.Println("Step 3: Inspecting template placeholders...")
5862

5963
// Use FindPlaceholdersCustom with a pattern matching « » delimiters
6064
pattern := buildGuillemetsPattern()
@@ -89,7 +93,7 @@ func main() {
8993
// -------------------------------------------------------------------------
9094
// Step 4: Prepare merge data and validate
9195
// -------------------------------------------------------------------------
92-
fmt.Println("Step 3: Validating template against data...")
96+
fmt.Println("Step 4: Validating template against data...")
9397

9498
data := template.MergeData{
9599
// Organization info
@@ -142,7 +146,7 @@ func main() {
142146
// -------------------------------------------------------------------------
143147
// Step 5: Merge the template to produce a personalized letter
144148
// -------------------------------------------------------------------------
145-
fmt.Println("Step 4: Merging template with data...")
149+
fmt.Println("Step 5: Merging template with data...")
146150

147151
if err := template.MergeTemplate(doc, data, opts); err != nil {
148152
log.Fatalf("Merge failed: %v", err)
@@ -157,7 +161,7 @@ func main() {
157161
// -------------------------------------------------------------------------
158162
// Step 6: Verify — reopen the merged document and print its content
159163
// -------------------------------------------------------------------------
160-
fmt.Println("Step 5: Verifying merged document content...")
164+
fmt.Println("Step 6: Verifying merged document content...")
161165

162166
merged, err := docx.OpenDocument(outputPath)
163167
if err != nil {
@@ -175,7 +179,7 @@ func main() {
175179
// -------------------------------------------------------------------------
176180
// Step 7: Batch merge — send reminders to multiple contacts
177181
// -------------------------------------------------------------------------
178-
fmt.Println("Step 6: Batch merge for multiple contacts...")
182+
fmt.Println("Step 7: Batch merge for multiple contacts...")
179183

180184
contacts := []template.MergeData{
181185
{

examples/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ Run the included test script to verify all examples compile:
269269
./test_all.sh
270270
```
271271

272-
This will test all 14 working examples and report results.
272+
This will test all 15 working examples and report results.
273273

274274
---
275275

pkg/template/placeholder.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ func scanParagraph(para domain.Paragraph, pattern *regexp.Regexp, ctx paragraphC
9494
matches := pattern.FindAllStringSubmatchIndex(text, -1)
9595

9696
for _, match := range matches {
97+
// Ensure there is at least one capturing group before indexing.
98+
if len(match) < 4 {
99+
// Pattern does not provide the expected capturing group; skip this match.
100+
continue
101+
}
97102
// match[0:2] is full match, match[2:4] is first capture group
98103
fullMatch := text[match[0]:match[1]]
99104
name := text[match[2]:match[3]]

0 commit comments

Comments
 (0)