Skip to content
Permalink
Browse files
8277026: Remove blank lines remaining from snippet markup
Reviewed-by: jjg
  • Loading branch information
pavelrappo committed Dec 1, 2021
1 parent 0a01baa commit da2be99cfe1121d5a59fa575ea0ac425ad4dbd0f
Showing 7 changed files with 249 additions and 11 deletions.
@@ -401,6 +401,7 @@ protected Content snippetTagOutput(Element element, SnippetTree tag, StyledText
Element e = null;
String t = null;
boolean linkEncountered = false;
boolean markupEncountered = false;
Set<String> classes = new HashSet<>();
for (Style s : styles) {
if (s instanceof Style.Name n) {
@@ -414,6 +415,8 @@ protected Content snippetTagOutput(Element element, SnippetTree tag, StyledText
// TODO: diagnostic output
}
} else if (s instanceof Style.Markup) {
markupEncountered = true;
break;
} else {
// TODO: transform this if...else into an exhaustive
// switch over the sealed Style hierarchy when "Pattern
@@ -423,7 +426,9 @@ protected Content snippetTagOutput(Element element, SnippetTree tag, StyledText
}
}
Content c;
if (linkEncountered) {
if (markupEncountered) {
return;
} else if (linkEncountered) {
assert e != null;
String line = sequence.toString();
String strippedLine = line.strip();
@@ -438,7 +443,7 @@ protected Content snippetTagOutput(Element element, SnippetTree tag, StyledText
c = new ContentBuilder(whitespace, htmlWriter.linkToContent(element, e, t, strippedLine));
// We don't care about trailing whitespace.
} else {
c = HtmlTree.SPAN(Text.of(utils.normalizeNewlines(sequence)));
c = HtmlTree.SPAN(Text.of(text));
classes.forEach(((HtmlTree) c)::addStyle);
}
code.add(c);
@@ -139,6 +139,7 @@ record OffsetAndLine(int offset, String line) { }
String rawLine = next.line();
boolean addLineTerminator = iterator.hasNext() || trailingNewline;
String line;
boolean hasMarkup = false;
markedUpLine.reset(rawLine);
if (!markedUpLine.matches()) { // (1)
line = rawLine + (addLineTerminator ? "\n" : "");
@@ -168,6 +169,7 @@ record OffsetAndLine(int offset, String line) { }
// TODO: log this with NOTICE;
line = rawLine + (addLineTerminator ? "\n" : "");
} else { // (3)
hasMarkup = true;
String payload = rawLine.substring(0, markedUpLine.end("payload"));
line = payload + (addLineTerminator ? "\n" : "");
}
@@ -185,7 +187,7 @@ record OffsetAndLine(int offset, String line) { }

thisLineTags.clear();

append(text, Set.of(), line);
append(text, line.isBlank() && hasMarkup ? Set.of(new Style.Markup()) : Set.of(), line);
// TODO: mark up trailing whitespace!
lineStart += line.length();
}
@@ -37,21 +37,21 @@ public sealed interface Style {

/**
* A style that describes a link. Characters of this style are typically
* processed by wrapping into an HTML {@code A} element pointing to the
* processed by being wrapped into an HTML {@code A} element pointing to the
* provided target.
*/
record Link(String target) implements Style { }

/**
* A named style. Characters of this style are typically processed by
* wrapping into an HTML {@code SPAN} element with the {@code class}
* being wrapped into an HTML {@code SPAN} element with the {@code class}
* attribute which is obtained from the provided name.
*/
record Name(String name) implements Style { }

/**
* A marker of belonging to markup. Characters of this style are typically
* processed by omitting from the output.
* processed by being omitted from the output.
*/
record Markup() implements Style { }
}
@@ -74,8 +74,8 @@ protected void init() {
}

/*
* For each character of this text adds the provided objects to a set of
* objects associated with that character.
* For each character of this text adds the provided styles to a set of
* styles associated with that character.
*/
public void addStyle(Set<? extends Style> additionalStyles) {
styles.add(0, length(), additionalStyles);
@@ -87,7 +87,7 @@ public int length() {

/*
* Replaces all characters of this text with the provided sequence of
* characters, each of which is associated with all the provided objects.
* characters, each of which is associated with all the provided styles.
*/
public void replace(Set<? extends Style> styles, CharSequence plaintext) {
replace(0, length(), styles, plaintext);
@@ -62,7 +62,6 @@ public void testPositiveOuterMarkup(Path base) throws Exception {
tea=black
""".formatted(markup),
"""

<span class="bold">coffee=espresso
</span>tea=black
""");
@@ -370,6 +370,239 @@ public void testStart(Path base) throws Exception {
testPositive(base, testCases);
}

/*
* These are corner cases. As such they are expected to rarely happen in
* practise. These tests merely capture what the results looked when
* the feature was integrated. This might help when refactoring
* and refreshing the feature, to better understand the impact of
* the proposed changes.
*/
@Test
public void testPositiveInlineTagMarkup_ReplaceOnBlankLine(Path base) throws Exception {
var testCases = List.of(
// the complete line is being replaced
new TestCase("one",
"""
// @start region=one @replace regex=".*" replacement="-----"
one
// @end
""",
"""
-----one
"""
),
// the contents of the line, but not the line terminator is being replaced
new TestCase("two",
"""
// @start region=two @replace regex=".+" replacement="*****"
two
// @end
""",
"""
*****two
"""
),
new TestCase(
"""
// @replace regex="duke" replacement="duchess"
""",
"""
"""
)
);
testPositive(base, testCases);
}

@Test
public void testPositiveInlineTagMarkup_BlankLinesRegionEquivalence(Path base) throws Exception {
var testCases = List.of(
new TestCase("example1",
"""
// @start region="example1"
if (v.isPresent()) {
System.out.println("v: " + v.get());
} // @end
""",
"""
if (v.isPresent()) {
System.out.println("v: " + v.get());
}"""),
new TestCase("example2",
"""
if (v.isPresent()) { // @start region="example2"
System.out.println("v: " + v.get());
} // @end
""",
"""
if (v.isPresent()) {
System.out.println("v: " + v.get());
}"""),
new TestCase("example3",
"""
// @start region="example3" :
if (v.isPresent()) {
System.out.println("v: " + v.get());
// @end :
}
""",
"""
if (v.isPresent()) {
System.out.println("v: " + v.get());
}""")
);
testPositive(base, testCases);
}

@Test
public void testPositiveInlineTagMarkup_BlankLinesEquivalence(Path base) throws Exception {
var testCases = List.of(
new TestCase(
"""
// @start region="example"
if (v.isPresent()) {
System.out.println("v: " + v.get());
}
// @end
""",
"""
if (v.isPresent()) {
System.out.println("v: " + v.get());
}
"""),
new TestCase(
"""
if (v.isPresent()) { // @start region="example"
System.out.println("v: " + v.get());
} // @end
""",
"""
if (v.isPresent()) {
System.out.println("v: " + v.get());
}
"""),
new TestCase(
"""
// @start region="example" :
if (v.isPresent()) {
System.out.println("v: " + v.get());
// @end :
}
""",
"""
if (v.isPresent()) {
System.out.println("v: " + v.get());
}
""")
);
testPositive(base, testCases);
}

@Test
public void testPositiveInlineTagMarkup_BlankLinesFromStartEnd(Path base) throws Exception {
// A markup line that contains either @start or @end is removed.
var testCases = List.of(
new TestCase("""
First line
// @start region="a"
Third line
// @end
Fifth line
""",
"""
First line
Third line
Fifth line
"""),
new TestCase("""
First line
// @start region="a"
// @start region="b"
Third line
// @end
Fifth line
// @end
""",
"""
First line
Third line
Fifth line
"""),
// note incidental whitespace removal in test cases below
new TestCase("a", """
First line
// @start region="a"
Third line
// @end
Fifth line
""",

"""
Third line
"""),
new TestCase("b", """
First line
// @start region="a"
// @start region="b"
Third line
// @end
Fifth line
// @end
""",
"""
Third line
""")
);
testPositive(base, testCases);
}

@Test
public void testPositiveInlineTagMarkup_BlankLinesFromNextLineMarkup(Path base) throws Exception {
// A markup line that refers to the next line is removed.
var testCases = List.of(
new TestCase("""
First line
// @highlight:
Third line
""",
"""
First line
<span class="bold"> Third line
</span>"""),
new TestCase("""
First line
// @link target="Object#equals(Object)":
Third line
""",
replace("""
First line
link(Third line)
""", "link\\((.+?)\\)", r -> link(true, "java.lang.Object#equals(Object)", r.group(1)))
),
new TestCase("""
First line
// @replace regex=.+ replacement="x":
Third line
""",
"""
First line
x
"""),
new TestCase("""
First line
// @start region=a:
Third line
// @end:
Fifth line
""",
"""
First line
Third line
Fifth line
""")
);
testPositive(base, testCases);
}

private static String link(boolean linkPlain,
String targetReference,
String content)
@@ -278,7 +278,6 @@ public static void printMessage(String msg) {
checkExit(Exit.OK);
checkLinks();
final var javaContent = """

System.out.println(msg);
""";
final var propertiesContent = """

1 comment on commit da2be99

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on da2be99 Dec 1, 2021

Choose a reason for hiding this comment

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

Please sign in to comment.