Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add json ASCII string render #1358

Merged
merged 13 commits into from Jul 3, 2018
Merged

add json ASCII string render #1358

merged 13 commits into from Jul 3, 2018

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.

type SecureJSONPrefix string

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.

None yet

3 participants