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

src: lib: cli: Expand all args, and canonize logpath #117

Conversation

joaoantoniocardoso
Copy link
Member

@joaoantoniocardoso joaoantoniocardoso commented Feb 20, 2025

With this patch we can pass env variables within the args, as in:

export WEB_SERVER=127.0.0.1:1234
export FAKECLIENT_KIND=debug
cargo run -- \
  --verbose \
  --log-path $PWD/logs \
  --web-server $WEB_SERVER \
  fakeclient://$FAKECLIENT_KIND

@joaoantoniocardoso joaoantoniocardoso marked this pull request as draft February 20, 2025 17:24
@joaoantoniocardoso joaoantoniocardoso force-pushed the expand_all_and_canonize_logpath branch 4 times, most recently from 15a29cf to 6007a81 Compare February 20, 2025 19:32
@joaoantoniocardoso joaoantoniocardoso force-pushed the expand_all_and_canonize_logpath branch from 6007a81 to b2de93f Compare February 20, 2025 19:36
@joaoantoniocardoso joaoantoniocardoso marked this pull request as ready for review February 20, 2025 19:42
}

/// Constructs our manager, Should be done inside main
/// Note: differently from init(), this doesn't expand env variables
Copy link
Member Author

@joaoantoniocardoso joaoantoniocardoso Feb 21, 2025

Choose a reason for hiding this comment

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

Just an additional comment here, this would be very hard to do: we don't have a simple and maintainable way to obtain a command-line string from Args and re-parse the string into Args after expanding.

@patrickelectric patrickelectric merged commit 55b0501 into bluerobotics:master Feb 21, 2025
8 checks passed
@joaoantoniocardoso joaoantoniocardoso deleted the expand_all_and_canonize_logpath branch February 21, 2025 17:26
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