Skip to content

Conversation

@duguying
Copy link
Contributor

add a json render that rendering json as ASCII string

@codecov
Copy link

codecov bot commented May 16, 2018

Codecov Report

Merging #1358 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1358      +/-   ##
==========================================
+ Coverage   97.99%   98.01%   +0.02%     
==========================================
  Files          36       36              
  Lines        1841     1860      +19     
==========================================
+ Hits         1804     1823      +19     
  Misses         30       30              
  Partials        7        7
Impacted Files Coverage Δ
context.go 96.12% <100%> (+0.02%) ⬆️
render/json.go 95.45% <100%> (+1.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d17a125...33eb8b4. Read the comment docs.

@appleboy appleboy added this to the 1.3 milestone Jul 1, 2018
context_test.go Outdated
w := httptest.NewRecorder()
c, _ := CreateTestContext(w)

c.AsciiJSON(204, []string{"lang", "Go语言"})
Copy link
Member

Choose a reason for hiding this comment

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

Please change 204 to http status code in HTTP package for maintainable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which status code should i use? you mean http.StatusNoContent or using other status code

Copy link
Member

Choose a reason for hiding this comment

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

http.StatusNoContent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@appleboy
Copy link
Member

appleboy commented Jul 2, 2018

@thinkerou Please help to review this PR.


var jsonContentType = []string{"application/json; charset=utf-8"}
var jsonpContentType = []string{"application/javascript; charset=utf-8"}
var jsonAsciiContentType = []string{"application/json"}
Copy link
Member

Choose a reason for hiding this comment

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

add charset-utf-8?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, needn't. it's ascii.

render/json.go Outdated
} else {
cvt = fmt.Sprintf("\\u%04x", int64(r))
}
result = result + cvt
Copy link
Member

@thinkerou thinkerou Jul 2, 2018

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@thinkerou
Copy link
Member

@duguying If you can add some example codes and comments to README, it's very nice, thanks!

@duguying
Copy link
Contributor Author

duguying commented Jul 2, 2018

@thinkerou readme added.

README.md Outdated
"tag": "<br>",
}

// will output : {"lang":"GO\u8bed\u8a00","tag":"\u003cbr\u003e"}
Copy link
Member

Choose a reason for hiding this comment

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

indent

@thinkerou
Copy link
Member

LGTM, thanks!

@appleboy appleboy merged commit 85221af into gin-gonic:master Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants