-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Jetty 12 : XmlAppendable use Charset, not String #8609
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to collapse these constructors into 1
} | ||
|
||
public XmlAppendable(Appendable out) throws IOException | ||
{ | ||
this(out, 2); | ||
} | ||
|
||
public XmlAppendable(Appendable out, String encoding) throws IOException | ||
public XmlAppendable(Appendable out, Charset charset) throws IOException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constructor is not used outside of this class, can probably be removed.
{ | ||
this(out, 2, encoding); | ||
this(out, 2, charset); | ||
} | ||
|
||
public XmlAppendable(Appendable out, int indent) throws IOException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constructor is not used outside of this class, can probably be removed.
} | ||
|
||
public XmlAppendable(Appendable out, int indent, String encoding) throws IOException | ||
public XmlAppendable(Appendable out, int indent, Charset charset) throws IOException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constructor is not used outside of this class, can probably be removed/collapsed into the constructors that are used.
@@ -30,31 +33,31 @@ | |||
private final Stack<String> _tags = new Stack<>(); | |||
private String _space = ""; | |||
|
|||
public XmlAppendable(OutputStream out, String encoding) throws IOException | |||
public XmlAppendable(OutputStream out, Charset charset) throws IOException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constructor is never used with a Charset that isn't UTF-8, should probably just remove this constructor too and have only 1 constructor that has only 1 parameter OutputStream
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM apart the possibly 2 unnecessary imports.
@@ -16,6 +16,7 @@ | |||
import java.io.FileNotFoundException; | |||
import java.io.IOException; | |||
import java.io.OutputStream; | |||
import java.nio.charset.StandardCharsets; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary import?
@@ -16,6 +16,7 @@ | |||
import java.io.FileNotFoundException; | |||
import java.io.IOException; | |||
import java.io.OutputStream; | |||
import java.nio.charset.StandardCharsets; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary import?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retrospective +1
Change to have XmlAppendable use Charset not String to create outputstream and whatnot.
Taken from PR #8597