Improve handling of prepend_sys_path, fixes #1330
Description
- If
os.nameequals'nt' then don't split using colons - Split on any whitespace, not just the
' 'character so people can break up long lists into multiple lines. - Also normalize and trim paths to prevent issues with the previous point.
Fixes: #1330
How should I test this?
Checklist
This pull request is:
- [ ] A documentation / typographical error fix
- Good to go, no issue or tests are needed
- [x] A short code fix
- please include the issue number, and create an issue if none exists, which must include a complete example of the issue. one line code fixes without an issue and demonstration will not be accepted.
- Please include:
Fixes: #<issue number>in the commit message - please include tests. one line code fixes without tests will not be accepted.
- [ ] A new feature implementation
- please include the issue number, and create an issue if none exists, which must include a complete example of how the feature would look.
- Please include:
Fixes: #<issue number>in the commit message - please include tests.
Can someone please give brief guidance on how to create a test for this.
If os.name equals 'nt' then don't split using colons
I would use os.pathsep for this
Split on any whitespace, not just the ' ' character so people can break up long lists into multiple lines.
also wired that space is considered a separator, since a path can have a space in it.
@zzzeek do you remember the reason?
Split on any whitespace, not just the ' ' character so people can break up long lists into multiple lines.
also wired that space is considered a separator, since a path can have a space in it.
@zzzeek do you remember the reason?
There was a complex legacy arrangement here when the path separator system was first introduced, so I would look at the history there to see what our default separator used to be etc.
I like using os.pathsep seems much more portable.
Am I good to just remove splitting on whitespace altogether?
hi there, and sorry, I looked at what we are actually doing here.
This is a new use case, and it looks like prepend_sys_path was very unfortunately glossed over when we added configurable path separators. So we have the same legacy situation where we should not have a loosely defined scheme like this.
I propose we add another new config attribute prepend_sys_path_separator and have it work in the same way as version_path_separator - if not present, then the exact broken scheme here is used for backwards compat. for new projects, the default will be "os".
Taht is like this:
# sys.path path, will be prepended to sys.path if present.
# defaults to the current working directory. for multiple paths, the path separator
# is defined by prepend_sys_path_separator
prepend_sys_path = .
# prepend sys path separator; As mentioned above, this is the character used to split
# sys_path paths. The default within new alembic.ini files is "os", which uses os.pathsep.
# If this key is omitted entirely, it falls back to the legacy behavior of splitting on spaces and/or commas.
# Valid values for prepend_sys_path_separator are:
#
# prepend_sys_path_separator = :
# prepend_sys_path_separator = ;
# prepend_sys_path_separator = space
prepend_sys_path_separator = os # Use os.pathsep. Default configuration used for new projects.
I replaced the earlier implementation with a new one that does what @zzzeek described. Haven't tried it yet though, will do that on Monday