Skip to content

Make case sensitivity optional #282

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

Merged
merged 1 commit into from
Jun 10, 2018

Conversation

caesarxuchao
Copy link
Contributor

@codecov
Copy link

codecov bot commented Jun 8, 2018

Codecov Report

Merging #282 into master will increase coverage by 0.05%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #282      +/-   ##
==========================================
+ Coverage   81.27%   81.32%   +0.05%     
==========================================
  Files          41       41              
  Lines        4971     4979       +8     
==========================================
+ Hits         4040     4049       +9     
+ Misses        814      810       -4     
- Partials      117      120       +3
Impacted Files Coverage Δ
config.go 88.7% <100%> (+0.06%) ⬆️
iter_object.go 64.97% <42.85%> (-1.13%) ⬇️
reflect.go 92.63% <50%> (-0.92%) ⬇️
reflect_struct_decoder.go 45.88% <80%> (+0.83%) ⬆️

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 8744d7c...b92cf78. Read the comment docs.

@caesarxuchao
Copy link
Contributor Author

I tried unit tests on the master branch and many tests were broken. I suspect it's caused by dependency issues? I used "dep ensure" to download dependencies. It's much appreciated If you have some pointers on how to solve the local unit test failures.

go test ./... output: https://raw.githubusercontent.com/caesarxuchao/go/test/testlog

@sai-g
Copy link

sai-g commented Jun 9, 2018

@caesarxuchao I faced the similar issue in running tests. I used the following command to fetch test dependencies, and it worked go get -t ./...

@taowen taowen merged commit 7cceb6c into json-iterator:master Jun 10, 2018
@caesarxuchao
Copy link
Contributor Author

Thanks @sai-g. I also installed the json-iter/go at the wrong path. After moving it to $GOPATH/github.com/json-iterator/go, and run go get -t ./..., I'm able to pass the test locally.

@@ -51,7 +52,7 @@ func createStructDecoder(ctx *ctx, typ reflect2.Type, fields map[string]*structF
return &skipObjectDecoder{typ}
case 1:
for fieldName, fieldDecoder := range fields {
fieldHash := calcHash(fieldName)
fieldHash := calcHash(fieldName, ctx.caseSensitive())
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this code above also make decoding case-insensitive?

for k, binding := range bindings {
  if _, found := fields[strings.ToLower(k)]; !found {
    fields[strings.ToLower(k)] = binding.Decoder.(*structFieldDecoder)
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

A test case that exercises positive and negative cases would be good

Copy link
Contributor

Choose a reason for hiding this comment

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

Opened #285

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this code above also make decoding case-insensitive?

Yes, this was the culprit.

@nikhita nikhita mentioned this pull request Jun 11, 2018
zhenzou pushed a commit to zhenzou/jsoniter that referenced this pull request Feb 2, 2022
…sensitivity

Make case sensitivity optional
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.

5 participants