Skip to content

Add support Camel Case for key to avoid crash#36

Closed
vsay01 wants to merge 1 commit into
square:masterfrom
vsay01:master
Closed

Add support Camel Case for key to avoid crash#36
vsay01 wants to merge 1 commit into
square:masterfrom
vsay01:master

Conversation

@vsay01

@vsay01 vsay01 commented Aug 31, 2017

Copy link
Copy Markdown

Hopefully it support the camel case for the key

@pforhan

pforhan commented Aug 31, 2017

Copy link
Copy Markdown
Contributor

You'd have to adjust tests, too. For what it's worth, there's already #35 that clarifies the error messages.

@vsay01

vsay01 commented Aug 31, 2017

Copy link
Copy Markdown
Author

@pforhan It seems test case illegalTokenCharactersFailFast will fail if we use Camel Case for the token. Also in #35 there is a test case putFailUppercaseNotAllowedMiddle as well.
Does the library want to support only the lowercase and _ for token?

For my case, i need to have Camel Case that's why I thought it would be useful for others as well. Would love to hear the direction from you, I can either close this PR or fix the unit test. Thanks

@vsay01

vsay01 commented Aug 31, 2017

Copy link
Copy Markdown
Author

For now I fix the unit tests in my local branch, and all the test cases are passed.
If the library want to support the Upper case then I will make the following commit so that the build pass.

   //commented out this test case to support if want to support Upper Case
   /*@Test public void illegalTokenCharactersFailFast() {
      try {
        from("blah {NoUppercaseAllowed}");
        fail("Expected IllegalArgumentException");
      } catch (IllegalArgumentException e) {
      }
    }*/

    @Test
    public void tokenCanHaveUpperCase() {
        assertThat(from("blah {UppercaseAllowed}").put("UppercaseAllowed", "Uppercase Allowed").format().toString()).isEqualTo(
                "blah Uppercase Allowed");
    }

@pforhan

pforhan commented Aug 31, 2017

Copy link
Copy Markdown
Contributor

If you look at the issue history (like #9) , there's not been a lot of desire for it, that's why I beefed up the error messages and tests to be more explicit about the expected key format.

@vsay01

vsay01 commented Aug 31, 2017

Copy link
Copy Markdown
Author

@pforhan Thanks for the information. I will close this pull request as the library won't support the Upper case as the choice of the library as mentioned in #9.
For all who need support for upper case, you can find Fixes #17 provided the fix or visit my forked repo. .
i planned to support as well:

#{token}

Thanks for the library.

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.

3 participants