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

Add text method to element #8

Merged
merged 13 commits into from
Nov 16, 2021
Merged

Conversation

sujathakumar
Copy link
Contributor

No description provided.

result.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@sujathakumar sujathakumar requested a review from joente November 15, 2021 20:11
Copy link
Member

@joente joente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request @iamneal!

Could you take a look at the test results? There are some lint issues with the pr. Otherwise it looks fine.

Test CI and Test Lint

@iamneal
Copy link
Contributor

iamneal commented Nov 15, 2021

yeah I'll take a look

@sujathakumar sujathakumar requested a review from joente November 15, 2021 23:20
@joente joente changed the base branch from master to dev November 16, 2021 07:24
@joente joente merged commit 04edcc3 into cesbit:dev Nov 16, 2021
@joente joente mentioned this pull request Nov 16, 2021
@joente
Copy link
Member

joente commented Nov 16, 2021

@iamneal ,

Maybe we could add a GetNodeString function to Result for retrieving a part of the string?

Something like:

// GetNodeString
func (r *Result) GetNodeString(n *Node) string {
	return r.expression[n.start:n.end]
}

This would require to parse the Node to the Result, but it keeps the Node struct light and does not unnecessary create a lot of new strings. (only when truly required).

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.

3 participants