Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix os.path.commonprefix to work by directory components #32556

Closed
smontanaro opened this issue Jul 9, 2000 · 7 comments
Closed

Fix os.path.commonprefix to work by directory components #32556

smontanaro opened this issue Jul 9, 2000 · 7 comments
Assignees
@smontanaro

Comments

@smontanaro
Copy link
Contributor

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
    Copy link
    Contributor Author

    No description provided.

    @smontanaro smontanaro self-assigned this Jul 9, 2000
    @smontanaro smontanaro self-assigned this Jul 9, 2000
    @effbot
    Copy link
    Mannequin

    effbot mannequin commented Jul 9, 2000

    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
    Copy link
    Member

    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
    Copy link
    Contributor Author

    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
    Copy link
    Contributor Author

    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
    Copy link
    Member

    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
    Copy link
    Contributor Author

    checked in

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @smontanaro smontanaro

    Labels
    None yet
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @gvanrossum @smontanaro