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

Fix/Improve $(eval ...) #2269

Open
wants to merge 3 commits into
base: noetic-devel
Choose a base branch
from
Open

Fix/Improve $(eval ...) #2269

wants to merge 3 commits into from

Conversation

rhaschke
Copy link
Contributor

@rhaschke rhaschke commented Aug 24, 2022

Evaluation of global symbols in list comprehension context (e.g. $(eval [arg(var) for var in ['fuga', 'arg']])) was failing:
AttributeError: '_DictWrapper' object has no attribute 'get'

Essentially, symbols need to be passed as the globals argument to eval() and this argument needs to inherit from dict.

Additionally, this PR improves the discarding of private symbols (starting with double underscore). Previously, any double underscores were considered offending, even those in the middle of a symbol or in literal text.
These are the same changes, I have implemented for xacro.

We just need to protect against symbols starting with double underscores.
Having double underscores anywhere else isn't a problem.
@rhaschke
Copy link
Contributor Author

Ping @jacobperron

@peci1
Copy link
Contributor

peci1 commented Nov 28, 2022

This would make writing list comprehension expressions in launch files much easier! Thanks for the PR.

@peci1
Copy link
Contributor

peci1 commented Apr 9, 2023

@mjcarroll @sloretz friendly request for review. This PR is good to go according to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants