Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/6312#discussion_r201838689 --- Diff: flink-core/src/main/java/org/apache/flink/configuration/description/LineBreakElement.java --- @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.configuration.description; + +/** + * Represents a line break in the {@link Description}. + */ +public class LineBreakElement implements BlockElement { + + /** + * Creates a line break in the description. + */ + public static LineBreakElement linebreak() { + return new LineBreakElement(); + } + + private LineBreakElement() { + } + + @Override + public String format(Formatter formatter) { + return formatter.format(this); --- End diff -- hmm.................................................................. Alright so this works but I still think it's kinda weird that the element calls into the formatter; this effectively gives the element control over how it is formatted, since it _could_ just return an arbitrary string. Let me sketch out an alternative, the general idea being to generalize the parent interface (DescriptionElement) to accommodate all sub-classes, and introduce an enum for categorization. With this the elements are just a container for data, and most (but not all logic unfortunately unless we go for instanceof checks) formatting logic is in the formatter. ``` public enum ElementType { TEXT, LINK, LIST, LINE_BREAK, SEQUENCE // replaces your nested Text constructor } public interface Element { String getValue() List<Element> getChildren() ElementType getType() } class HtmlFormatter implements Formatter { format(Description description) { description.getElements().stream() .forEach(this::format) .collect(Collectors.joining()) } String format(Element element) { switch (element.getType()) { case TEXT: return element.getValue() case LIST: StringBuilder sb = new StringBuilder() for (Element item : element.getChildren()) { sb.append(<whatver html/formatting prefix you use for lists>) sb.append(format(item)) sb.append(<whatver html/formatting suffix you use for lists>) } return sb.toString() case LINK: return "<a href=" + element.getValue + ">" case LINE_BREAK: reutrn "<br>" case SEQUENCE: StringBuilder sb = new StringBuilder() for (Element item : element.getChildren()) { sb.append(format(item)) } return sb.toString() } } } class Link implements Element { private final String value Link(String text) { this.value = text } getValue() { return this.value } getChildren() { return Collections.emptyList() } getType() { return LINK } } class Text implements Element { Text(String text) { this.value = text this.children = Collections.emptyList() } getValue() { return this.value } getChildren() { return Collections.emptyList } getType() { return TEXT } } class List implements Element { private final List<Element> elements List(Element ... elements) { this.elements = Arrays.asList(elements) } getValue() { return null } getChildren() { return elements } getType() { return LIST } } class LineBreak implements Element { getValue() { return null } getChildren() { return Collections.emptyList } getType() { return LINE_BREAK } } class ComboArrayWhatever implements Element { private final List<Element> elements List(Element ... elements) { this.elements = Arrays.asList(elements) } getValue() { return null } getChildren() { return Collections.emptyList } getType() { return SEQUENCE } } ```
---