Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added function to get Rich Texts from blocks #171

Merged
merged 2 commits into from
Feb 15, 2024
Merged

Conversation

wulfharth7
Copy link
Contributor

@wulfharth7 wulfharth7 commented Feb 14, 2024

I've opened a PR for the #165 but the initial PR became a mess. These are my first contributions so whenever I fix something, I usually rush and the commit history gets messy because of my excitement :D

Anyways, I wanted to fix that so I've opened a new branch to create a new PR with a better history.

@jomei
Copy link
Owner

jomei commented Feb 14, 2024

@wulfharth7 build failed, please take a look

@wulfharth7
Copy link
Contributor Author

wulfharth7 commented Feb 14, 2024

@wulfharth7 build failed, please take a look

Interesting. I have the solution. So basically when I transform these functions from these

func (p *ParagraphBlock) GetRichTextString() string {
	return concatenateRichText(p.Paragraph.RichText)
}

func (h *Heading2Block) GetRichTextString() string {
	return concatenateRichText(h.Heading2.RichText)
}

to these

func (p ParagraphBlock) GetRichTextString() string {
	return concatenateRichText(p.Paragraph.RichText)
}

func (h Heading2Block) GetRichTextString() string {
	return concatenateRichText(h.Heading2.RichText)
}

It passes the build tests. Do you have any ideas why is that so? I've cloned my first commit on the first PR and tested it. Turns out it also fails the tests (even though github didn't say so).

I've checked the block_test.go on the lines 95-128. Thats the place where it gives the error for these 2 functions. I'll check further but I need your thoughts on this topic as why it happens. Or why could that happen?

I could just transform the functions, push the commit and the functions would start working without any build errors as I've shown in the screenshots on my first PR.

But before that I just wanted to ask why, do you have any clues?

I can just do the changes and push though @jomei

@jomei
Copy link
Owner

jomei commented Feb 15, 2024

@wulfharth7 To be honest there's no specific reason to use value receiver instead of pointer one..
So let's change to value receivers here and may be change to pointers everywhere in a separate PR later.

@wulfharth7
Copy link
Contributor Author

@jomei Done. This time it should pass the tests as I've checked. I'll check further for this pointer thing later when I have time.

@jomei jomei merged commit 88e7f04 into jomei:main Feb 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants