Skip to content

Simplify parser#10

Merged
aaronkison merged 2 commits intomasterfrom
simplify-parser
Feb 4, 2025
Merged

Simplify parser#10
aaronkison merged 2 commits intomasterfrom
simplify-parser

Conversation

@aaronkison
Copy link
Copy Markdown
Contributor

@aaronkison aaronkison commented Feb 2, 2025

I found the existing implementation to be too difficult to understand and reason. Hopefully this takes a giant leap into making it more sensible.

This was motivated by me initially seeing a potential problem, that is if the parser skips all empty chunks then it'll never trigger the is_last_record and potentially try to read off the edge of the stream. After rewriting it, it makes more sense as it just detects if the stream ended early (complete chunks end with a RS, and .split(RS, -1) adds a '' at the end of the records to ensure it misses that condition. It's structurally backwards, but functions correctly.

Essentially, it performs in two modes:

  1. When the record is wrapped on either side by an RS it treats it as a complete record
  2. When the record doesn't have an RS after it, it first checks if its valid json. If its valid json, it returns it, otherwise it stores it in a buffer and waits for the next parse to add it to the buffer and continue

I decided to make those modes more explicit in the logic. As the modes only apply to the last record in the list, we first process all the records normally first and only treat the final one specially.

@aaronkison aaronkison changed the base branch from master to update-ruby-to-v3 February 2, 2025 23:00
@aaronkison aaronkison force-pushed the simplify-parser branch 8 times, most recently from 41b9ecf to 4adc120 Compare February 3, 2025 00:31
Base automatically changed from update-ruby-to-v3 to master February 4, 2025 00:37
@aaronkison aaronkison requested a review from nickburgin February 4, 2025 00:37
Copy link
Copy Markdown
Member

@nickburgin nickburgin left a comment

Choose a reason for hiding this comment

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

looks good to me

@aaronkison aaronkison merged commit 9bccd7c into master Feb 4, 2025
@aaronkison aaronkison deleted the simplify-parser branch February 4, 2025 20:34
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.

4 participants