3
25
Fork
You've already forked fpdf
10

feat: add getters #93

Merged
sbinet merged 3 commits from :main into main 2025年03月18日 10:14:12 +01:00
Contributor
Copy link

Related Issue: #91

Summary

This PR introduces getters for all trivial setters, including styleStr and familyStr, to improve state management. These getters allow users to retrieve current values before making modifications, making it easier to restore the previous state when needed.

Changes Made

Added getters for all trivial setters, including:

  • GetFontFamily()
  • GetFontStyle()
  • etc.
  • Updated documentation to reflect the new getters.
  • Added unit tests to ensure correctness.

Testing

Ran go test to verify all tests pass.
Confirmed that the new getters return the expected values.

Related Issue: #91 ### Summary This PR introduces getters for all trivial setters, including styleStr and familyStr, to improve state management. These getters allow users to retrieve current values before making modifications, making it easier to restore the previous state when needed. ### Changes Made Added getters for all trivial setters, including: - `GetFontFamily()` - `GetFontStyle()` - etc. - Updated documentation to reflect the new getters. - Added unit tests to ensure correctness. ### Testing Ran go test to verify all tests pass. Confirmed that the new getters return the expected values.
sbinet requested changes 2025年03月14日 10:59:04 +01:00
Dismissed
sbinet left a comment
Copy link

thanks for the PR.
I have a couple of nitpicks.

See below.

thanks for the PR. I have a couple of nitpicks. See below.
fpdf.go Outdated
@ -566,6 +583,11 @@ func SetDefaultCompression(compress bool) {
gl.noCompress=!compress
}
// GetCompression returns true if page compression is enabled, false otherwise.
Owner
Copy link

s/GetCompression returns true if page compression is enabled, false otherwise./GetCompression returns whether page compression is enabled./

`s/GetCompression returns true if page compression is enabled, false otherwise./GetCompression returns whether page compression is enabled./`
Coronon marked this conversation as resolved
fpdf.go Outdated
@ -1054,6 +1128,18 @@ func (f *Fpdf) SetLineCapStyle(styleStr string) {
}
}
// GetLineCapStyle returns the current line cap style.
Owner
Copy link

s/GetLineCapStyle returns the current line cap style/GetLineJoinStyle/ returns the current line join style

`s/GetLineCapStyle returns the current line cap style/GetLineJoinStyle/ returns the current line join style`
Coronon marked this conversation as resolved
fpdf.go Outdated
@ -4082,12 +4207,22 @@ func (f *Fpdf) SetCreationDate(tm time.Time) {
f.creationDate=tm
}
// GetCreationDate returns the document's internal ModDate value.
Owner
Copy link

s/GetCreationDate/GetModificationDate/

`s/GetCreationDate/GetModificationDate/`
Coronon marked this conversation as resolved
@ -0,0 +15,4 @@
*ANYSPECIAL,DIRECT,INDIRECT,ORCONSEQUENTIALDAMAGESORANYDAMAGES
*WHATSOEVERRESULTINGFROMLOSSOFUSE,DATAORPROFITS,WHETHERINAN
*ACTIONOFCONTRACT,NEGLIGENCEOROTHERTORTIOUSACTION,ARISINGOUTOF
*ORINCONNECTIONWITHTHEUSEORPERFORMANCEOFTHISSOFTWARE.
Owner
Copy link

the copyright from the original author (Kurt Jung) doesn't apply here.
we can leave it out (but keep the go-pdf above)

