RFR: JDK-8261665: Clean up naming of StringContent and FixedStringContent

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8261665: Clean up naming of StringContent and FixedStringContent

Jonathan Gibbons-2
Relatively simple update to clean up some of the old naming in the `html.markup` world.

`FixedStringContent` is renamed to just `Text` and a new factory method `Text.of(CharSequence)` introduced.

`StringContent` is renamed to `TextBuilder`.

`StringContent` was the original type and is widely used; `FixedStringContent` was added much later, along with `Contents` to support shared reusable tree nodes.  But most uses of `StringContent` do not need mutability, and most can be replaced with the lighter-weight `Text` class.  This change is done, just leaving `textBuilder` to be used in `ContentBuilder` and `HtmlTree` which support the `add(CharSequence)` method.

-------------

Commit messages:
 - JDK-8261665: Clean up naming of StringContent and FixedStringContent

Changes: https://git.openjdk.java.net/jdk/pull/2556/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2556&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261665
  Stats: 584 lines in 41 files changed: 198 ins; 207 del; 179 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2556.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2556/head:pull/2556

PR: https://git.openjdk.java.net/jdk/pull/2556
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261665: Clean up naming of StringContent and FixedStringContent

Hannes Wallnöfer
On Fri, 12 Feb 2021 19:51:34 GMT, Jonathan Gibbons <[hidden email]> wrote:

> Relatively simple update to clean up some of the old naming in the `html.markup` world.
>
> `FixedStringContent` is renamed to just `Text` and a new factory method `Text.of(CharSequence)` introduced.
>
> `StringContent` is renamed to `TextBuilder`.
>
> `StringContent` was the original type and is widely used; `FixedStringContent` was added much later, along with `Contents` to support shared reusable tree nodes.  But most uses of `StringContent` do not need mutability, and most can be replaced with the lighter-weight `Text` class.  This change is done, just leaving `textBuilder` to be used in `ContentBuilder` and `HtmlTree` which support the `add(CharSequence)` method.

Nice!

I guess it could be questioned whether the `TextBuilder` class is still needed, since its only purpose is to have text appended to it instead of adding a new `Text` object to a content list in the `add(CharSequence)` methods of `ContentBuilder` and `HtmlTree`.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SystemPropertiesWriter.java line 35:

> 33: import jdk.javadoc.internal.doclets.formats.html.markup.HtmlTree;
> 34: import jdk.javadoc.internal.doclets.formats.html.Navigation.PageMode;
> 35: import jdk.javadoc.internal.doclets.formats.html.markup.TextBuilder;

It looks like you're not using TextBuilder in this class after all.

-------------

Marked as reviewed by hannesw (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2556
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261665: Clean up naming of StringContent and FixedStringContent

Jonathan Gibbons

On 2/22/21 7:24 AM, Hannes Wallnöfer wrote:

> On Fri, 12 Feb 2021 19:51:34 GMT, Jonathan Gibbons <[hidden email]> wrote:
>
>> Relatively simple update to clean up some of the old naming in the `html.markup` world.
>>
>> `FixedStringContent` is renamed to just `Text` and a new factory method `Text.of(CharSequence)` introduced.
>>
>> `StringContent` is renamed to `TextBuilder`.
>>
>> `StringContent` was the original type and is widely used; `FixedStringContent` was added much later, along with `Contents` to support shared reusable tree nodes.  But most uses of `StringContent` do not need mutability, and most can be replaced with the lighter-weight `Text` class.  This change is done, just leaving `textBuilder` to be used in `ContentBuilder` and `HtmlTree` which support the `add(CharSequence)` method.
> Nice!
>
> I guess it could be questioned whether the `TextBuilder` class is still needed, since its only purpose is to have text appended to it instead of adding a new `Text` object to a content list in the `add(CharSequence)` methods of `ContentBuilder` and `HtmlTree`.

Yes, agreed. In the original design work, we thought that appending text
would be more common, but this work shows that for the most part, we
"append" text to the overall document by wrapping it in some enclosing
element. I think it would be a good follow-up to analyze use of
TextBuilder to see how useful it remains.

-- Jon


>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SystemPropertiesWriter.java line 35:
>
>> 33: import jdk.javadoc.internal.doclets.formats.html.markup.HtmlTree;
>> 34: import jdk.javadoc.internal.doclets.formats.html.Navigation.PageMode;
>> 35: import jdk.javadoc.internal.doclets.formats.html.markup.TextBuilder;
> It looks like you're not using TextBuilder in this class after all.
>
> -------------
>
> Marked as reviewed by hannesw (Reviewer).
>
> PR: https://git.openjdk.java.net/jdk/pull/2556
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261665: Clean up naming of StringContent and FixedStringContent [v2]

Jonathan Gibbons-2
In reply to this post by Jonathan Gibbons-2
> Relatively simple update to clean up some of the old naming in the `html.markup` world.
>
> `FixedStringContent` is renamed to just `Text` and a new factory method `Text.of(CharSequence)` introduced.
>
> `StringContent` is renamed to `TextBuilder`.
>
> `StringContent` was the original type and is widely used; `FixedStringContent` was added much later, along with `Contents` to support shared reusable tree nodes.  But most uses of `StringContent` do not need mutability, and most can be replaced with the lighter-weight `Text` class.  This change is done, just leaving `textBuilder` to be used in `ContentBuilder` and `HtmlTree` which support the `add(CharSequence)` method.

Jonathan Gibbons has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:

 - Merge remote-tracking branch 'upstream/master' into text-rename
   
   # Conflicts:
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassUseWriter.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Contents.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ModuleWriterImpl.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageUseWriter.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageWriterImpl.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java
   # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TreeWriter.java
 - JDK-8261665: Clean up naming of StringContent and FixedStringContent

-------------

Changes: https://git.openjdk.java.net/jdk/pull/2556/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2556&range=01
  Stats: 574 lines in 41 files changed: 200 ins; 206 del; 168 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2556.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2556/head:pull/2556

PR: https://git.openjdk.java.net/jdk/pull/2556
Reply | Threaded
Open this post in threaded view
|

Integrated: JDK-8261665: Clean up naming of StringContent and FixedStringContent

Jonathan Gibbons-2
In reply to this post by Jonathan Gibbons-2
On Fri, 12 Feb 2021 19:51:34 GMT, Jonathan Gibbons <[hidden email]> wrote:

> Relatively simple update to clean up some of the old naming in the `html.markup` world.
>
> `FixedStringContent` is renamed to just `Text` and a new factory method `Text.of(CharSequence)` introduced.
>
> `StringContent` is renamed to `TextBuilder`.
>
> `StringContent` was the original type and is widely used; `FixedStringContent` was added much later, along with `Contents` to support shared reusable tree nodes.  But most uses of `StringContent` do not need mutability, and most can be replaced with the lighter-weight `Text` class.  This change is done, just leaving `textBuilder` to be used in `ContentBuilder` and `HtmlTree` which support the `add(CharSequence)` method.

This pull request has now been integrated.

Changeset: 3132b1c4
Author:    Jonathan Gibbons <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/3132b1c4
Stats:     574 lines in 41 files changed: 200 ins; 206 del; 168 mod

8261665: Clean up naming of StringContent and FixedStringContent

Reviewed-by: hannesw

-------------

PR: https://git.openjdk.java.net/jdk/pull/2556