Handle platform specific files in plugin #588
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| ///<reference path="../.d.ts"/> | ||
| "use strict"; | ||
| import path = require("path"); | ||
| import util = require("util"); | ||
|
|
||
| export class ProjectFilesManager implements IProjectFilesManager { | ||
| constructor(private $fs: IFileSystem, | ||
| private $platformsData: IPlatformsData) { } | ||
|
|
||
| public processPlatformSpecificFiles(directoryPath: string, platform: string, excludedDirs?: string[]): IFuture<void> { | ||
| return (() => { | ||
| var contents = this.$fs.readDirectory(directoryPath).wait(); | ||
| var files: string[] = []; | ||
|
|
||
| _.each(contents, fileName => { | ||
| let filePath = path.join(directoryPath, fileName); | ||
| let fsStat = this.$fs.getFsStats(filePath).wait(); | ||
| if(fsStat.isDirectory() && !_.contains(excludedDirs, fileName)) { | ||
| this.processPlatformSpecificFilesCore(platform, this.$fs.enumerateFilesInDirectorySync(filePath)).wait(); | ||
| } else if(fsStat.isFile()) { | ||
| files.push(filePath); | ||
| } | ||
| }); | ||
| this.processPlatformSpecificFilesCore(platform, files).wait(); | ||
|
There was a problem hiding this comment. Maybe we can improve the performance here: let futures: IFuture<void>[];
_.each(contents, fileName => {
let filePath = path.join(directoryPath, fileName);
let fsStat = this.$fs.getFsStats(filePath).wait();
if(fsStat.isDirectory() && !_.contains(excludedDirs, fileName)) {
future.push(this.processPlatformSpecificFilesCore(platform, this.$fs.enumerateFilesInDirectorySync(filePath)));
} else if (fsStat.isFile()) {
files.push(filePath);
}
return Future.fromResult();
});
futures.push(this.processPlatformSpecificFilesCore(platform, files));
Future.wait(futures);I've not tested it and this is not a merge stopper. There was a problem hiding this comment. after discussion, we've decided to keep the current code |
||
|
|
||
| }).future<void>()(); | ||
| } | ||
|
|
||
| private processPlatformSpecificFilesCore(platform: string, files: string[]): IFuture<void> { | ||
| // Renames the files that have `platform` as substring and removes the files from other platform | ||
| return (() => { | ||
| _.each(files, fileName => { | ||
|
There was a problem hiding this comment. Also not a merge stopper, but consider: let futures = files.map(fileName => {
let platformInfo = ProjectFilesManager.parsePlatformSpecificFileName(path.basename(fileName), this.$platformsData.platformsNames);
let shouldExcludeFile = platformInfo && platformInfo.platform !== platform;
if (shouldExcludeFile) {
return this.$fs.deleteFile(fileName);
} else if (platformInfo && platformInfo.onDeviceName) {
return this.$fs.rename(fileName, path.join(path.dirname(fileName), platformInfo.onDeviceName));
}
return Future.fromResult();
});
Future.wait(futures);There was a problem hiding this comment. after discussion, we've decided to keep the current code |
||
| var platformInfo = ProjectFilesManager.parsePlatformSpecificFileName(path.basename(fileName), this.$platformsData.platformsNames); | ||
| var shouldExcludeFile = platformInfo && platformInfo.platform !== platform; | ||
| if (shouldExcludeFile) { | ||
| this.$fs.deleteFile(fileName).wait(); | ||
| } else if (platformInfo && platformInfo.onDeviceName) { | ||
| this.$fs.rename(fileName, path.join(path.dirname(fileName), platformInfo.onDeviceName)).wait(); | ||
| } | ||
| }); | ||
| }).future<void>()(); | ||
| } | ||
|
|
||
| private static parsePlatformSpecificFileName(fileName: string, platforms: string[]): any { | ||
| var regex = util.format("^(.+?)\\.(%s)(\\..+?)$", platforms.join("|")); | ||
| var parsed = fileName.match(new RegExp(regex, "i")); | ||
| if (parsed) { | ||
| return { | ||
| platform: parsed[2], | ||
| onDeviceName: parsed[1] + parsed[3] | ||
| }; | ||
| } | ||
| return undefined; | ||
| } | ||
| } | ||
| $injector.register("projectFilesManager", ProjectFilesManager); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| /// <reference path=".d.ts" /> | ||
| "use strict"; | ||
|
|
||
| import yok = require('../lib/common/yok'); | ||
| import fsLib = require("../lib/common/file-system"); | ||
| import projectFilesManagerLib = require("../lib/services/project-files-manager"); | ||
| import hostInfoLib = require("../lib/common/host-info"); | ||
| import StaticConfigLib = require("../lib/config"); | ||
| import ErrorsLib = require("../lib/common/errors"); | ||
| import path = require("path"); | ||
| import temp = require("temp"); | ||
| temp.track(); | ||
|
|
||
| var assert = require("chai").assert; | ||
|
|
||
| function createTestInjector() { | ||
| let testInjector = new yok.Yok(); | ||
| testInjector.register("fs", fsLib.FileSystem); | ||
| testInjector.register("hostInfo", hostInfoLib.HostInfo); | ||
| testInjector.register("staticConfig", StaticConfigLib.StaticConfig); | ||
| testInjector.register("projectFilesManager", projectFilesManagerLib.ProjectFilesManager); | ||
| testInjector.register("errors", ErrorsLib.Errors); | ||
| testInjector.register("platformsData", { | ||
| platformsNames: ["ios", "android"] | ||
| }); | ||
|
|
||
| return testInjector; | ||
| } | ||
|
|
||
| function createFiles(testInjector: IInjector, filesToCreate: string[]): IFuture<string> { | ||
| return (() => { | ||
| let fs = testInjector.resolve("fs"); | ||
| let directoryPath = temp.mkdirSync("Project Files Manager Tests"); | ||
|
|
||
| _.each(filesToCreate, file => fs.writeFile(path.join(directoryPath, file), "").wait()); | ||
|
|
||
| return directoryPath; | ||
| }).future<string>()(); | ||
| } | ||
|
|
||
| describe('Project Files Manager Tests', () => { | ||
| let testInjector: IInjector, projectFilesManager: IProjectFilesManager; | ||
| beforeEach(() => { | ||
| testInjector = createTestInjector(); | ||
| projectFilesManager = testInjector.resolve("projectFilesManager"); | ||
| }); | ||
| it("filters android specific files", () => { | ||
| let files = ["test.ios.x", "test.android.x"]; | ||
| let directoryPath = createFiles(testInjector, files).wait(); | ||
|
|
||
| projectFilesManager.processPlatformSpecificFiles(directoryPath, "android").wait(); | ||
|
|
||
| let fs = testInjector.resolve("fs"); | ||
| assert.isFalse(fs.exists(path.join(directoryPath, "test.ios.x")).wait()); | ||
| assert.isTrue(fs.exists(path.join(directoryPath, "test.x")).wait()); | ||
| assert.isFalse(fs.exists(path.join(directoryPath, "test.android.x")).wait()); | ||
| }); | ||
| it("filters ios specific files", () => { | ||
| let files = ["index.ios.html", "index1.android.html", "a.test"]; | ||
| let directoryPath = createFiles(testInjector, files).wait(); | ||
|
|
||
| projectFilesManager.processPlatformSpecificFiles(directoryPath, "ios").wait(); | ||
|
|
||
| let fs = testInjector.resolve("fs"); | ||
| assert.isFalse(fs.exists(path.join(directoryPath, "index1.android.html")).wait()); | ||
| assert.isFalse(fs.exists(path.join(directoryPath, "index1.html")).wait()); | ||
| assert.isTrue(fs.exists(path.join(directoryPath, "index.html")).wait()); | ||
| assert.isTrue(fs.exists(path.join(directoryPath, "a.test")).wait()); | ||
| }); | ||
| it("doesn't filter non platform specific files", () => { | ||
| let files = ["index1.js", "index2.js", "index3.js"]; | ||
| let directoryPath = createFiles(testInjector, files).wait(); | ||
|
|
||
| projectFilesManager.processPlatformSpecificFiles(directoryPath, "ios").wait(); | ||
|
|
||
| let fs = testInjector.resolve("fs"); | ||
| assert.isTrue(fs.exists(path.join(directoryPath, "index1.js")).wait()); | ||
| assert.isTrue(fs.exists(path.join(directoryPath, "index2.js")).wait()); | ||
| assert.isTrue(fs.exists(path.join(directoryPath, "index3.js")).wait()); | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add unit tests.