Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ public int hashCode() {
return result;
}

@Override
public String toString() {
return "<" + name + " " + attributes + ">";
}

public static AnsiAttributeElement bold() {
return new AnsiAttributeElement(AnsiAttributeElement.AnsiAttrType.BOLD, "b", "");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.IOException;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.Stack;

/**
Expand Down Expand Up @@ -93,12 +94,19 @@ private void stopConcealing() {
this.out = logOutput;
}

private void openTag(AnsiAttributeElement tag) throws IOException {
/**
* @return A copy of the {@link AnsiAttributeElement}s which are currently opened, in order from outermost to innermost tag.
*/
/*package*/ List<AnsiAttributeElement> getOpenTags() {
return new ArrayList<>(openTags);
}

/*package*/ void openTag(AnsiAttributeElement tag) throws IOException {
openTags.add(tag);
tag.emitOpen(emitter);
}

private void closeOpenTags(AnsiAttrType until) throws IOException {
/*package*/ void closeOpenTags(AnsiAttrType until) throws IOException {
while (!openTags.isEmpty()) {
int index = openTags.size() - 1;
if (until != null && openTags.get(index).ansiAttrType == until)
Expand Down
59 changes: 48 additions & 11 deletions src/main/java/hudson/plugins/ansicolor/ColorConsoleAnnotator.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@
import hudson.model.Queue;
import hudson.model.Run;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nonnull;
import jenkins.model.Jenkins;
import org.apache.commons.io.output.CountingOutputStream;
import org.apache.commons.io.output.NullOutputStream;
Expand All @@ -51,17 +54,24 @@ final class ColorConsoleAnnotator extends ConsoleAnnotator<Object> {

private final String colorMapName;
private final String charset;
private final @Nonnull List<AnsiAttributeElement> openTags;

ColorConsoleAnnotator(String colorMapName, String charset) {
ColorConsoleAnnotator(String colorMapName, String charset, List<AnsiAttributeElement> openTags) {
this.colorMapName = colorMapName;
this.charset = charset;
LOGGER.fine("creating annotator with colorMapName=" + colorMapName + " charset=" + charset);
this.openTags = openTags;
LOGGER.log(Level.FINE, "creating annotator with colorMapName={0} charset={1}, openTags={2}", new Object[] { colorMapName, charset, openTags });
}

ColorConsoleAnnotator(String colorMapName, String charset) {
this(colorMapName, charset, Collections.emptyList());
}

@Override
public ConsoleAnnotator<Object> annotate(Object context, MarkupText text) {
String s = text.getText();
if (s.indexOf('\u001B') != -1) {
List<AnsiAttributeElement> nextOpenTags = openTags;
if (s.indexOf('\u001B') != -1 || !openTags.isEmpty()) {
AnsiColorMap colorMap = Jenkins.get().getDescriptorByType(AnsiColorBuildWrapper.DescriptorImpl.class).getColorMap(colorMapName);
CountingOutputStream outgoing = new CountingOutputStream(new NullOutputStream());
class EmitterImpl implements AnsiAttributeElement.Emitter {
Expand All @@ -70,40 +80,67 @@ class EmitterImpl implements AnsiAttributeElement.Emitter {
int lastPoint = -1; // multiple HTML tags may be emitted for one control sequence
@Override
public void emitHtml(String html) {
LOGGER.finest("emitting " + html + " @" + incoming.getCount());
LOGGER.log(Level.FINEST, "emitting {0} @{1}", new Object[] { html, incoming.getCount() });
text.addMarkup(incoming.getCount(), html);
if (incoming.getCount() != lastPoint) {
lastPoint = incoming.getCount();
int hide = incoming.getCount() - outgoing.getCount() - adjustment;
LOGGER.finest("hiding " + hide + " @" + (outgoing.getCount() + adjustment));
text.addMarkup(outgoing.getCount() + adjustment, outgoing.getCount() + adjustment + hide, "<span style=\"display: none\">", "</span>");
adjustment += hide;
// If openTags is not empty, but there are no escape sequences directly on this line, or if we
// are just closing tags at the end of the line, there is nothing to hide.
if (hide != 0) {
LOGGER.log(Level.FINEST, "hiding {0} @{1}", new Object[] { hide, outgoing.getCount() + adjustment });
text.addMarkup(outgoing.getCount() + adjustment, outgoing.getCount() + adjustment + hide, "<span style=\"display: none\">", "</span>");
adjustment += hide;
}
}
}
public void emitHtmlDirect(String html) {
text.addMarkup(incoming.getCount(), html);
}
}
EmitterImpl emitter = new EmitterImpl();
CountingOutputStream incoming = new CountingOutputStream(new AnsiHtmlOutputStream(outgoing, colorMap, emitter));
AnsiHtmlOutputStream ansiOs = new AnsiHtmlOutputStream(outgoing, colorMap, emitter);
CountingOutputStream incoming = new CountingOutputStream(ansiOs);
emitter.incoming = incoming;
try {
for (AnsiAttributeElement element : openTags) {
// We need to reopen tags that were still open at the end of the previous line in the
// so the stream's state is correct in case those tags are closed mid-line.
ansiOs.openTag(element);
}
byte[] data = s.getBytes(charset);
for (int i = 0; i < data.length; i++) {
// Do not use write(byte[]) as offsets in incoming would not be accurate.
incoming.write(data[i]);
}
nextOpenTags = ansiOs.getOpenTags();
// Close any tags that are still open at the end of the line.
ansiOs.closeOpenTags(null);
} catch (IOException x) {
LOGGER.log(Level.WARNING, null, x);
}
LOGGER.finer(() -> "\"" + StringEscapeUtils.escapeJava(s) + "\" → \"" + StringEscapeUtils.escapeJava(text.toString(true)) + "\"");
}
return this;
return openTags == nextOpenTags
? this
: new ColorConsoleAnnotator(colorMapName, charset, nextOpenTags);
Copy link
Member Author

Choose a reason for hiding this comment

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

Based on my reading of the ConsoleAnnotator Javadocs, if we want to track state from line-to-line then we need to handle it by returning a new instance like this. Maybe wrong to return this instead of creating a new state when the open tags match.

Copy link
Member

Choose a reason for hiding this comment

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

I thought the suggested fix for JENKINS-34019 was just to close all outstanding tags at the end of a line, as if this were a user error, not to try to make them apply across lines.

Copy link
Member Author

@dwnusbaum dwnusbaum Nov 19, 2018

Choose a reason for hiding this comment

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

When a style continues to the next line, the ConsoleAnnotatorFactory can insert a closing tag at the end of the line and an opening tag at the beginning of the next line. This prevents the timestamps from being affected.

Full comment. What you propose would certainly be simpler, but I'm not sure what the expected behavior is. What do you think should be the behavior for the test case I added?

Copy link
Member

@jglick jglick Jan 3, 2019

Choose a reason for hiding this comment

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

I feel like it does not matter much, since it is unlikely people would have multiline formatting intentionally, so it is OK to keep the implementation simpler.

Copy link
Member Author

Choose a reason for hiding this comment

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

#145 shows a problem with output of terraform at the command line, and based on a screenshot of the text it looks like it might be using multiline formatting, but I'm not sure. I'll leave this as-is for now until I have time to investigate in more depth.

Copy link
Member

Choose a reason for hiding this comment

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

Confirmed via

docker run --rm hashicorp/terraform plan 2>&1 | od -a

(Though it also looks like the Terraform output is buggy—ANSI sequences are not nestable, and it is emitting several esc [ 0 m sequences in a row.)

}

private Object readResolve() {
// Compatibility for instances serialized before the openTags field was added.
if (openTags == null) {
return new ColorConsoleAnnotator(colorMapName, charset);
} else {
return this;
}
}

@Extension
public static final class Factory extends ConsoleAnnotatorFactory<Object> {

@Override
public ConsoleAnnotator<Object> newInstance(Object context) {
LOGGER.fine("context=" + context);
LOGGER.log(Level.FINE, "context={0}", context);
if (context instanceof Run) {
ColorizedAction action = ((Run) context).getAction(ColorizedAction.class);
if (action != null) {
Expand All @@ -122,7 +159,7 @@ public ConsoleAnnotator<Object> newInstance(Object context) {
if (exec instanceof Run) {
ColorizedAction action = ((Run) exec).getAction(ColorizedAction.class);
if (action != null) {
return new ColorConsoleAnnotator(action.colorMapName, /* JEp-206 */ "UTF-8");
return new ColorConsoleAnnotator(action.colorMapName, /* JEP-206 */ "UTF-8");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,33 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
});
}

@Test
Copy link
Member Author

Choose a reason for hiding this comment

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

I should create a JIRA issue for these and add @Issue.

public void testMultilineEscapeSequence() throws Exception {
story.then(r -> {
FreeStyleProject p = r.createFreeStyleProject();
p.getBuildWrappersList().add(new AnsiColorBuildWrapper(null));
p.getBuildersList().add(new TestBuilder() {
@Override
public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
listener.getLogger().println("\u001B[1;34mThis text should be bold and blue");
listener.getLogger().println("Still bold and blue");
listener.getLogger().println("\u001B[mThis text should be normal");
return true;
}
});
FreeStyleBuild b = r.buildAndAssertSuccess(p);
StringWriter writer = new StringWriter();
b.getLogText().writeHtmlTo(0L, writer);
String html = writer.toString();
System.out.print(html);
assertThat(html.replaceAll("<span style=\"display: none\">.+?</span>", ""),
allOf(
containsString("<b><span style=\"color: #1E90FF;\">This text should be bold and blue\n</span></b>"),
Copy link
Member Author

Choose a reason for hiding this comment

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

The literal \n characters in these tests would probably cause them to fail on Windows.

containsString("<b><span style=\"color: #1E90FF;\">Still bold and blue\n</span></b>"),
not(containsString("\u001B[m"))));
});
}

@Issue("JENKINS-54133")
@Test
public void testWorkflowWrap() throws Exception {
Expand Down