the copyright from the original author (Kurt Jung) doesn't apply here. we can leave it out (but keep the `go-pdf` above)
Coronon marked this conversation as resolved
@ -0,0 +44,4 @@
alpha,blendModeStr:=pdf.GetAlpha()
if!floatEqual(alpha,0.17){
t.Errorf("Expected alpha to be 0.17, got %v",alpha)
Owner
Copy link

Go style "dictates" that test string stanzas should be of the form ("got=%v, want=%v", got, want).
Could you please modify this and the other occurrences in fpdf_getter_test.go accordingly ?

e.g.

ifgot,want:=alpha,0.17;!floatEqual(got,want){t.Errorf("invalid alpha value: got=%v, want=%v",got,want)}

thanks.

Go style "dictates" that test string stanzas should be of the form `("got=%v, want=%v", got, want)`. Could you please modify this and the other occurrences in `fpdf_getter_test.go` accordingly ? _e.g._ ```go if got, want := alpha, 0.17; !floatEqual(got, want) { t.Errorf("invalid alpha value: got=%v, want=%v", got, want) } ``` thanks.
Coronon marked this conversation as resolved
@ -0,0 +150,4 @@
creationDate:=pdf.GetCreationDate()
ifcreationDate!=setDate{
Owner
Copy link

probably best to test equality with time.Time.Equal

probably best to test equality with `time.Time.Equal`
Coronon marked this conversation as resolved
@ -0,0 +298,4 @@
reader,err:=fontLoader.Open("test")
ifreader==nil{
t.Errorf("Expected reader to be not nil")
Owner
Copy link

if reader is nil, better call t.Fatalf instead.

if `reader` is `nil`, better call `t.Fatalf` instead.
Coronon marked this conversation as resolved
@ -0,0 +460,4 @@
modificationDate:=pdf.GetModificationDate()
ifmodificationDate!=setDate{
Owner
Copy link

probably best to test equality with time.Time.Equal

probably best to test equality with `time.Time.Equal`
Coronon marked this conversation as resolved
Author
Contributor
Copy link

Thank you for your feedback and for catching those issues in the documentation! With all the repetitive getters, some must have slipped through. Let me know if you spot anything else that needs tweaking! :)

Thank you for your feedback and for catching those issues in the documentation! With all the repetitive getters, some must have slipped through. Let me know if you spot anything else that needs tweaking! :)
sbinet requested changes 2025年03月18日 09:19:59 +01:00
Dismissed
sbinet left a comment
Copy link

thanks.

last round of changes, and I'll merge this in.

thanks. last round of changes, and I'll merge this in.
@ -0,0 +55,4 @@
ifgot,want:=autoPageBreak,true;got!=want{
t.Errorf("invalid autoPageBreak: got=%v, want=%v",got,want)
}
ifgot,want:=margin,float64(10);!floatEqual(got,want){
Owner
Copy link

could be written as:

ifgot,want:=margin,10.0;!floatEqual(got,want){

but ok.

could be written as: ```go if got, want := margin, 10.0; !floatEqual(got, want) { ``` but ok.
Coronon marked this conversation as resolved
@ -0,0 +351,4 @@
funcTestGetJavascript(t*testing.T){
pdf:=fpdf.New("P","mm","A4","")
pdf.SetJavascript("<script>console.log('fpdf is awesome')<script>")
Owner
Copy link

I am not very well versed in the expected syntax for the embedded javascript but, shouldn't this read:

pdf.SetJavascript("<script>console.log('fpdf is awesome')</script>")

instead ? (ie: correctly closing the script tag ?)

perhaps we could actually lift the provided javascript code in:

constwant=`<script>console.log('fpdf is awesome')</script>`pdf.SetJavacript(want)got:=pdf.GetJavascript()ifgot==nil||*got!=want{...}
I am not very well versed in the expected syntax for the embedded javascript but, shouldn't this read: ```go pdf.SetJavascript("<script>console.log('fpdf is awesome')</script>") ``` instead ? (_ie:_ correctly closing the `script` tag ?) perhaps we could actually lift the provided javascript code in: ```go const want = `<script>console.log('fpdf is awesome')</script>` pdf.SetJavacript(want) got := pdf.GetJavascript() if got == nil || *got != want { ... } ```
Author
Contributor
Copy link

You are absolutely right! I should stop working late at night ^^

You are absolutely right! I should stop working late at night ^^
Coronon marked this conversation as resolved
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
go-pdf/fpdf!93
Reference in a new issue
go-pdf/fpdf
No description provided.
Delete branch ":main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?