ring-clojure / ring-json

Ring middleware for handling JSON
313 stars 47 forks source link

Add stream support for response body - StreamableResponseBody impl, attempt two #61

Closed pete23 closed 5 years ago

pete23 commented 5 years ago

Apologies for the extra PR. I got stuck in git hell and am too tired to fix it, given that this PR should do the trick. It's the exact same as previous but with your request to make streamable->string private.

weavejester commented 5 years ago

I think this all seems fine. Thanks for your work on this. If you have time, could you align the let clauses? If not, I'll handle that in a separate commit.

pete23 commented 5 years ago

Align the let clauses? My last patch did that I thought. Or do you mean something other than indent?

weavejester commented 5 years ago

I mean lining up the keys and values. e.g.

    (let [generator (if (:stream? options)
                      ->JsonStreamingResponseBody
                      json/generate-string)
          options   (dissoc options :stream?)
          json-resp (update-in response [:body] generator options)]
    (let [handler  (constantly {:status 200 :headers {} :body {:foo "bar" :baz "quz"}})
          response ((wrap-json-response handler {:stream? true :pretty true}) {})
          body     (streamable->string (:body response))]

But I can tweak the spacing myself in a formatting commit if you want.

pete23 commented 5 years ago

Ah sorry yes if you could fix it that might be easier - it's not just me, there seems to be inconsistency between the RHS clause alignment style and the single-space between LHS and RHS clauses style throughout.

pete23 commented 5 years ago

Awesome! Thanks.