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

FormattingStyle follow-up #2327

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions gson/src/main/java/com/google/gson/FormattingStyle.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@

package com.google.gson;

import com.google.gson.stream.JsonWriter;
import java.util.Objects;

/**
* A class used to control what the serialization looks like.
* A class used to control what the serialization output looks like.
*
* <p>It currently defines the kind of newline to use, and the indent, but
* might add more in the future.</p>
*
* @see GsonBuilder#setPrettyPrinting(FormattingStyle)
* @see JsonWriter#setFormattingStyle(FormattingStyle)
* @see <a href="https://en.wikipedia.org/wiki/Newline">Wikipedia Newline article</a>
*
* @since $next-version$
Expand All @@ -32,7 +35,11 @@ public class FormattingStyle {
private final String newline;
private final String indent;

static public final FormattingStyle DEFAULT =
/**
* The default pretty printing formatting style using {@code "\n"} as
* newline and two spaces as indent.
*/
public static final FormattingStyle DEFAULT =
new FormattingStyle("\n", " ");

private FormattingStyle(String newline, String indent) {
Expand All @@ -44,35 +51,35 @@ private FormattingStyle(String newline, String indent) {
}
if (!indent.matches("[ \t]*")) {
throw new IllegalArgumentException(
"Only combinations of spaces and tabs allowed in indent.");
"Only combinations of spaces and tabs are allowed in indent.");
}
this.newline = newline;
this.indent = indent;
}

/**
* Creates a {@link FormattingStyle} with the specified newline setting.
* Creates a {@code FormattingStyle} with the specified newline setting.
Copy link
Collaborator Author

@Marcono1234 Marcono1234 Feb 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have changed this to @code because this is within the documentation of FormattingStyle, so linking to itself is probably not necessary.

The (somewhat dated) "How to Write Doc Comments for the Javadoc Tool" guide also says: "Use in-line links economically"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be inclined not to do this. To quote Google's internal Best Practices document on javadoc:

Any specific type mentioned in prose should be hyperlinked every time in that documentation block. Historical advice was to link once and only once, but using {@link} every time is simpler, future-proof, and more tool-friendly. IDEs and code indexers can recognize this as a reference to a specific API and make it navigable. As an example, if that API is renamed later, the tools should know to fix this reference.

(Comment applies throughout.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point about validating the referenced name makes sense, but to me that appears to be rather a workaround for missing Javadoc functionality (e.g. @code tag which represents a reference).

Personally I still think in the HTML output the links can be distracting / confusing, giving the impression that they refer to a different, similarly named class.

I have reverted these @link changes now though.

*
* <p>It can be used to accommodate certain OS convention, for example
* hardcode {@code "\r"} for Linux and macos, {@code "\r\n"} for Windows, or
* hardcode {@code "\n"} for Linux and macOS, {@code "\r\n"} for Windows, or
* call {@link java.lang.System#lineSeparator()} to match the current OS.</p>
*
* <p>Only combinations of {@code \n} and {@code \r} are allowed.</p>
*
* @param newline the string value that will be used as newline.
* @return a newly created {@link FormattingStyle}
* @return a newly created {@code FormattingStyle}
*/
public FormattingStyle withNewline(String newline) {
return new FormattingStyle(newline, this.indent);
}

