Skip to content

Commit

Permalink
Integrate code review
Browse files Browse the repository at this point in the history
- Rename isExcludedPath to isPathExcluded
- Also resolve with null instead of undefined if a resource has been
  excluded
- Rename GLOB to glob as already decided earlier
  • Loading branch information
RandomByte committed May 29, 2019
1 parent 2447359 commit 98db583
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 14 deletions.
4 changes: 2 additions & 2 deletions lib/adapters/AbstractAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AbstractAdapter extends AbstractReaderWriter {
* @public
* @param {Object} parameters Parameters
* @param {string} parameters.virBasePath Virtual base path
* @param {string[]} [parameters.excludes] List of GLOB patterns to exclude
* @param {string[]} [parameters.excludes] List of glob patterns to exclude
*/
constructor({virBasePath, excludes = [], project}) {
if (new.target === AbstractAdapter) {
Expand Down Expand Up @@ -89,7 +89,7 @@ class AbstractAdapter extends AbstractReaderWriter {
* @param {string} virPath Virtual Path
* @returns {boolean} True if path is excluded, otherwise false
*/
isExcludedPath(virPath) {
isPathExcluded(virPath) {
return micromatch(virPath, this._excludes).length > 0;
}

Expand Down
8 changes: 5 additions & 3 deletions lib/adapters/FileSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class FileSystem extends AbstractAdapter {
* @param {Object} parameters Parameters
* @param {string} parameters.virBasePath Virtual base path
* @param {string} parameters.fsBasePath (Physical) File system path
* @param {string[]} [parameters.excludes] List of GLOB patterns to exclude
* @param {string[]} [parameters.excludes] List of glob patterns to exclude
*/
constructor({virBasePath, project, fsBasePath, excludes}) {
super({virBasePath, project, excludes});
Expand Down Expand Up @@ -66,6 +66,8 @@ class FileSystem extends AbstractAdapter {
});
}));
}
// Only glob if the only pattern is an empty string.
// Starting with globby v8 or v9 empty glob patterns "" act like "**"
if (patterns.length > 1 || patterns[0] !== "") {
const matches = await glob(patterns, opt);
for (let i = matches.length - 1; i >= 0; i--) {
Expand Down Expand Up @@ -107,8 +109,8 @@ class FileSystem extends AbstractAdapter {
* @returns {Promise<module:@ui5/fs.Resource>} Promise resolving to a single resource or null if not found
*/
_byPath(virPath, options, trace) {
if (this.isExcludedPath(virPath)) {
return Promise.resolve();
if (this.isPathExcluded(virPath)) {
return Promise.resolve(null);
}

return new Promise((resolve, reject) => {
Expand Down
6 changes: 3 additions & 3 deletions lib/adapters/Memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Memory extends AbstractAdapter {
* @public
* @param {Object} parameters Parameters
* @param {string} parameters.virBasePath Virtual base path
* @param {string[]} [parameters.excludes] List of GLOB patterns to exclude
* @param {string[]} [parameters.excludes] List of glob patterns to exclude
*/
constructor({virBasePath, project, excludes}) {
super({virBasePath, project, excludes});
Expand Down Expand Up @@ -81,8 +81,8 @@ class Memory extends AbstractAdapter {
* @returns {Promise<module:@ui5/fs.Resource>} Promise resolving to a single resource
*/
_byPath(virPath, options, trace) {
if (this.isExcludedPath(virPath)) {
return Promise.resolve();
if (this.isPathExcluded(virPath)) {
return Promise.resolve(null);
}
return new Promise((resolve, reject) => {
if (!virPath.startsWith(this._virBasePath) && virPath !== this._virBaseDir) {
Expand Down
10 changes: 5 additions & 5 deletions lib/resourceFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const resourceFactory = {
* @public
* @callback module:@ui5/fs.resourceFactory~getProjectExcludes
* @param {Object} Project
* @returns {string[]} List of GLOB patterns to exclude
* @returns {string[]} List of glob patterns to exclude
*/

/**
Expand Down Expand Up @@ -50,7 +50,7 @@ const resourceFactory = {
* @param {boolean} [parameters.useNamespaces=false] Use project namespaces as path prefixes
* DEPRECATED: Use parameter <code>getVirtualBasePathPrefix</code> instead.
* @param {module:@ui5/fs.resourceFactory~getProjectExcludes} [parameters.getProjectExcludes]
* Callback to retrieve the exclude GLOBs of a project
* Callback to retrieve the exclude globs of a project
* @param {module:@ui5/fs.resourceFactory~getVirtualBasePathPrefix} [parameters.getVirtualBasePathPrefix]
* Callback to retrieve a prefix for a given virtual base path of a project if required
* @returns {Object} Object containing <code>source</code> and <code>dependencies</code> resource readers
Expand Down Expand Up @@ -167,9 +167,9 @@ const resourceFactory = {
* @param {string} parameters.virBasePath Virtual base path to create the adapter for
* @param {boolean} parameters.useNamespaces Use project namespace as path prefix
* @param {module:@ui5/fs.resourceFactory~getProjectExcludes} [parameters.getProjectExcludes]
* Callback to retrieve the exclude GLOB of a project
* Callback to retrieve the exclude glob of a project
* @param {module:@ui5/fs.resourceFactory~getVirtualBasePathPrefix} [parameters.getVirtualBasePathPrefix]
* Callback to retrieve the exclude GLOB of a project
* Callback to retrieve the exclude glob of a project
* @returns {Promise<string[]>} Promise resolving to list of normalized glob patterns
*/
_createFsAdapterForVirtualBasePath({
Expand Down Expand Up @@ -251,7 +251,7 @@ const resourceFactory = {
* @param {Object} parameters Parameters
* @param {string} parameters.virBasePath Virtual base path
* @param {string} [parameters.fsBasePath] File system base path
* @param {string[]} [parameters.excludes] List of GLOB patterns to exclude
* @param {string[]} [parameters.excludes] List of glob patterns to exclude
* @returns {module:@ui5/fs.adapters.FileSystem|module:@ui5/fs.adapters.Memory} File System- or Virtual Adapter
*/
createAdapter({fsBasePath, virBasePath, project, excludes}) {
Expand Down
6 changes: 5 additions & 1 deletion test/lib/glob.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,11 @@ test("glob with multiple patterns with static exclude", (t) => {
});
});

/* Generic Micromatch tests for understanding GLOB exclude behavior*/
/*
Generic Micromatch tests for understanding glob exclude behavior.
Not active since they only test external code and our adapter-tests should
already test for any incompatible changes.
*/
// test("micromatch exclude test", async (t) => {
// const micromatch = require("micromatch");

Expand Down

0 comments on commit 98db583

Please sign in to comment.