jenkinsci / ansicolor-plugin

Jenkins ANSI Color Plugin
https://plugins.jenkins.io/ansicolor/
MIT License
252 stars 82 forks source link

Another issue with timestamper #275

Open hcldan opened 9 months ago

hcldan commented 9 months ago

Jenkins and plugins versions report

Environment ```text Paste the output here ```

What Operating System are you using (both controller, and any agents involved in the problem)?

Ubuntu

Reproduction steps

  1. Timestamper enabled globally for all pipeline jobs
  2. AnsiColor 1.0.3
  3. Run a build and get really weird looking output.

Expected Results

Expect the console color blocks to have an uninterrupted display for each line.

Actual Results

image

image

The div is rendering as block forcing a new line after the timestamp span.

Anything else?

No response

Are you interested in contributing a fix?

No response

chill389cc commented 4 months ago

I'd like to bump this. It occurs on the vga and gnome-terminal modes because those modes set defaultFg and defaultBg to non-null values, which creates a wrapping <div> tag around the line. Since the timestamp is a <span> which gets added before ansiColor's div, the div gets bumped to a new line because <span>'s are naturally inline but <div>'s are not. Here is the line of code causing this: https://github.com/jenkinsci/ansicolor-plugin/blob/74393de55727a57a83eb66b0ddd55142700ec415/src/main/java/hudson/plugins/ansicolor/AnsiHtmlOutputStream.java#L236-L240

We could just change the <div> to a <span> there but it doesn't look quite as clean with the background colors, there are gaps between each line that have the overall pages background color so its not as pretty as it looks with the <div>'s. However, I'd still prefer a solution that didn't use <div>'s so that these themes didn't conflict with other plugins.

You can get the same appearance by adding display: inline-block; to the style tag of the outer spans (what are currently divs). However, I'm seeing one issue where the closing </span> tag gets put on the next line of text which prevents the next line of text from appearing with a carriage return. Instead, it appears immediately after the preceeding line. I'm not explaining that very well, but I'll attach screenshots of what I mean.

This is what I get if I use <span>'s and add display: inline-block: image

This is what it should look like: image

This is the code I've edited to get to what the first screenshot looks like:


if (defaultFg != null || defaultBg != null) {
-    openTag(new AnsiAttributeElement(AnsiAttrType.DEFAULT, "div", "style=\"" +
+    openTag(new AnsiAttributeElement(AnsiAttrType.DEFAULT, "span", "style=\"" +
+        "display: inline-block;" +
        (defaultBg != null ? "background-color: " + colorMap.getNormal(defaultBg) + ";" : "") +
        (defaultFg != null ? "color: " + colorMap.getNormal(defaultFg) + ";" : "") + "\""));
}
dblock commented 4 months ago

@chill389cc Want to try writing some tests and PRing your change?

chill389cc commented 4 months ago

@dblock I understand that creating a failing unit test outlining the problem I identified in my previous post would be the most helpful way to fix the problem. Unfortunately I don't have as much time to look at this as I'd like. I've created a branch and I've pushed up my WIP code that I showed above. I've tried to make a unit test for this problem but I haven't pushed anything. I think the problem appears in the annotating of multiple consecutive lines, so it isn't as simple as creating a test like assertThat(annotate(<something multi-line>, AnsiColorMap.VGA), is(<not what it should>). If I get a chance in the future I'll poke around more but that's what I've got for now. Thanks for your work on this plugin!