The Builder's charset field is already initialized to Charset.getDefault(), so I thought we should only overwrite it with a non-null value.
On Tue, Oct 6, 2015 at 4:31 PM, Gary Gregory <[email protected]> wrote: > Not quite right IMO, should be if charset == null then charset = > Charset.defaultCharset (). > > Gary > > > -------- Original message -------- > From: [email protected] > Date: 10/06/2015 06:34 (GMT-08:00) > To: [email protected] > Subject: [1/2] logging-log4j2 git commit: LOG4J2-783 code cleanup: removed > old/misleading reference to UTF-8 charset > > Repository: logging-log4j2 > Updated Branches: > refs/heads/master 868f15f9b -> 8b914119e > > > LOG4J2-783 code cleanup: removed old/misleading reference to UTF-8 > charset > > PatternLayout should and does use the platform default charset. The > createLayout() factory method specified UTF-8 as the default charset, > which is misleading and confusing for developers, so I removed the > default and added a comment. > > This was not a real problem because XmlConfiguration uses > PatternLayout.Builder and not the factory method, so in reality the > platform default charset was correctly being used if no charset is > specified. > > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo > Commit: > http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/93e937b6 > Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/93e937b6 > Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/93e937b6 > > Branch: refs/heads/master > Commit: 93e937b6330b11411aa7b8f61804d424c6bcc346 > Parents: 868f15f > Author: rpopma <[email protected]> > Authored: Tue Oct 6 15:21:47 2015 +0200 > Committer: rpopma <[email protected]> > Committed: Tue Oct 6 15:21:47 2015 +0200 > > ---------------------------------------------------------------------- > .../apache/logging/log4j/core/layout/PatternLayout.java | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/93e937b6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java > index 9874af0..6c57820 100644 > --- > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java > +++ > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java > @@ -238,7 +238,7 @@ public final class PatternLayout extends > AbstractStringLayout { > * @param replace > * A Regex replacement String. > * @param charset > - * The character set. > + * The character set. The platform default is used if not > specified. > * @param alwaysWriteExceptions > * If {@code "true"} (default) exceptions are always written > even if the pattern contains no exception tokens. > * @param noConsoleNoAnsi > @@ -255,7 +255,8 @@ public final class PatternLayout extends > AbstractStringLayout { > @PluginElement("PatternSelector") final PatternSelector > patternSelector, > @PluginConfiguration final Configuration config, > @PluginElement("Replace") final RegexReplacement replace, > - @PluginAttribute(value = "charset", defaultString = "UTF-8") > final Charset charset, > + // LOG4J2-783 use platform default by default, so do not > specify defaultString for charset > + @PluginAttribute(value = "charset") final Charset charset, > @PluginAttribute(value = "alwaysWriteExceptions", > defaultBoolean = true) final boolean alwaysWriteExceptions, > @PluginAttribute(value = "noConsoleNoAnsi", defaultBoolean = > false) final boolean noConsoleNoAnsi, > @PluginAttribute("header") final String header, > @@ -395,7 +396,10 @@ public final class PatternLayout extends > AbstractStringLayout { > } > > public Builder withCharset(final Charset charset) { > - this.charset = charset; > + // LOG4J2-783 if null, use platform default by default > + if (charset != null) { > + this.charset = charset; > + } > return this; > } > > >
