Add Document Builder type #13
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a-h
reviewed
Aug 4, 2021
DocumentBuilder uses the builder pattern and Gemini's line-type specification to pragmatically build gemini files line-by-line.
aThorp96
changed the title
aThorp96
commented
Aug 8, 2021
document.go
Outdated
Comment on lines
20
to
21
| builder := new(strings.Builder) | ||
| return DocumentBuilder{"", builder, ""} |
There was a problem hiding this comment.
This may look a bit weird but this was my solution to avoid copying the builder. If there is an easier-to-read solution I'd be happy to integrate it.
aThorp96
commented
Aug 8, 2021
| "bytes" | ||
| "strings" | ||
|
|
||
| "github.com/pkg/errors" |
There was a problem hiding this comment.
See commit message for dependency discussion
aThorp96
commented
Aug 8, 2021
document.go
Outdated
Comment on lines
140
to
169
| // Build builds the document into a serialized byte slice. | ||
| func (doc *DocumentBuilder) Build() ([]byte, error) { | ||
| buf := bytes.Buffer{} | ||
|
|
||
| // Write header | ||
| _, err := buf.WriteString(doc.header) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "Error building document header") | ||
| } | ||
| if !strings.HasSuffix(doc.header, "\n") { | ||
| _, err = buf.WriteString("\n") | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "Error building document header") | ||
| } | ||
| } | ||
|
|
||
| // Write body | ||
| _, err = buf.WriteString(doc.body.String()) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "Error building document body") | ||
| } | ||
|
|
||
| // Write footer | ||
| _, err = buf.WriteString(doc.footer) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "Error building document footer") | ||
| } | ||
|
|
||
| return buf.Bytes(), nil | ||
| } |
There was a problem hiding this comment.
There is a lot going on here, compared to the previous iteration, but not as much as it looks. I believe we need to ensure there are newlines in between the header and body, which makes this a bit longer than it otherwise would. Also, this error handling feels a bit more verbose than normal. I'm a bit rusty with my Go though so perhaps there is a more concise way to handle these.
- Use a string builder instead of string to minimize allocations - Use proper doc-comment format With the use of stringBuilder, errors have been introduced into the package. I added the dependency `github.com/pkg/errors` to help handle them, but if we'd prefer to stick with the standard library I think that'd be fair. My thinking was the circumstances that caused strings.Builder.WriteString() to error would warrent a stack trace and metadata, something which is more pragmatically done using `github.com/pkg/errors`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello! I implemented these features when using your library but I think they might serve better purpose as part of the library.
The two features are
2. A RequireInputHandler which allows the user to provide a custom prompt (META) on the response.Moving to a separate PR per request