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.

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

check for windows-style root paths #2

Merged
merged 3 commits into from
Mar 10, 2016

Conversation

aggieben
Copy link
Contributor

Windows root paths don't look like '/', but rather 'C:'. This change will allow the function to end it's recursion correctly on Windows. Addresses #1

Windows root paths don't look like '/', but rather 'C:\'.  This change will allow the function to end it's recursion correctly on Windows.
@aggieben
Copy link
Contributor Author

just to clarify: I did this in collusion with @morethanfire

@aggieben
Copy link
Contributor Author

Also, another possible way to do this that might be easier to understand at first glance is something like this:

    function fileExistsWithCaseSync(filepath) {
        var dir = path.dirname(filepath);
        var paths = path.parse(dir);
        //if ((dir === '/' || (os.platform() === 'win32' && dir.split(':')[1] === '\\')) || dir === '.') return;
        if ((paths.dir === paths.root) || dir === '.') return;
        var filenames = fs.readdirSync(dir);
        if (filenames.indexOf(path.basename(filepath)) === - 1) {

            // This could easily be accomplished with various requires, but I want to keep this module thin.
            var lowercasedFilenames = [];
            for (var i = 0; i < filenames.length; i++) {
                lowercasedFilenames.push(filenames[i].toLowerCase());
            }
            var index = lowercasedFilenames.indexOf(path.basename(filepath).toLowerCase());
            return path.join(dir, filenames[index]);
        }
        return fileExistsWithCaseSync(dir);
    }

@Urthen
Copy link
Owner

Urthen commented Mar 10, 2016

Good catch! I don't have a windows dev box so I wasn't able to test that.

I'll double-check this new version and release a patch.

Urthen added a commit that referenced this pull request Mar 10, 2016
@Urthen Urthen merged commit f9dbe36 into Urthen:master Mar 10, 2016
@Urthen Urthen mentioned this pull request Mar 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants