Improve insertion point behaviour #3092
Conversation
On missing an insertion point fail with a user facing error and avoid panicing due to an invalid slice on an empty array.
Small improvements on creating insertion points. Removes the need to trim the resulting file for newlines. Removes a buffer allocation to build the temporary insertion.
| for i := 0; targetScanner.Scan(); i++ { | ||
| if i > 0 { | ||
| // These writes cannot fail, they will panic if they cannot allocate. | ||
| _, _ = postInsertionContent.Write(newline) |
There was a problem hiding this comment.
Create a newline only if we are going to append more content to the file. This removes the need to trim after the buffer is finalized.
| _, _ = postInsertionContent.Write(insertedContent) | ||
| // Insert the content from the insertion point file. Handle newlines in | ||
| // a platform-agnostic manner. | ||
| insertedContentReader := strings.NewReader(insertionPointFile.GetContent()) |
There was a problem hiding this comment.
Prefer strings.NewReader over bytes.NewBufferString as the string is RO.
| // Insert the content from the insertion point file. Handle newlines in | ||
| // a platform-agnostic manner. | ||
| insertedContentReader := strings.NewReader(insertionPointFile.GetContent()) | ||
| writeWithPrefixAndLineEnding(postInsertionContent, insertedContentReader, whitespace, newline) |
There was a problem hiding this comment.
Write to the buffer instead of allocating a new slice and then writing to the buffer.
There was a problem hiding this comment.
How is this currently tested? Are you confident that the new code is being exercised?
I'm hesitant to change 3-4 year old code whose performance doesn't matter at all, that has been working, just for a perf improvement, unless we're confident that it is fully tested (maybe it is).
|
Tests are here with some benchmarks that show the improvements. #3090 improved the coverage for the error cases captured in that issue. Agree that the benchmarks don't matter for this flow but thought this simplified it and removed the workarounds for both newline printing, that caused the original panic issue, and the need to calculate a buffer size. |
Small improvements on creating insertion points. Removes the need to trim the resulting file for newlines. Removes a buffer allocation to build each insertion.