handle other alignments alongside "J" in UTF8 CellFormat #105

JaredReisinger · 2025-10-23T02:37:25+02:00

JaredReisinger commented

2025-10-23 02:37:25 +02:00

The "J" justification handling in CellFormat() was an equality test, not a strings.Contains() check (like the others), and therefore it was only succeeding on a "J" with no other (vertical) alignment.

This changes that check to strings.Contains() and adds a test that verifies that the justification codepath is executed even for an alignment like "JA".

Fixes #104

The "J" justification handling in CellFormat() was an equality test, not a `strings.Contains()` check (like the others), and therefore it was *only* succeeding on a "J" with no other (vertical) alignment. This changes that check to `strings.Contains()` and adds a test that verifies that the justification codepath is executed even for an alignment like "JA". Fixes #104

JaredReisinger added 1 commit

2025-10-23 02:37:26 +02:00

handle other alignments alongside "J" in UTF8 CellFormat 58108610d0

The "J" justification handling in CellFormat() was an equality test,
not a `strings.Contains()` check (like the others), and therefore it
was *only* succeeding on a "J" with no other (vertical) alignment.

This changes that check to `strings.Contains()` and adds a test that
verifies that the justification codepath is executed even for an
alignment like "JA".

Fixes #104

JaredReisinger added 1 commit

2025-10-23 02:51:01 +02:00

missed a duplicated dy offset calculation 6c81fcf0ee

(This logic really could stand some refactoring, although that will
 make it drift even further from the original FPDF.)

sbinet reviewed

2025-10-30 16:33:17 +01:00

sbinet left a comment

hi,

thanks for your interest in fpdf and for tackling this.

I have a couple of minor comments.
see below.

hi, thanks for your interest in `fpdf` and for tackling this. I have a couple of minor comments. see below.

fpdf_test.go Outdated

					
				@ -329,0 +349,4 @@

					//

					//   BT 31.19 785.17 Td (This is justified-baseline text.)Tj ET

					//

					// If is *is* justified, well get placement of individual words:

-// If is *is* justified, well get placement of individual words:
+// If it *is* justified, we'll get placement of individual words:

```diff -// If is *is* justified, well get placement of individual words: +// If it *is* justified, we'll get placement of individual words: ```

🤦, thanks for catching the typo!

fpdf_test.go

					
				@ -329,0 +369,4 @@

					// we're getting to the correct codepath.

					// pattern := regexp.MustCompile(`\nBT 0 Tw .* Td \[\(This\) .* \(is\) .* \(justified-baseline\) .* \(text.\) ] TJ ET\n`)

					pattern := regexp.MustCompile(`\bBT 0 Tw .* Td \[\(`)

couldn't we just look for the exact string BT 0 Tw 31.19 796.37 Td [(This) -4870.000( ) (is) -4870.000( ) (justified-baseline) -4870.000( ) (text.) ] TJ ET instead ?

surely the PDF production is reproducible, right ?

couldn't we just look for the exact string `BT 0 Tw 31.19 796.37 Td [(This) -4870.000( ) (is) -4870.000( ) (justified-baseline) -4870.000( ) (text.) ] TJ ET` instead ? surely the PDF production is reproducible, right ?

What I wasn't sure about, since this was my first foray into fpdf, was whether running on different machines might have slight differences in the low-order bits of the float64. I was erring on the side of "don't make the test overly fragile". If you think an exact string match won't be too fragile, I'm happy to change it to that. 😊

What I wasn't sure about, since this was my first foray into `fpdf`, was whether running on different machines might have slight differences in the low-order bits of the float64. I was erring on the side of "don't make the test overly fragile". If you think an exact string match won't be too fragile, I'm happy to change it to that. 😊

the original fpdf code (and this one still) was actually comparing bytes-for-bytes equality with reference files:

https://codeberg.org/go-pdf/fpdf/src/branch/main/internal/example/example.go#L132

so I think we can go the exact string match. (and adapt later on if stringified floating-point comparison bites us later on in the game).

the original `fpdf` code (and this one still) was actually comparing bytes-for-bytes equality with reference files: https://codeberg.org/go-pdf/fpdf/src/branch/main/internal/example/example.go#L132 so I think we can go the exact string match. (and adapt later on if stringified floating-point comparison bites us later on in the game).

