This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Fix for bug 494589
Type: Stage:
Components: Library (Lib) Versions: Python 2.2
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: mhammond Nosy List: jackdied, mhammond, nnorwitz, tebeka, tim.peters
Priority: normal Keywords: patch

Created on 2002-12-26 07:49 by tebeka, last changed 2022-04-10 16:06 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
ntpath.diff tebeka, 2002-12-29 07:28 Diff file
nt2.diff nnorwitz, 2003-01-04 03:31 patch 2 (nn)
expandvars.patch mhammond, 2003-01-29 23:11 Patch with test suite
ntpath.diff tebeka, 2003-01-30 11:30 diff from previous fix
Messages (14)
msg42102 - (view) Author: Miki Tebeka (tebeka) * Date: 2002-12-26 07:49
This is a fix for bug 494589 (os.path.expandvars)
I suggest using the same code in ntpath and posixpath.
(Maybe have a commonpath.py and let both import it?)

Python version 2.2.2
OS: NT4 SP6 (checked on NT and cygwin)

Miki
msg42103 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2002-12-26 18:07
Logged In: YES 
user_id=33168

There's no uploaded file!  You have to check the
checkbox labeled "Check to Upload & Attach File"
when you upload a file.

Please try again.

(This is a SourceForge annoyance that we can do
nothing about. :-( )
msg42104 - (view) Author: Miki Tebeka (tebeka) * Date: 2002-12-29 07:28
Logged In: YES 
user_id=358087

This time the checkbox is checked. :-)

Miki
msg42105 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2003-01-04 03:31
Logged In: YES 
user_id=33168

The patch didn't apply for me, so I created a new one and
attached it.  I can't test this.  Maybe Tim is interested.
msg42106 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-01-13 17:59
Logged In: YES 
user_id=31435

Mark, can you make time to look at this?  I can't.
msg42107 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2003-01-15 03:44
Logged In: YES 
user_id=14198

Is there any reason why:

from posixpath import expandvars

is not a better patch?  From what I can see, posixpath's
version works fine for Windows (windows os.environ is case
insensitive)
msg42108 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2003-01-15 12:38
Logged In: YES 
user_id=14198

In fact, why not go the whole-hog, and remove all code in
ntpath.py that is identical to posixpath.py

Example patch attached <wink>
msg42109 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-01-15 19:14
Logged In: YES 
user_id=31435

Sounds like an excellent idea to me, Mark!  The glory is all 
yours, if you're man enough to accept it <wink>.
msg42110 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2003-01-16 05:24
Logged In: YES 
user_id=14198

It was late last night - the idea of ripping out all
duplicated code wont work.  A consolidation may be possible,
but I haven't time.  I'm deleting that patch, but still
believe that

from posixpath import expandvars

is reasonable.  Comments?
msg42111 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-01-17 01:24
Logged In: YES 
user_id=31435

Did you look at bug 494589?  As I noted there, there are 
semantic diffferences between the ntpath and posixpath 
versions of .expandvars() (like ntpath mapping $$ to $, 
and not expanding within single quotes).

I personally have no use for the differences, but can't say 
whether anyone else does.  The author of the ntpath version 
took time to write comments about its pecularities, so they 
weren't accidents at the time.  Incompatible changes are 
usually PEP material.
msg42112 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2003-01-29 23:11
Logged In: YES 
user_id=14198

Yes, I was too eager there.

So back to the original patch - it looks good, except it
seems to fail in one case I can see:
$FOO$FOO

is not correctly expanded.  Put a space between the vars, or
enclose them in braces, and it works correctly.

This isn't really a regression though - the old code doesn't
handle that case correctly either.  posixpath does.

See the new patch I uploaded - it contains the original
code, plus a patch to test_ntpath.py to test the semantics.
 Is it possible to fix the patch to handle this case?  I
haven't time to dig out my regex book <wink>
msg42113 - (view) Author: Miki Tebeka (tebeka) * Date: 2003-01-30 11:30
Logged In: YES 
user_id=358087

Hopefully this should do the trick (if I'll remember to
attache the file :-)
All I did was to allow a $ only right after the first one.

Miki
msg42114 - (view) Author: Jack Diederich (jackdied) * (Python committer) Date: 2007-03-09 01:56
does the following svn message mean this should be closed?

r53460 | sjoerd.mullender | 2007-01-16 11:42:38 -0500 (Tue, 16 Jan 2007) | 4 lines

Fixed ntpath.expandvars to not replace references to non-existing
variables with nothing.  Also added tests.
This fixes bug #494589.
msg42115 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2007-03-10 04:19
Given the bug is closed, seems reasonable to close this patch too.  Thanks for pointing it out Jack.
History
Date User Action Args
2022-04-10 16:06:03adminsetgithub: 37648
2002-12-26 07:49:54tebekacreate