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

Improve the formatting of the preamble #1352

Merged
merged 7 commits into from Dec 5, 2022
Merged

Improve the formatting of the preamble #1352

merged 7 commits into from Dec 5, 2022

Conversation

joerick
Copy link
Contributor

@joerick joerick commented Nov 26, 2022

The preamble has been getting a bit scruffy lately, this tidies it up.

  • fixed the issue with environment thinking it was overridden when it wasn't
  • objects like BuildSelector provide a custom, nested output, which is more friendly than the repr
  • overrides are grouped when they have the same value
  • using str rather than repr for other objects, makes e.g. Paths easier to read
  • options that are set to their defaults are dimmed using an ANSI code

Simple example

Here I'm setting CIBW_ENVIRONMENT="a=1 b=2" CIBW_PLATFORM=linux

Before

image

After

image

more complicated example

This example has a pyproject.toml:

[tool.cibuildwheel]
build = ["cp38*", "cp37*"]
skip = ["*musllinux*"]
environment = {FOO="BAR"}

test-command = "pyproject"

manylinux-x86_64-image = "manylinux1"

environment-pass = ["EXAMPLE_ENV"]

[tool.cibuildwheel.macos]
test-requires = "else"

[[tool.cibuildwheel.overrides]]
select = "cp37*"
test-command = "pyproject-override"
manylinux-x86_64-image = "manylinux2014"

Before

image

After

image


At a code level, worth noting is that the Options object is passed in an environ object, rather than reading it from os.environ. This allows us to construct a 'default' Options object to compare with. Also, it requires less monkey-patching to test, which is nice.

@joerick
Copy link
Contributor Author

joerick commented Nov 27, 2022

image

ANSI 'gray' (\033[90m aka 'bright black') doesn't look good on github mobile for some reason, i guess it's just interpreted as 'black'. I'll try a mid-gray from the 256-color palette instead.

@joerick
Copy link
Contributor Author

joerick commented Nov 30, 2022

Github actions on web appears to forget the current ANSI color on newline. I'll need to refresh the color on each newline.

@joerick joerick marked this pull request as ready for review December 4, 2022 19:16
Copy link
Contributor

@henryiii henryiii left a comment

Choose a reason for hiding this comment

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

Looks nice, a few minor nit picks / comments that you can ignore or address at your discretion. :)

cibuildwheel/options.py Outdated Show resolved Hide resolved
cibuildwheel/options.py Outdated Show resolved Hide resolved
Comment on lines +679 to +680
Github Actions forgets the current ANSI style on every new line. This
function repeats the current ANSI style on every new line.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is weird - though I see how it could happen, GH does a lot of optimization for logs. Is there a GH ticket or commend anywhere? I've not generally noticed GH breaking elsewhere with color, but maybe most things like compilers and pytest do per-line coloring too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is not. I should raise an issue somewhere. Not sure where though. Maybe I'll just raise it at actions/runner and see what they say.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

joerick and others added 2 commits December 5, 2022 19:04
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
@joerick joerick added the automerge Tells https://github.com/apps/mergery to squash-merge the PR when the button is green. label Dec 5, 2022
@joerick joerick merged commit 61b6cc8 into main Dec 5, 2022
@joerick joerick deleted the improve-preamble branch December 5, 2022 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Tells https://github.com/apps/mergery to squash-merge the PR when the button is green.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants