Skip to content

Expose Parser and add OpenAI example in usage guide#2

Merged
benjreinhart merged 4 commits into
benjreinhart:mainfrom
martosaur:am-expose-parser
May 20, 2026
Merged

Expose Parser and add OpenAI example in usage guide#2
benjreinhart merged 4 commits into
benjreinhart:mainfrom
martosaur:am-expose-parser

Conversation

@martosaur

Copy link
Copy Markdown
Contributor

Hi @benjreinhart and thank you for the library, it's amazing!

I kind of miss version 0.2.0 manual parsing, since it allowed me to use Req's callback streaming and, most importantly, make Req eventually return whole response body once the stream is complete. This is useful, for example, if any response steps expect a full body. One example is PostHog's LLMAnalytics Req plugin.

This PR just adds Parser.init/0 function, makes parse/0 and parse/1 public and adds an example in usage guide.

@benjreinhart

Copy link
Copy Markdown
Owner

Hey @martosaur, appreciate the PR!

I will happily add this ability back. However, I have some implementation preferences that differ from what is here.

  1. Let's do everything through the top-level module, e.g., ServerSentEvents.parse/1 and ServerSentEvents.parse/2
  2. Let's keep the parser module private with moduledoc: false
  3. Can we use Parser.new instead of Parser.init? I believe new is more idiomatic here
  4. Can we add mention of .parse to readme? It doesn't need to be a full big example, but maybe a simple one, similar to what was there for the older version of this library?

@martosaur

Copy link
Copy Markdown
Contributor Author

@benjreinhart of course! Just pushed the changes

@benjreinhart benjreinhart left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You know, i'm so sorry, but I was wrong. It doesn't make sense to me to expose a new() function to construct a struct from another module. I think your first pass here makes sense.

Sorry to thrash, but I think moving back to exposing the parser module and using that to do the lower-level parsing is the right approach.

Thanks

@martosaur

Copy link
Copy Markdown
Contributor Author

Haha it's all good, I left initial commit intact just in case. Please check now!

@martosaur martosaur requested a review from benjreinhart May 20, 2026 16:06
@moduledoc """
Low level SSE parser.

@type event :: %{

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Was this intentional? I think we can leave this here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes! Sorry should have highlighted this. It's identical to ServerSentEvents.event so I just made this module to use that one. It'll be something like that:

Image

@benjreinhart benjreinhart merged commit 34e94b5 into benjreinhart:main May 20, 2026
@martosaur martosaur deleted the am-expose-parser branch May 20, 2026 17:28
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