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

Initial proposal for $C to enhance character literal usage #698 #700

Open
wants to merge 1 commit into
base: master
from

Conversation

@galderz
Copy link
Contributor

commented Jan 28, 2019

This is an initial proposal to provide an easier way to deal with character literals.

The majority of characters can currently be dealt with using '$L'. However, special characters such as ' or \ (amongst others), would require special handling by the users.

In essence, a user would need to define a method such as com.squareup.javapoet.Util.characterLiteralWithoutSingleQuotes() and pass characters through that method to get it printed as expected.

Rather than putting the burden on the user, @bjansen has proposed in issue #698 to add support for single-quoted character literals via $C. I also think this is a good enhancement.

As an initial proposal, this PR should not be considered as the final product. Here's a list of pending tasks:

  • Documentation
  • Further test cases should be added to verify its robustness.
  • singleQuoteCharacterInLiteral should probably be removed since it's likely covered already by other tests. I just added this test to compare existing alternatives for handling special characters.
  • Convert AnnotationSpec.Builder.addMemberForValue to use $C instead of $L
@galderz galderz referenced this pull request Jan 28, 2019

@galderz galderz force-pushed the galderz:t_character_literal_proposal_698 branch 2 times, most recently from 52f4041 to 1c18952 Jan 28, 2019

@kenzierocks
Copy link
Contributor

left a comment

Not an official mantainer, but I saw a couple things that could be improved.

char c = '\'';
CodeBlock block = CodeBlock.builder().add("$C", c).build();
assertThat(block.toString()).isEqualTo("'\\''");
}

This comment has been minimized.

Copy link
@kenzierocks

kenzierocks Jan 28, 2019

Contributor

Perhaps add tests for other escaped literals?

This comment has been minimized.

Copy link
@galderz

galderz Jan 29, 2019

Author Contributor

Yeah, as mentioned in the description, I was planning to add more tests :). I've just added some more tests for other escaped character literals and squashed the commits.

@@ -360,6 +363,10 @@ private TypeName argToType(Object o) {
throw new IllegalArgumentException("expected type but was " + o);
}

private Character argToChar(Object o) {
return o != null ? ((char) o) : null;

This comment has been minimized.

Copy link
@kenzierocks

kenzierocks Jan 28, 2019

Contributor

(Apparently GitHub lost this one)

Check instanceof Character and throw IllegalArgumentException like above?

This comment has been minimized.

Copy link
@galderz

galderz Jan 29, 2019

Author Contributor

Good point, should be fixed now.

Initial proposal for $C to enhance character literal usage #698
* Added ascii, unicode and other escaped character tests.

@galderz galderz force-pushed the galderz:t_character_literal_proposal_698 branch from 1c18952 to 51f5347 Jan 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.