Update for os.path.expandvars bug 49589
Here follows the comment I just added to bug "[ 494589 ] os.path.expandvars deletes things on w32". It concerns some mismatching (buggy) implementation / docstrings / comments for the ntpath.py and dospath.py. (Especially) As it is my first post as well as bugfix try for any open-source project, I am very happy receiving constructiv critic. tim_one is right. There is plenty of dodgy things hiding behind the os.path world, especially when it comes to os.path.expandvars() There are two problems here. - Mismatch in between the doc strings of the different implementation of expandvars and the "official" os.path.expandvars documentation. - the ntpath and dospath implementations are buggy when compared to their comments/docstrings. About the first problem, the inconsistency created some time ago in between the different implementations tasks makes it difficult to choose a solution. Everyone will probably agree that all the platform specific implementations of expandvars should have the same functionality. The one that should be taken over will probably need to be announced by the BDFL. Some rule which should not have let this here happen, and on which I believe we all will agree on: Same interface=same documentation->same functionality To implement either copy paste exactly the same expandvars definition from one platform to another (NT, DOS, POSIX), or somehow rather arrange that when there is no specific implementation for the platform, a "default" python implementation is used on the os.path level. To maximize the fruits of my small work, I would of course prefer that the version below becomes the standard and that the documentation get updated. To be complete, shall the documentation remain unchanged and the implementation of dos and nt gets adapted (copied from posix), the mac implementation could remain unchanged. But I feel its docstring and its documentation should be in line with the rest of the implementations. So my view point-> same interface, same documentation For the second problem - as of now a real bug whatever we decide, I wrote within this comment (hereafter) a new expandvars version which fits the docstring documentation of dospath.py and the comments of ntpath.py. Sorry you will be getting no patch from me at the moment since sourceforge's anonymous CVS access does not like me. Please note that my version borrows alot from the posixpath.py implementation and my changes are the ones of a python amateur who is open to critic. #expandvars() implementation _varprog = None _findquotes = None def expandvars(path): """Expand paths containing shell variable substitutions. The following rules apply: - no expansion within single quotes - no escape character, except for '$$' which is translated into '$' - ${varname} is accepted. - varnames can be made out of letters, digits and the character '_'""" global _varprog, _findquotes if '$' not in path: return path if not _varprog: import re _varprog = re.compile(r'\$(\w+|\{[^}]*\}|\$)') _findquotes = re.compile("'.*?'") quoteareas = [] i = 0 while 1: quotearea = _findquotes.search(path, i) if not quotearea: break (i, j) = quotearea.span(0) quoteareas.append((i, j)) i = j i = 0 while 1: m = _varprog.search(path, i) if not m: break i, j = m.span(0) insidequotes=None for (quotebegin, quoteend) in quoteareas: if quotebegin < i and quoteend > i: insidequotes=1 break if insidequotes: i = j continue name = m.group(1) if name[:1] == '$': path = path[:i] + '$' + path[j:] i = i + 1 else: if name[:1] == '{' and name[-1:] == '}': name = name[1:-1] if os.environ.has_key(name): tail = path[j:] path = path[:i] + os.environ[name] i = len(path) path = path + tail else: i = j return path Regards, Ben.
participants (1)
-
Behrang Dadsetan