/**
* Creates a {@link FormattingStyle} with the specified indent string.
* Creates a {@code FormattingStyle} with the specified indent string.
*
* <p>Only combinations of spaces and tabs allowed in indent.</p>
*
* @param indent the string value that will be used as indent.
* @return a newly created {@link FormattingStyle}
* @return a newly created {@code FormattingStyle}
*/
public FormattingStyle withIndent(String indent) {
return new FormattingStyle(this.newline, indent);
Expand Down
8 changes: 6 additions & 2 deletions gson/src/main/java/com/google/gson/GsonBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -481,18 +481,22 @@ public GsonBuilder addDeserializationExclusionStrategy(ExclusionStrategy strateg
* Configures Gson to output JSON that fits in a page for pretty printing. This option only
* affects JSON serialization.
*
* <p>This is a convenience method which simply calls {@link #setPrettyPrinting(FormattingStyle)}
* with {@link FormattingStyle#DEFAULT}.
*
* @return a reference to this {@code GsonBuilder} object to fulfill the "Builder" pattern
*/
public GsonBuilder setPrettyPrinting() {
return setPrettyPrinting(FormattingStyle.DEFAULT);
}

/**
* Configures Gson to output JSON that uses a certain kind of formatting stile (for example newline and indent).
* This option only affects JSON serialization.
* Configures Gson to output JSON that uses a certain kind of formatting style (for example newline and indent).
* This option only affects JSON serialization. By default Gson produces compact JSON output without any formatting.
*
* <p>Has no effect if the serialized format is a single line.</p>
*
* @param formattingStyle the formatting style to use, {@code null} for compact JSON output.
* @return a reference to this {@code GsonBuilder} object to fulfill the "Builder" pattern
* @since $next-version$
*/
Expand Down
9 changes: 5 additions & 4 deletions gson/src/main/java/com/google/gson/stream/JsonWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public JsonWriter(Writer out) {
* @param indent a string containing only whitespace.
*/
public final void setIndent(String indent) {
if (indent.length() == 0) {
if (indent.isEmpty()) {
setFormattingStyle(null);
} else {
setFormattingStyle(FormattingStyle.DEFAULT.withIndent(indent));
Expand All @@ -226,15 +226,15 @@ public final void setIndent(String indent) {

/**
* Sets the pretty printing style to be used in the encoded document.
* No pretty printing if null.
* No pretty printing is used if the given style is {@code null}.
Marcono1234 marked this conversation as resolved.
Show resolved Hide resolved
*
* <p>Sets the various attributes to be used in the encoded document.
* For example the indentation string to be repeated for each level of indentation.
* Or the newline style, to accommodate various OS styles.</p>
*
* <p>Has no effect if the serialized format is a single line.</p>
*
* @param formattingStyle the style used for pretty printing, no pretty printing if null.
* @param formattingStyle the style used for pretty printing, no pretty printing if {@code null}.
* @since $next-version$
*/
public final void setFormattingStyle(FormattingStyle formattingStyle) {
Expand All @@ -249,7 +249,8 @@ public final void setFormattingStyle(FormattingStyle formattingStyle) {
/**
* Returns the pretty printing style used by this writer.
*
* @return the FormattingStyle that will be used.
* @return the {@code FormattingStyle} that will be used, {@code null} if compact
* JSON output is produced.
* @since $next-version$
*/
public final FormattingStyle getFormattingStyle() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class FormattingStyleTest {
private static final String EXPECTED_CR = buildExpected("\r", " ");
private static final String EXPECTED_LF = buildExpected("\n", " ");
private static final String EXPECTED_CRLF = buildExpected("\r\n", " ");
private static final String EXPECTED_COMPACT = buildExpected("", "");

// Various valid strings that can be used for newline and indent
private static final String[] TEST_NEWLINES = {
Expand Down Expand Up @@ -89,6 +90,13 @@ public void testNewlineOs() {
assertEquals(EXPECTED_OS, json);
}

@Test
public void testCompact() {
Gson gson = new GsonBuilder().setPrettyPrinting(null).create();
String json = gson.toJson(INPUT);
assertEquals(EXPECTED_COMPACT, json);
Marcono1234 marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void testVariousCombinationsToString() {
for (String indent : TEST_INDENTS) {
Expand Down Expand Up @@ -127,7 +135,8 @@ public void testVariousCombinationsParse() {
@Test
public void testStyleValidations() {
try {
// TBD if we want to accept \u2028 and \u2029. For now we don't.
// TBD if we want to accept \u2028 and \u2029. For now we don't because JSON specification
// does not consider them to be newlines
FormattingStyle.DEFAULT.withNewline("\u2028");
fail("Gson should not accept anything but \\r and \\n for newline");
} catch (IllegalArgumentException expected) {
Expand Down
15 changes: 15 additions & 0 deletions gson/src/test/java/com/google/gson/stream/JsonWriterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -871,4 +871,19 @@ public void testSetGetFormattingStyle() throws IOException {

assertThat(jsonWriter.getFormattingStyle().getNewline()).isEqualTo(lineSeparator);
}

@Test
public void testSetGetFormattingStyle_Compact() throws IOException {
StringWriter stringWriter = new StringWriter();
JsonWriter jsonWriter = new JsonWriter(stringWriter);
jsonWriter.setFormattingStyle(null);

jsonWriter.beginArray();
jsonWriter.value(1);
jsonWriter.value(2);
jsonWriter.endArray();

assertThat(stringWriter.toString()).isEqualTo("[1,2]");
assertThat(jsonWriter.getFormattingStyle()).isNull();
}
}