looking at the produced PDF, I figured out why the "full" regexp doesn't match. The PDF actually contains:

BT 0 Tw 31.19 785.17 Td [(^@T^@h^@i^@s) -4870.000(^@ ) (^@i^@s) -4870.000(^@ ) (^@j^@u^@s^@t^@i^@f^@i^@e^@d^@-^@b^@a^@s^@e^@l^@i^@n^@e) -4870.000(^@ ) (^@t^@e^@x^@t^@.) ] TJ ET

so I'd suggest the following test:

diff --git a/fpdf_test.go b/fpdf_test.go
index 4dd037b..2d41f0e 100644
--- a/fpdf_test.go
+++ b/fpdf_test.go
@@ -25,7 +25,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"testing"
 
@@ -341,8 +340,8 @@ func TestIssue104CellFormatJustifiedVerticalAlign(t *testing.T) {
 	pdf.SetFont("DejaVu", "", 10)
 
 	pdf.CellFormat(100, 10, "This is justified-baseline text.", "", 0, "JA", false, 0, "")
-	if pdf.Error() != nil {
-		t.Fatalf("not expecting error when rendering text")
+	if err := pdf.Error(); err != nil {
+		t.Fatalf("could not render text: %v", err)
 	}
 
 	// If we didn't get justified text, the PDF output will contain:
@@ -352,26 +351,13 @@ func TestIssue104CellFormatJustifiedVerticalAlign(t *testing.T) {
 	// If it *is* justified, we'll get placement of individual words:
 	//
 	//   BT 0 Tw 31.19 796.37 Td [(This) -4870.000( ) (is) -4870.000( ) (justified-baseline) -4870.000( ) (text.) ] TJ ET
-	//
-	// We don't need to look for exact positioning, we just need to look for the
-	// indication that each word is positioned.
 	w := &bytes.Buffer{}
 	if err := pdf.Output(w); err != nil {
-		t.Errorf("unexpected err: %s", err)
+		t.Errorf("could not write PDF: %v", err)
 	}
 
-	// I'm having a weird problem where the pattern matches through:
-	//
-	//   BT 0 Tw 31.19 796.37 Td [(
-	//
-	// ... but as soon as I include "This", it fails to match! Fortunately, the
-	// leading instructions ("BT 0 Tw...") are unique enough to let us know
-	// we're getting to the correct codepath.
-
-	// pattern := regexp.MustCompile(`\nBT 0 Tw .* Td \[\(This\) .* \(is\) .* \(justified-baseline\) .* \(text.\) ] TJ ET\n`)
-	pattern := regexp.MustCompile(`\bBT 0 Tw .* Td \[\(`)
-
-	if !pattern.MatchReader(w) {
-		t.Fatal("unable to find justified text in PDF output")
+	const pattern = "BT 0 Tw 31.19 785.17 Td [(\x00T\x00h\x00i\x00s) -4870.000(\x00 ) (\x00i\x00s) -4870.000(\x00 ) (\x00j\x00u\x00s\x00t\x00i\x00f\x00i\x00e\x00d\x00-\x00b\x00a\x00s\x00e\x00l\x00i\x00n\x00e) -4870.000(\x00 ) (\x00t\x00e\x00x\x00t\x00.) ] TJ ET"
+	if !bytes.Contains(w.Bytes(), []byte(pattern)) {
+		t.Fatalf("could not find justified text in PDF output")
 	}
 }

looking at the produced PDF, I figured out why the "full" regexp doesn't match. The PDF actually contains: ```pdf BT 0 Tw 31.19 785.17 Td [(^@T^@h^@i^@s) -4870.000(^@ ) (^@i^@s) -4870.000(^@ ) (^@j^@u^@s^@t^@i^@f^@i^@e^@d^@-^@b^@a^@s^@e^@l^@i^@n^@e) -4870.000(^@ ) (^@t^@e^@x^@t^@.) ] TJ ET ``` so I'd suggest the following test: ```diff diff --git a/fpdf_test.go b/fpdf_test.go index 4dd037b..2d41f0e 100644 --- a/fpdf_test.go +++ b/fpdf_test.go @@ -25,7 +25,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "runtime" "testing" @@ -341,8 +340,8 @@ func TestIssue104CellFormatJustifiedVerticalAlign(t *testing.T) { pdf.SetFont("DejaVu", "", 10) pdf.CellFormat(100, 10, "This is justified-baseline text.", "", 0, "JA", false, 0, "") - if pdf.Error() != nil { - t.Fatalf("not expecting error when rendering text") + if err := pdf.Error(); err != nil { + t.Fatalf("could not render text: %v", err) } // If we didn't get justified text, the PDF output will contain: @@ -352,26 +351,13 @@ func TestIssue104CellFormatJustifiedVerticalAlign(t *testing.T) { // If it *is* justified, we'll get placement of individual words: // // BT 0 Tw 31.19 796.37 Td [(This) -4870.000( ) (is) -4870.000( ) (justified-baseline) -4870.000( ) (text.) ] TJ ET - // - // We don't need to look for exact positioning, we just need to look for the - // indication that each word is positioned. w := &bytes.Buffer{} if err := pdf.Output(w); err != nil { - t.Errorf("unexpected err: %s", err) + t.Errorf("could not write PDF: %v", err) } - // I'm having a weird problem where the pattern matches through: - // - // BT 0 Tw 31.19 796.37 Td [( - // - // ... but as soon as I include "This", it fails to match! Fortunately, the - // leading instructions ("BT 0 Tw...") are unique enough to let us know - // we're getting to the correct codepath. - - // pattern := regexp.MustCompile(`\nBT 0 Tw .* Td \[\(This\) .* \(is\) .* \(justified-baseline\) .* \(text.\) ] TJ ET\n`) - pattern := regexp.MustCompile(`\bBT 0 Tw .* Td \[\(`) - - if !pattern.MatchReader(w) { - t.Fatal("unable to find justified text in PDF output") + const pattern = "BT 0 Tw 31.19 785.17 Td [(\x00T\x00h\x00i\x00s) -4870.000(\x00 ) (\x00i\x00s) -4870.000(\x00 ) (\x00j\x00u\x00s\x00t\x00i\x00f\x00i\x00e\x00d\x00-\x00b\x00a\x00s\x00e\x00l\x00i\x00n\x00e) -4870.000(\x00 ) (\x00t\x00e\x00x\x00t\x00.) ] TJ ET" + if !bytes.Contains(w.Bytes(), []byte(pattern)) { + t.Fatalf("could not find justified text in PDF output") } } ```

JaredReisinger added 1 commit

2025-10-31 01:13:30 +01:00

fix typo in comment 14ee037b33

sbinet reviewed

2025-10-31 09:09:29 +01:00

fpdf_test.go Outdated

					
				@ -329,0 +349,4 @@

					//

					//   BT 31.19 785.17 Td (This is justified-baseline text.)Tj ET

					//

					// If is *is* justified, we'll get placement of individual words:

there were actually 2 typos :)

If it *is* justified, we'll get placement of individual words:

(s/If is/If it/)

there were actually 2 typos :) ``` If it *is* justified, we'll get placement of individual words: ``` (`s/If is/If it/`)

🤦‍♂️... fixed!

JaredReisinger added 1 commit

2025-12-09 19:23:26 +01:00

one more comment typo fix 8360f9f658

r3-gabriel commented

2026-05-23 15:19:45 +02:00

First-time contributor

I also ran into this issue today (justify is ignored if any vertical alignment is set).

From the history of this pull request, it looks like the fix is simple; but it has not been applied because the test function was not completed.

Is there something left to be done on the test? Or is the fix not as straightforward as it seems?

I also ran into this issue today (justify is ignored if any vertical alignment is set). From the history of this pull request, it looks like the fix is simple; but it has not been applied because the test function was not completed. Is there something left to be done on the test? Or is the fix not as straightforward as it seems?

sbinet merged commit 49f5a634f6 into main

2026-05-27 13:41:39 +02:00

sbinet referenced this pull request from a commit

2026-05-27 13:41:45 +02:00

fpdf: handle other alignments alongside "J" in UTF8 CellFormat

sbinet commented

2026-05-27 13:42:46 +02:00

it was just a case of me dropping the ball.

apologies to @JaredReisinger .

it was just a case of me dropping the ball. apologies to @JaredReisinger .

r3-gabriel commented

2026-05-27 14:53:12 +02:00