Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(76563)

#10395: new os.path function to extract common prefix based on path components

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 7 months ago by ronaldoussoren
Modified:
4 years, 11 months ago
Reviewers:
storchaka, rafik, merwok
CC:
loewis, rhettinger, pmoore, Ronald Oussoren, eric.smith, ezio.melotti, eric.araujo, r.david.murray, santa4nt, devnull_psf.upfronthosting.co.za, eric.snow, someuniquename_gmail.com, storchaka, rafik, paddy3118_gmail.com
Visibility:
Public.

Patch Set 1 #

Total comments: 6

Patch Set 2 #

Total comments: 16

Patch Set 3 #

Total comments: 1

Patch Set 4 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/os.path.rst View 1 2 3 1 chunk +16 lines, -3 lines 0 comments Download
Lib/ntpath.py View 1 2 3 2 chunks +67 lines, -1 line 0 comments Download
Lib/posixpath.py View 1 2 3 2 chunks +48 lines, -1 line 0 comments Download
Lib/test/test_ntpath.py View 1 2 3 1 chunk +69 lines, -0 lines 0 comments Download
Lib/test/test_posixpath.py View 1 2 3 1 chunk +54 lines, -0 lines 0 comments Download

Messages

Total messages: 8
storchaka_gmail.com
http://bugs.python.org/review/10395/diff/6507/Lib/posixpath.py File Lib/posixpath.py (right): http://bugs.python.org/review/10395/diff/6507/Lib/posixpath.py#newcode481 Lib/posixpath.py:481: raise ValueError('All paths must be absolute') I think this ...
6 years, 7 months ago #1
rafik
http://bugs.python.org/review/10395/diff/6507/Lib/posixpath.py File Lib/posixpath.py (right): http://bugs.python.org/review/10395/diff/6507/Lib/posixpath.py#newcode481 Lib/posixpath.py:481: raise ValueError('All paths must be absolute') On 2012/11/04 17:39:05, ...
6 years, 7 months ago #2
storchaka_gmail.com
ntpath tests needed. http://bugs.python.org/review/10395/diff/6518/Doc/library/os.path.rst File Doc/library/os.path.rst (right): http://bugs.python.org/review/10395/diff/6518/Doc/library/os.path.rst#newcode65 Doc/library/os.path.rst:65: Availability: Unix, Windows. (It has not ...
6 years, 7 months ago #3
eric.araujo
http://bugs.python.org/review/10395/diff/6518/Doc/library/os.path.rst File Doc/library/os.path.rst (right): http://bugs.python.org/review/10395/diff/6518/Doc/library/os.path.rst#newcode65 Doc/library/os.path.rst:65: Mac OS 9 hasn’t been supported for many versions ...
6 years, 7 months ago #4
storchaka_gmail.com
http://bugs.python.org/review/10395/diff/6518/Doc/library/os.path.rst File Doc/library/os.path.rst (right): http://bugs.python.org/review/10395/diff/6518/Doc/library/os.path.rst#newcode65 Doc/library/os.path.rst:65: On 2012/11/06 16:20:32, eric.araujo wrote: > Mac OS 9 ...
6 years, 7 months ago #5
rafik
http://bugs.python.org/review/10395/diff/6518/Lib/posixpath.py File Lib/posixpath.py (right): http://bugs.python.org/review/10395/diff/6518/Lib/posixpath.py#newcode508 Lib/posixpath.py:508: return prefix + join(*common).rstrip(sep) On 2012/11/06 15:45:05, storchaka wrote: ...
6 years, 7 months ago #6
storchaka_gmail.com
Good. http://bugs.python.org/review/10395/diff/6574/Lib/test/test_ntpath.py File Lib/test/test_ntpath.py (right): http://bugs.python.org/review/10395/diff/6574/Lib/test/test_ntpath.py#newcode269 Lib/test/test_ntpath.py:269: self.assertEquals(ntpath.commonpath(['spam']), 'spam') self.assertEqual
6 years, 7 months ago #7
storchaka_gmail.com
6 years, 7 months ago #8
http://bugs.python.org/review/10395/diff/6518/Lib/posixpath.py
File Lib/posixpath.py (right):

http://bugs.python.org/review/10395/diff/6518/Lib/posixpath.py#newcode508
Lib/posixpath.py:508: return prefix + join(*common).rstrip(sep)
On 2012/11/13 01:17:12, rafik wrote:
> On 2012/11/06 15:45:05, storchaka wrote:
> > Why not sep.join(*common)?
> 
> Using rstrip is the easiest way I found for enforcing the "no trailing slash"
> rule on the returned path. sep.join(['usr', 'lib', '']) gives 'usr/lib/'

join(*'/usr//lib'.split('/')) gives 'usr/lib', '/'.join('/usr//lib'.split('/'))
gives '/usr//lib'.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+