Fix os.path.commonprefix to work by directory components

[smontanaro]

BPO	400788
Nosy	@gvanrossum, @smontanaro
Files	None: None

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/smontanaro'
closed_at = <Date 2000-07-12.16:58:52.000>
created_at = <Date 2000-07-09.15:32:38.000>
labels = []
title = 'Fix os.path.commonprefix to work by directory components'
updated_at = <Date 2000-07-12.16:58:52.000>
user = 'https://github.com/smontanaro'

bugs.python.org fields:

activity = <Date 2000-07-12.16:58:52.000>
actor = 'skip.montanaro'
assignee = 'skip.montanaro'
closed = True
closed_date = None
closer = None
components = ['None']
creation = <Date 2000-07-09.15:32:38.000>
creator = 'skip.montanaro'
dependencies = []
files = ['2562']
hgrepos = []
issue_num = 400788
keywords = ['patch']
message_count = 7.0
messages = ['33137', '33138', '33139', '33140', '33141', '33142', '33143']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'effbot', 'skip.montanaro']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue400788'
versions = []

[smontanaro]

[effbot]

splitting on os.sep isn't good enough for Windows (it supports both "/" and "\"). the correct way would be to split on either os.sep or os.altsep (if not None), or to normalize the path's before comparing them.

except for this, I'm +0 on this one.

[gvanrossum]

Why use n - copy.copy(m)? It's much simpler to use n = m[:].

Note that dospath probably also has this. And macpath should have one.

I'm +1 on fixing the functionality if it's indeed broken as Skip reports; haven't looked at the code in detail (except I noted the import copy).

[smontanaro]

Guido asked:

> Why use n - copy.copy(m)? It's much simpler to use n = m[:].

Just a brain fart. I will upload a revised patch.

I will also try splitting on both os.sep and os.altsep as Fredrik indicated.

[smontanaro]

This version of the patch tries splitting with os.sep then
if that doesn't create a length > 1 list tries os.altsep.
It also adds the same code to ntpath.py, dospath.py and macpath.py. Someone needs to test them!

[gvanrossum]

Check it in -- you already checked in the changes to the test!

I propose to use m[:] instead of copy.copy(m), and not import the copy module.

[smontanaro]

checked in