EventSource icon indicating copy to clipboard operation
EventSource copied to clipboard

Update EventParser.swift to Support CR LF

Open Lakr233 opened this issue 8 months ago • 6 comments
trafficstars

PR for #27

Lakr233 avatar Feb 27 '25 03:02 Lakr233

Would you be able to add tests to verify your solution?

Recouse avatar Feb 27 '25 22:02 Recouse

Hi. I have updated the code to support mixed CRLF and LFLF with tests. But I don't know if we should ever support mixed contents.

Lakr233 avatar Feb 28 '25 03:02 Lakr233

I don't think mixed content will ever be a case.

I'll check the PR and merge it ASAP. Thank you!

Recouse avatar Feb 28 '25 16:02 Recouse

Would you like to implement the optimization on your side? Kinda busy right now :P

Lakr233 avatar Mar 02 '25 02:03 Lakr233

Sure, I'll give it a shot, and maybe I'll open a new PR

Recouse avatar Mar 02 '25 11:03 Recouse

@Recouse think this PR will come in anytime soon? SSE spec says any of the 3 are supported "Lines must be separated by either a U+000D CARRIAGE RETURN U+000A LINE FEED (CRLF) character pair, a single U+000A LINE FEED (LF) character, or a single U+000D CARRIAGE RETURN (CR) character." so we are choking on spec compliant servers. Thanks! Have had terrific luck with your framework otherwise and appreciate you building it!

stallent avatar Mar 14 '25 14:03 stallent

Maybe you can change Parser implementation from Data to String. Use CharacterSet.whitespacesAndNewlines to avoid handle those.

wolfcon avatar May 13 '25 03:05 wolfcon

I specifically used Data to avoid unnecessary conversions between Data and String. I have some drafts that add CRLF support, I haven't had the time to focus on them full-time, but I'll probably open a new PR as soon as it's ready.

Recouse avatar May 13 '25 07:05 Recouse

Maybe you can change Parser implementation from Data to String.

Use CharacterSet.whitespacesAndNewlines to avoid handle those.

You can't. Some of the data starts with white spaces. It was fixed earlier.

Lakr233 avatar May 13 '25 15:05 Lakr233

Maybe you can change Parser implementation from Data to String. Use CharacterSet.whitespacesAndNewlines to avoid handle those.

You can't. Some of the data starts with white spaces. It was fixed earlier.

CharacterSet.newlines ... I have a mistake.

wolfcon avatar May 16 '25 02:05 wolfcon

@Recouse I have replace all function with O(n) cplx so would you please update with me?

Lakr233 avatar May 27 '25 06:05 Lakr233

I've been busy and haven't had time to review it yet. I'll take a look in the coming days, thanks for your contribution.

Recouse avatar Jul 05 '25 10:07 Recouse

Everything looks good to me. There's just one small improvement I'd like to suggest, removing the Data → String → Data conversions in the cleanMessageData(_:) method. I've made the changes and tested them. If you add me as a contributor in your repo, I could push the updates to this PR branch.

Recouse avatar Jul 06 '25 20:07 Recouse

Everything looks good to me. There's just one small improvement I'd like to suggest, removing the Data → String → Data conversions in the cleanMessageData(_:) method. I've made the changes and tested them. If you add me as a contributor in your repo, I could push the updates to this PR branch.

Added.

Lakr233 avatar Jul 06 '25 23:07 Lakr233

Oops, I accidentally pushed everything to my repo instead of yours.

Recouse avatar Jul 07 '25 06:07 Recouse