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

PackageVariable now returns the default on "true" #4591

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

mwichmann
Copy link
Collaborator

@mwichmann mwichmann commented Sep 1, 2024

In all doc versions until 4.8.0, PackageVariable had wording like: The option will support the values yes, true, on, enable or search, in which case the specified default will be used, but the code didn't actually do that, it just returned True. With this change it now returns the default value, with a slight tweak - if the default is one of the spelled out enabling strings, it returns the boolean True instead. This should be harmless - the produced value is still going to evaluate true, even if it can now be a path string.

The indication that the default is produced if a truthy string is given is restored to the manpage (it was never dropped from the User Guide).

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 read the README.rst)
  • I have updated the appropriate documentation

@mwichmann mwichmann added the Variables Variables() subsystem label Sep 1, 2024
@mwichmann
Copy link
Collaborator Author

Windows fail on the test addition, because the tested path ended up not properly escaped. Will investigate.

STDOUT =========================================================================
scons: Reading SConscript files ...

STDERR =========================================================================
  File "C:\Users\mats\AppData\Local\Temp\scons\testcmd.30516.xh_86zgt\SConstruct.path", line 8

    default='C:\Users\mats\AppData\Local\Temp\scons\testcmd.30516.xh_86zgt\path\to\tinycbor'

            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 2-3: truncated \UXXXXXXXX escape

@mwichmann
Copy link
Collaborator Author

Windows fail on the test addition, because the tested path ended up not properly escaped. Will investigate.

Forgot the r to make it a rawstring - one-character fix. Works for me on Windows now.

In all doc versions until 4.8.0, PackageVariable had wording like:
"The option will support the values yes, true, on, enable or search,
in which case the specified default will be used", but the code didn't
actually do that, it just returned True. With this change it now returns
the default value, with a slight tweak - if the default is one of the
spelled out enabling strigs, it returns the boolean True instead.

The indication that the default is produced if a truthy string is given
is restored to the manpage (it was never dropped from the User Guide).

Signed-off-by: Mats Wichmann <[email protected]>
Else it fails on Windows.

Signed-off-by: Mats Wichmann <[email protected]>
@bdbaddog bdbaddog merged commit aad42dd into SCons:master Sep 11, 2024
6 of 8 checks passed
@mwichmann mwichmann deleted the maint/PkgVariable branch September 11, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Variables Variables() subsystem
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants