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 #37091 - Pretty print long YAML list #684

Merged
merged 1 commit into from
Mar 24, 2024

Conversation

Et7f3
Copy link
Contributor

@Et7f3 Et7f3 commented Jan 23, 2024

Hello,

I come after Katello/katello#10850. During testing a long list was printed for packages_name.

  1. It was hard to read
  2. Ruby print list in the same format than YAML compact form. It doesn't seems the original author was aware (I think it expected a String because it called it name and not names)

@Et7f3 Et7f3 force-pushed the pretty_print_yaml_list branch from f7e2fb5 to c5a9f08 Compare January 23, 2024 16:49
@Et7f3 Et7f3 changed the title Fix #37091 - Pretty print long YAML list Fixes #37091 - Pretty print long YAML list Jan 23, 2024
@Et7f3 Et7f3 force-pushed the pretty_print_yaml_list branch from c5a9f08 to 3993394 Compare January 24, 2024 10:52
@@ -41,7 +40,7 @@ model: JobTemplate
<%- end -%>
tasks:
- package:
name: <%= input('name') %>
<%= indent(8) { to_yaml({"name" => input('name')}).gsub(/---\n/, '') } -%>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to remove my erb loop/if pasta code. to_yaml macro doesn't seems to support :UseHeader => true https://github.com/ruby/ruby/blob/4099b04c099899d92dab3f34f1152038ed718d79/test/psych/helper.rb#L51

@nofaralfasi
Copy link
Contributor

@Et7f3, I apologize for the delay.
Could you please rebase? Currently the tests are broken, but once they're fixed I will merge this PR.

@Et7f3 Et7f3 force-pushed the pretty_print_yaml_list branch from 3993394 to 2221b27 Compare February 19, 2024 19:11
@Et7f3
Copy link
Contributor Author

Et7f3 commented Feb 19, 2024

for the delay

No issue it is OSS.

Could you please rebase?

It was just to retrigger the CI ? I don't see conflict with it.

@nofaralfasi nofaralfasi merged commit 7e7d502 into theforeman:master Mar 24, 2024
14 checks passed
@nofaralfasi
Copy link
Contributor

Thanks @Et7f3!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants