From 7f8b997d5c3f4fa99d5718a29a70c99af6da80b5 Mon Sep 17 00:00:00 2001 From: Jeff Williams Date: Tue, 11 Jul 2017 15:48:49 -0700 Subject: [PATCH] cleanup --- lib/jsdoc/path.js | 40 +++++++++++++++++++++------------------- test/specs/jsdoc/path.js | 33 +++++++++++++++++---------------- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/lib/jsdoc/path.js b/lib/jsdoc/path.js index 61a37d51..ec464795 100644 --- a/lib/jsdoc/path.js +++ b/lib/jsdoc/path.js @@ -86,8 +86,8 @@ exports.commonPrefix = function(paths) { * Retrieve the fully qualified path to the requested resource. * * Plugins and templates will be found somewhat similar to how `require()` works, except that the - * directory in which the JSDoc configuration file is will be considered, too, the JSDoc package - * directory will be considered as a fallback, and a globally installed resource won't be found + * directory in which the JSDoc configuration file is will be considered, too; the JSDoc package's + * directory will be considered as a fallback; and a globally installed resource won't be found * unless it comes with JSDoc. * * If the resource path is specified as a path relative to module or package (starting with `.` or @@ -106,19 +106,20 @@ exports.commonPrefix = function(paths) { * Includes the filename if one was provided. */ exports.getResourcePath = function(filepath, filename) { - var pathElems; var result = null; var searchDirs = []; - function pathExists(_path) { + function directoryExists(dir) { + var stats; + try { - fs.readdirSync(_path); + stats = fs.statSync(dir); + + return stats.isDirectory(); } catch (e) { return false; } - - return true; } // resources that are installed modules may not have been specified with a filepath @@ -126,18 +127,16 @@ exports.getResourcePath = function(filepath, filename) { filepath = filename; filename = undefined; } - pathElems = filepath.split(path.sep); // Special case `node_modules/foo`, to accommodate this legacy workaround advertised by - // third-party plugin and template authors - if (pathElems[0] === 'node_modules') { - pathElems.unshift('.'); - filepath = pathElems.join(path.sep); + // third-party plugin and template authors. + if ( /^node_modules\//.test(filepath) ) { + filepath = path.join('.', filepath); } // search in different sets of directories depending on whether filepath is expressly relative // to "current" directory or not - searchDirs = pathElems[0].indexOf('.') === 0 ? + searchDirs = /^\./.test(filepath) ? // look first in "current" (where JSDoc was executed), then in directory of config, and // _only then_ in JSDoc's directory [env.pwd, path.dirname(env.opts.configure || ''), env.dirname] : @@ -147,14 +146,17 @@ exports.getResourcePath = function(filepath, filename) { path.join(path.dirname(env.opts.configure || ''), 'node_modules'), env.dirname]; - // absolute paths are normalized by path.resolve on the first pass - searchDirs.forEach(function(_path) { - if (!result && _path) { - _path = path.resolve(_path, filepath); - if ( pathExists(_path) ) { - result = _path; + searchDirs.some(function(dir) { + if (dir) { + dir = path.resolve(dir, filepath); + if ( directoryExists(dir) ) { + result = dir; + + return true; } } + + return false; }); if (result) { diff --git a/test/specs/jsdoc/path.js b/test/specs/jsdoc/path.js index c5ceb2ba..f79bf5d4 100644 --- a/test/specs/jsdoc/path.js +++ b/test/specs/jsdoc/path.js @@ -146,56 +146,57 @@ describe('jsdoc/path', function() { it('resolves package-relative path that exists', function() { var resolved = path.getResourcePath('plugins'); - expect( resolved ).not.toBeNull(); + expect(resolved).not.toBeNull(); expect( path.isAbsolute(resolved) ).toBe(true); }); it('fails to resolve package-relative path that exists in ./', function() { var resolved = path.getResourcePath('util'); - expect( resolved ).toBeNull(); + expect(resolved).toBeNull(); }); it('resolves relative to ./ path that exists', function() { + // `path.join` discards the `.`, so we join with `path.sep` instead var p = ['.', 'util'].join(path.sep); var resolved = path.getResourcePath(p); - expect( resolved ).not.toBeNull(); + expect(resolved).not.toBeNull(); expect( path.isAbsolute(resolved) ).toBe(true); }); it('resolves relative to ../ path that exists', function() { - var p = ['..', 'jsdoc', 'util'].join(path.sep); + var p = path.join('..', 'jsdoc', 'util'); var resolved = path.getResourcePath(p); - expect( resolved ).not.toBeNull(); + expect(resolved).not.toBeNull(); expect( path.isAbsolute(resolved) ).toBe(true); }); it('resolves relative to ../ path that exists in ../ and package', function() { - var prel = ['..', 'plugins'].join(path.sep); + var prel = path.join('..', 'plugins'); var pabs = 'plugins'; var resolved = path.getResourcePath(prel); - expect( resolved ).not.toBeNull(); + expect(resolved).not.toBeNull(); expect( path.getResourcePath(pabs) ).not.toBeNull(); expect( path.isAbsolute(resolved) ).toBe(true); - expect( path.getResourcePath(pabs) ).not.toBe( resolved ); + expect( path.getResourcePath(pabs) ).not.toBe(resolved); }); it('resolves relative to . path that exists in package', function() { - var p = ['.', 'plugins'].join(path.sep); + var p = path.join('.', 'plugins'); var resolved = path.getResourcePath(p); - expect( resolved ).not.toBeNull(); + expect(resolved).not.toBeNull(); expect( path.isAbsolute(resolved) ).toBe(true); }); it('resolves path using node_modules/', function() { - var p = ['node_modules', 'marked'].join(path.sep); + var p = path.join('node_modules', 'marked'); var resolved = path.getResourcePath( path.dirname(p), path.basename(p) ); - expect( resolved ).not.toBeNull(); + expect(resolved).not.toBeNull(); expect( path.isAbsolute(resolved) ).toBe(true); }); @@ -203,16 +204,16 @@ describe('jsdoc/path', function() { var p = 'marked'; var resolved = path.getResourcePath(undefined, p); - expect( resolved ).not.toBeNull(); + expect(resolved).not.toBeNull(); expect( path.isAbsolute(resolved) ).toBe(true); }); it('leaves an absolute path as is', function() { - var p = path.resolve([env.dirname, 'anything'].join(path.sep)); + var p = path.resolve( path.join(env.dirname, 'anything') ); var resolved = path.getResourcePath(path.dirname(p), 'anything'); - expect( resolved ).not.toBeNull(); - expect( p ).toEqual( resolved ); + expect(resolved).not.toBeNull(); + expect(p).toBe(resolved); }); }); });