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

Resolve path arguments when specified on command line #575

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Aug 10, 2023

This change enhances the behavior of command line arguments which specify a path which might be relative to the current directory.

The new functionality captures the current working directory when the parser is constructed, and captures a given relative path on the command line by resolving it from that captured working directory. If an argument is left to the default value, it is not resolved by this new functionality and is left for resolution in the consuming code later.

The main use case for this capture-then-resolve behavior is to support changing the working directory of the colcon process. If that happens, the context of where a user intended a relative path to be based is lost, so this change captures the directory early in the colcon startup sequence.

An alternative approach to changing resolving the path only when specified on the command line could involve creating a custom argparse action implementation. A disadvantage would be a tighter coupling to the argparse API.

This change enhances the behavior of command line arguments which
specify a path which might be relative to the current directory.

The new functionality captures the current working directory when the
parser is constructed, and captures a given relative path on the command
line by resolving it from that captured working directory. If an
argument is left to the default value, it is not resolved by this new
functionality and is left for resolution in the consuming code later.

The main use case for this capture-then-resolve behavior is to support
changing the working directory of the colcon process. If that happens,
the context of where a user intended a relative path to be based is
lost, so this change captures the directory early in the colcon startup
sequence.
@cottsay cottsay self-assigned this Aug 10, 2023
@cottsay cottsay added the enhancement New feature or request label Aug 10, 2023
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Patch coverage: 60.00% and project coverage change: -0.09% ⚠️

Comparison is base (7b70e61) 81.88% compared to head (29bbb61) 81.79%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #575      +/-   ##
==========================================
- Coverage   81.88%   81.79%   -0.09%     
==========================================
  Files          62       63       +1     
  Lines        3648     3663      +15     
  Branches      705      706       +1     
==========================================
+ Hits         2987     2996       +9     
- Misses        609      615       +6     
  Partials       52       52              
Files Changed Coverage Δ
colcon_core/verb/build.py 0.00% <0.00%> (ø)
colcon_core/argument_type.py 60.00% <60.00%> (ø)
colcon_core/package_discovery/path.py 100.00% <100.00%> (ø)
colcon_core/verb/test.py 29.52% <100.00%> (+1.36%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cottsay cottsay merged commit c323f60 into master Aug 17, 2023
@delete-merged-branch delete-merged-branch bot deleted the cottsay/resolve-path-args branch August 17, 2023 22:44
@cottsay cottsay added this to the 0.13.0 milestone Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants