Skip to content

Fix pattern matching in .vscodeignore #271

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

Merged
joaomoreno merged 2 commits into from
Aug 3, 2018
Merged

Fix pattern matching in .vscodeignore #271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58c7d55
Fix pattern matching in .vscodeignore
JimiC Jul 3, 2018
4cc3e83
Apply requested changes
JimiC Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ function compile() {
const api = ts.dts
.pipe(filter('**/api.d.ts'));

const resources = gulp.src('src/**')
.pipe(filter(['**', '!**/*.ts']));
const resources = gulp.src('src/**', { dot: true })
.pipe(filter(['**', '!**/*.ts'], { dot: true }));
Copy link
Copy Markdown
Member

@joaomoreno joaomoreno Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually change anything?

Copy link
Copy Markdown
Contributor Author

@JimiC JimiC Aug 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is crucial, otherwise minimatch does not match dotted files and folders, resulting in .vscodeignore file and .vscode folder not getting included in the compiled output, thus the relative test fails.


return es.merge(js, api, resources)
.pipe(gulp.dest('out'));
Expand Down
13 changes: 11 additions & 2 deletions src/package.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ const readFile = denodeify<string, string, string>(fs.readFile);
const unlink = denodeify<string, void>(fs.unlink as any);
const exec = denodeify<string, { cwd?: string; env?: any; }, { stdout: string; stderr: string; }>(cp.exec as any, (err, stdout, stderr) => [err, { stdout, stderr }]);
const glob = denodeify<string, _glob.Options, string[]>(_glob);
const lstat = denodeify<string, fs.Stats>(fs.lstat);

const resourcesPath = path.join(path.dirname(__dirname), 'resources');
const vsixManifestTemplatePath = path.join(resourcesPath, 'extension.vsixmanifest');
const contentTypesTemplatePath = path.join(resourcesPath, '[Content_Types].xml');

const MinimatchOptions = { dot: true };
const MinimatchOptions: minimatch.Options = { dot: true };

export interface IFile {
path: string;
Expand Down Expand Up @@ -701,7 +702,15 @@ function collectFiles(cwd: string, useYarn = false): Promise<string[]> {

return readFile(path.join(cwd, '.vscodeignore'), 'utf8')
.catch<string>(err => err.code !== 'ENOENT' ? Promise.reject(err) : Promise.resolve(''))
.then(rawIgnore => rawIgnore.split(/[\n\r]/).map(s => s.trim()).filter(s => !!s))
.then(rawIgnore => rawIgnore.split(/[\n\r]/).filter(s => !!s).map(s => s.trim().replace(/\/$/, '')))
.then(ignore => {
const promises = ignore.map(i => {
return lstat(i)
.catch<fs.Stats>(() => Promise.resolve())
.then(stat => stat && stat.isDirectory() ? `${i}/**` : i);
});
return Promise.all(promises).then<string[]>(i => Promise.resolve(i));
})
Copy link
Copy Markdown
Member

@joaomoreno joaomoreno Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there really not a better way to do this without disk access?

Copy link
Copy Markdown
Contributor Author

@JimiC JimiC Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of another way to determine if an entry is a directory or a file. Suggestions are welcome.

.then(ignore => [...defaultIgnore, ...ignore, '!package.json'])
.then(ignore => ignore.filter(i => !/^\s*#/.test(i)))
.then(ignore => _.partition(ignore, i => !/^\s*!/.test(i)))
Expand Down
3 changes: 3 additions & 0 deletions src/test/fixtures/vscodeignore/.vscode/tasks.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"tasks": [],
}
8 changes: 8 additions & 0 deletions src/test/fixtures/vscodeignore/.vscodeignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# pattern of dotted directory without backslash
.vscode

# pattern of directory with trailing backslash
out/

# pattern of file name
*.log
1 change: 1 addition & 0 deletions src/test/fixtures/vscodeignore/logger.log
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
log
6 changes: 6 additions & 0 deletions src/test/fixtures/vscodeignore/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "uuid",
"publisher": "joaomoreno",
"version": "1.0.0",
"engines": { "vscode": "*" }
}
10 changes: 10 additions & 0 deletions src/test/package.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,16 @@ describe('collect', function () {
});
});

it('should ignore content of .vscodeignore', () => {
const cwd = fixture('vscodeignore');

return readManifest(cwd)
.then(manifest => collect(manifest, { cwd }))
.then(files => {
assert.equal(files.length, 3);
});
});

it('should ignore devDependencies', () => {
const cwd = fixture('devDependencies');
return readManifest(cwd)
Expand Down