Adding testcase for Add(..., subst)

Signed-off-by: Mats Wichmann <[email protected]>

Author: mwichmann

RELEASE.txt

@@ -50,7 +50,7 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
   Now matches the annotation and docstring (which were prematurely
   updated in 4.6). All SCons usage except unit test was already fully
   consistent with a bool.
-- The Variables object Add method now accepts a do_subst keyword argument
+- The Variables object Add method now accepts a subst keyword argument
   (defaults to True) which can be set to inhibit substitution prior to
   calling the variable's converter and validator.
 

SCons/Variables/VariablesTests.py

@@ -150,6 +150,28 @@ def test_Update(self) -> None:
         opts.Update(env, {})
         assert env['ANSWER'] == 54
 
+        # Test that the value is not substituted if 'subst' is False
+        def check_subst(key, value, env) -> None:
+            """Check that variable was not substituted before we get called."""
+            assert value == "$ORIGIN", \
+                f"Validator: '$ORIGIN' was substituted to {value!r}"
+
+        def conv_subst(value) -> None:
+            """Check that variable was not substituted before we get called."""
+            assert value == "$ORIGIN", \
+                f"Converter: '$ORIGIN' was substituted to {value!r}"
+            return value
+
+        opts.Add('NOSUB',
+                 help='Variable whose value will not be substituted',
+                 default='$ORIGIN',
+                 validator=check_subst,
+                 converter=conv_subst,
+                 subst=False)
+        env = Environment()
+        opts.Update(env)
+        assert env['NOSUB'] == "$ORIGIN"
+
         # Test that a bad value from the file is used and
         # validation fails correctly.
         test = TestSCons.TestSCons()

SCons/Variables/__init__.py

@@ -114,6 +114,9 @@ def _do_add(
         """Create a Variable and add it to the list.
 
         Internal routine, not public API.
+
+        .. versionadded:: 4.8.0
+              *subst* keyword argument is now recognized.
         """
         option = Variable()
 
@@ -252,7 +255,7 @@ def Update(self, env, args: Optional[dict] = None) -> None:
         for option in self.options:
             if option.converter and option.key in values:
                 if option.do_subst:
-                    value = env.subst(f'${option.key}')
+                    value = env.subst('${%s}' % option.key)
                 else:
                     value = env[option.key]
                 try:
@@ -270,7 +273,7 @@ def Update(self, env, args: Optional[dict] = None) -> None:
         for option in self.options:
             if option.validator and option.key in values:
                 if option.do_subst:
-                    value = env.subst('${%s}'%option.key)
+                    value = env.subst('${%s}' % option.key)
                 else:
                     value = env[option.key]
                 option.validator(option.key, value, env)

doc/man/scons.xml

@@ -4835,7 +4835,7 @@ not to any stored-values files.
 
 <variablelist>
   <varlistentry id="v-Add">
-  <term><replaceable>vars</replaceable>.<function>Add</function>(<parameter>key, [help, default, validator, converter, do_subst]</parameter>)</term>
+  <term><replaceable>vars</replaceable>.<function>Add</function>(<parameter>key, [help, default, validator, converter, subst]</parameter>)</term>
   <listitem>
 <para>Add a customizable &consvar; to the &Variables; object.
 <parameter>key</parameter>
@@ -4889,12 +4889,12 @@ it can raise a <exceptionname>ValueError</exceptionname>.
 
 <para>
 Substitution will be performed on the variable value
-as it is added, before the converter and validator are called,
-unless the optional <parameter>do_subst</parameter> parameter
+before the converter and validator are called,
+unless the optional <parameter>subst</parameter> parameter
 is false (default <literal>True</literal>).
 Suppressing substitution may be useful if the variable value
-looks like a &consvar; reference (<literal>$VAR</literal>)
-to be expanded later.
+looks like a &consvar; reference (e.g. <literal>$VAR</literal>)
+and the validator and/or converter should see it unexpanded.
 </para>
 
 <para>
@@ -4932,7 +4932,7 @@ vars.Add('COLOR', validator=valid_color)
 
 <para>
 <emphasis>Changed in version 4.8.0:</emphasis>
-added the <parameter>do_subst</parameter> parameter.
+added the <parameter>subst</parameter> parameter.
 </para>
   </listitem>
   </varlistentry>