Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #22209 - Render empty lines properly in output #380

Merged
merged 4 commits into from Sep 19, 2018

Conversation

adamruzicka
Copy link
Contributor

For some reason the output uses CRLF to mark a newline, writing CRs into
the terminal window led to line numbers not matching the lines. Also we
were splitting the output on newlines and trailing empty fields were
omitted.

To reproduce run the following

printf "\none\n"
printf "\ntwo\n"
printf "\nthree\n"

For some reason the output uses CRLF to mark a newline, writing CRs into
the terminal window led to line numbers not matching the lines. Also we
were splitting the output on newlines and trailing empty fields were
omitted.
@adamruzicka
Copy link
Contributor Author

This still doesn't work fully for ansible and it is related to the output colorization code. WIP

There was an issue when line had color information but was otherwise
empty. Also multiple consecutive spaces were removed by the browser due
to missing css property.
@@ -1,4 +1,5 @@
module JobInvocationOutputHelper
COLOR_PATTERN = /\e\[.*?m/.freeze

Choose a reason for hiding this comment

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

Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.

color = seq[/(\d+)m/,1]
"{{{format color:#{color}}}}"
end

current_color = 'default'
out = %{<span style="color: #{@current_color}">}
out = %{<span style="color: #{current_color}">}
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the issue now, that the colorization doesn't survive the the new line?
rhlogo.zip

Copy link
Member

Choose a reason for hiding this comment

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

rhlogo
The whole hat should be red

@adamruzicka
Copy link
Contributor Author

Fixed
image

Copy link
Member

@iNecas iNecas left a comment

Choose a reason for hiding this comment

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

Tested and works well. Waiting for Jenkins

@adamruzicka
Copy link
Contributor Author

Test failures don't seem to be related

@iNecas
Copy link
Member

iNecas commented Sep 19, 2018

Test failures unrelated. Thanks @adamruzicka

@iNecas iNecas merged commit 059545f into theforeman:master Sep 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants