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

snake_case to camelCase fails to match `_[letter][number]` #714

Closed
brycethomas opened this Issue Mar 22, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@brycethomas

brycethomas commented Mar 22, 2017

issue in the master revision at time of writing: https://github.com/dcodeIO/protobuf.js/blob/2a74fbf551e934b3212273e6a28ad65ac4436faf/src/parse.js

camelCase(time_t0) --> timeT0 (expected)
camelCase(time_t0) --> time_t0 (actual)

Repro

From parse.js, copy this snippet into Chrome dev tools:

var camelCaseRe = /_([a-z])(?=[a-z]|$)/g;

function camelCase(str) {
    return str.substring(0,1)
         + str.substring(1)
               .replace(camelCaseRe, function($0, $1) { return $1.toUpperCase(); });
}

Then evaluate:

camelCase('time_t0')

Root Cause

The issue is that the second regex capture group doesn't include numbers; note that the current regex works fine where two or more letters precede the number:

camelCase('time_t0') --> time_t0 (incorrect)
camelCase('time_tt0') --> timeTt0 (correct)

Not sure why the current regex checks two characters after the underscore—could it perhaps be reworked to only check the one? cf. Google's C++ implementation, which simply checks that the letter immediately following the underscore is in range a-z: https://github.com/google/protobuf/blob/7f3e23707122f31ebfa58b7280bd56cbe77cb44e/src/google/protobuf/util/field_mask_util.cc#L63

dcodeIO added a commit that referenced this issue Mar 22, 2017

@dcodeIO dcodeIO added the bug label Mar 22, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Mar 22, 2017

Should be fixed in master. Feel free to reopen if there are any remaining issues!

@dcodeIO dcodeIO closed this Mar 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment