Skip to content

Conversation

@smoors
Copy link
Contributor

@smoors smoors commented Nov 14, 2025

No description provided.

@smoors smoors requested a review from lexming November 14, 2025 20:46
@boegel boegel added this to the next release (5.2.0) milestone Nov 19, 2025

def wrap_shell_vars(strng, wrap_prefix, wrap_suffix):
"""
Wrap variables $VAR or ${VAR} in wrap_tmpl template string
Copy link
Member

Choose a reason for hiding this comment

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

There's no such thing as wrap_tmpl?

return mods


def wrap_shell_vars(strng, wrap_prefix, wrap_suffix):
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a dedicated unit test for this new function, along with the end-to-end tests you've already added?

# shell environment variable name: ${__}VAR_NAME_00_SUFFIX
REGEX_SHELL_VAR_PATTERN = r'[A-Z_]+[A-Z0-9_]+'
REGEX_SHELL_VAR = re.compile(rf'\$({REGEX_SHELL_VAR_PATTERN})')
REGEX_QUOTE_SHELL_VAR = re.compile(rf'[\"\']\$({REGEX_SHELL_VAR_PATTERN})[\"\']')
Copy link
Member

Choose a reason for hiding this comment

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

Strictly speaking this is part of the API, so we shouldn't just remove these constants, people may be using them (in hooks for example)

LOAD_TEMPLATE_DEPENDS_ON = 'depends_on("%(mod_name)s")'
IS_LOADED_TEMPLATE = 'isloaded("%s")'

OS_GETENV_TEMPLATE = r'os.getenv("%s")'
Copy link
Member

Choose a reason for hiding this comment

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

Same here, this is part of the API, we shouldn't just remove this...

@boegel boegel changed the title support both $VAR and ${VAR} variable formats in modextravars support both $VAR and ${VAR} variable formats in modextravars Nov 19, 2025
@boegel boegel changed the title support both $VAR and ${VAR} variable formats in modextravars add support both $VAR and ${VAR} variable formats in modextravars Nov 19, 2025
@boegel boegel changed the title add support both $VAR and ${VAR} variable formats in modextravars add support both $VAR and ${VAR} variable formats in modextravars Nov 19, 2025
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