You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Gary Gregory <ga...@gmail.com> on 2015/10/06 16:31:48 UTC
Fwd: [1/2] logging-log4j2 git commit: LOG4J2-783 code cleanup: removed
old/misleading reference to UTF-8 charset
Not quite right IMO, should be if charset == null then charset = Charset.defaultCharset ().
Gary
-------- Original message --------
From: rpopma@apache.org
Date: 10/06/2015 06:34 (GMT-08:00)
To: commits@logging.apache.org
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 <rp...@apache.org>
Authored: Tue Oct 6 15:21:47 2015 +0200
Committer: rpopma <rp...@apache.org>
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;
}
Re: Fwd: [1/2] logging-log4j2 git commit: LOG4J2-783 code cleanup:
removed old/misleading reference to UTF-8 charset
Posted by Remko Popma <re...@gmail.com>.
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 <ga...@gmail.com> wrote:
> Not quite right IMO, should be if charset == null then charset =
> Charset.defaultCharset ().
>
> Gary
>
>
> -------- Original message --------
> From: rpopma@apache.org
> Date: 10/06/2015 06:34 (GMT-08:00)
> To: commits@logging.apache.org
> 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 <rp...@apache.org>
> Authored: Tue Oct 6 15:21:47 2015 +0200
> Committer: rpopma <rp...@apache.org>
> 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;
> }
>
>
>