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

Turn SCons.Variables.Variable into a dataclass #4658

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mwichmann
Copy link
Collaborator

@mwichmann mwichmann commented Dec 6, 2024

This moves the initialization into the Variable class itself, rather than being done in the _do_add method of the Variables class. We get nice printing and comparison for free via the dataclass mechanism, so the old methods are dropped from Variable.

Also correct one long-standing minor inconsistency: if a list of names for a variable is given, it is split into name and aliases, with the name not duplicated into the aliases. If only one name is given (no aliases), it was also added as the sole entry in the aliases; now it is not added to the aliases. All the SCons usages of aliases combine them back together anyway, like:

    if arg in option.aliases + [option.key,]:

So there's no need for the behavior of the one-name form, including the name in aliases. One test did need to change for this - the test of a custom help formatter function was wired to accept the old form, but only because of the way the custom formatter in the test was written, so this is a self-contained problem.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

Also correct one long-standing minor inconsistency: if a list of names
for a variable is given, it is split into name and aliases, with the
name not duplicated into the aliases. If only one name is given, it is
both the name and the only entry in the aliases. All of the usages of
aliases combine them back together anyway, like:

    if arg in option.aliases + [option.key,]:

So there's no need for the behavior of the one-name form including the
name in aliases. One test did need to change for this - the test of a
custom help formatter function was wired to accept the old form, but
only because of the way the custom formatter in the test was written,
so this is a self-contained problem.

Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann mwichmann added the Variables Variables() subsystem label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Variables Variables() subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants