Skip to content

utilize pathlib.Path instead of os.path.join() #82967

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

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

doc-sheet
Copy link
Contributor

SUMMARY

Call pathlib.Path() instead of os.path.join() from path_join filter because of better handle of subdirectories.

It is now possible to call {{ path1~'/'~path2 | path_join }} to merge multiple path fragments without need to sanitize occasion slash characters.

Result converted to posix path to keep compatibility and old behavior.
e.g.

{{ ('c:', 'fsff') | path_join  }} -> c:/fsff
ISSUE TYPE
  • Feature Pull Request
ADDITIONAL INFORMATION

Currently to merge multiple path fragments with a nice single delimiter we need to do something like this:

{{ ('/opt', app, subpath | string | trim('/')) | path_join }}

Assuming app = 'myapp' and subpath anything good to be a path.
For example /var/lib/myapp/data

After this change it is safe to write

{{ '/opt/'~app~'/'~subpath | path_join }}
# or
{{ ('/opt', app~'/'~subpath) | path_join }}
# simple concatenation of strings ('/opt/'~app~'/'~subpath)
/opt/myapp//var/lib/myapp/data

# old path_join with trim or just new path_join
/opt/myapp/var/lib/myapp/data

@ansibot ansibot added feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. labels Apr 2, 2024
@doc-sheet
Copy link
Contributor Author

ready_for_review

@nitzmahone nitzmahone self-requested a review April 4, 2024 19:20
@nitzmahone nitzmahone removed the needs_triage Needs a first human triage before being processed. label Apr 4, 2024
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 11, 2024
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Apr 7, 2025
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Apr 15, 2025
@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_pr This PR has not been pushed to for more than one year. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels May 24, 2025
@doc-sheet
Copy link
Contributor Author

ready_for_review

@mattclay
Copy link
Member

mattclay commented Jun 4, 2025

@doc-sheet It looks like what you want is the normpath filter:

    - debug:
        msg: "{{ 'path/with//extra///slashes////' | normpath }}"
ok: [localhost] => {
    "msg": "path/with/extra/slashes"
}

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 4, 2025
@doc-sheet
Copy link
Contributor Author

Not quite, I don't want to mutate original string more then just clear double slashes

>>> os.path.normpath('asdasd/../fdsfs')
'fdsfs'

>>> Path('asdasd/../fdsfs').as_posix()
'asdasd/../fdsfs'

But I'll try normpath too, thanks for reminder!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants