-
Notifications
You must be signed in to change notification settings - Fork 766
General correction of glob patterns and config file locating #742
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
Changes from 13 commits
ed379ea
668d104
cd7970d
3c80f10
ac777fc
7728227
4227851
4b9ca6c
a6bb62d
b9d45ef
28c5e22
f98996b
a234012
bc3b235
2a6557f
91bc957
2cf1f5f
34b7a4c
e5ed318
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import * as Path from 'path'; | ||
| import { isArray } from 'util'; | ||
| import { Minimatch, IMinimatch } from 'minimatch'; | ||
|
|
||
| function convertToMinimatch(pattern: string): IMinimatch { | ||
| // Ensure uniform absolute path cross OS | ||
| // (POSIX would resolve c:/path to /path/to/repo/c:/path without this check) | ||
| if (Path.isAbsolute(pattern) && Path.sep === '/') { | ||
| pattern = pattern.replace(/^\w:/, ''); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the drive letter is removed, does this break access to other drives (e.g.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No it shouldn't. You could argue that the check is redundant, as people probably wouldn't use the drive notation anyway. But I am just so used to try and foresee stupid errors that I added it. Let me know if you think we keep it or not.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, thanks for explaining. We can leave it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just an update on the UNIX pathing system. I asked around and for UNIX systems (Linxus + MAC) you change drive by typing something like |
||
| } | ||
|
|
||
| if (pattern[0] !== '*') { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be checking for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm... yeah you might be right... I will correct it. |
||
| // pattern path is resolved even if it is an absolute path, | ||
| // to ensure correct format for the current OS | ||
| pattern = Path.resolve(pattern); | ||
| } | ||
|
|
||
| // Unify the path slashes before creating the minimatch, for more relyable matching | ||
| return new Minimatch(pattern.replace(/[\\]/g, '/'), { dot: true }); | ||
| } | ||
|
|
||
| export function createMinimatch(pattern: string[]): IMinimatch[]; | ||
| export function createMinimatch(pattern: string): IMinimatch; | ||
| export function createMinimatch(pattern: string | string[]): IMinimatch | IMinimatch[] { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since all usages are
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok... if you prefer, I can remove the overloads, but as the function is used two places already (which is why I created it in the first place), so I prefer to keep the function it self to avoid having duplicate the code in the code base.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay, that sounds good to me |
||
| return isArray(pattern) | ||
| ? pattern.map(convertToMinimatch) | ||
| : convertToMinimatch(pattern); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import * as Path from 'path'; | ||
| import { Minimatch } from 'minimatch'; | ||
|
|
||
| import isEqual = require('lodash/isEqual'); | ||
| import Assert = require('assert'); | ||
|
|
||
| import { createMinimatch } from '..'; | ||
|
|
||
| // Used to ensure uniform path cross OS | ||
| const absolutePath = (path: string) => Path.resolve(path.replace(/^\w:/, '')).replace(/[\\]/g, '/'); | ||
|
|
||
| describe('Paths', () => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for writing tests!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure thing |
||
| describe('createMinimatch', () => { | ||
| it('Converts a path to a Minimatch expression', () => { | ||
| const mm = createMinimatch('/some/path/**'); | ||
| Assert(mm instanceof Minimatch, 'Path not coverted to Minimatch'); | ||
| }); | ||
|
|
||
| it('Converts an array of paths to an array of Minimatch expressions', () => { | ||
| const mms = createMinimatch(['/some/path/**', '**/another/path/**', './relative/**/path']); | ||
| Assert(Array.isArray(mms), 'Didn\'t return an array'); | ||
|
|
||
| const allAreMm = mms.every((mm) => mm instanceof Minimatch); | ||
| Assert(allAreMm, 'Not all paths are coverted to Minimatch'); | ||
| }); | ||
|
|
||
| it('Minimatch can match absolute paths expressions', () => { | ||
| const paths = ['/unix/absolute/**/path', '\\windows\\alternative\\absolute\\path', 'C:\\Windows\\absolute\\*\\path', '**/arbitrary/path/**']; | ||
| const mms = createMinimatch(paths); | ||
| const patterns = mms.map(({ pattern }) => pattern); | ||
| const comparePaths = [ | ||
| absolutePath('/unix/absolute/**/path'), | ||
| absolutePath('/windows/alternative/absolute/path'), | ||
| absolutePath('/Windows/absolute/*/path'), | ||
| '**/arbitrary/path/**' | ||
| ]; | ||
|
|
||
| Assert(isEqual(patterns, comparePaths), `Patterns have been altered:\nMMS: ${patterns}\nPaths: ${comparePaths}`); | ||
|
|
||
| Assert(mms[0].match(absolutePath('/unix/absolute/some/sub/dir/path')), 'Din\'t match unix path'); | ||
| Assert(mms[1].match(absolutePath('/windows/alternative/absolute/path')), 'Din\'t match windows alternative path'); | ||
| Assert(mms[2].match(absolutePath('/Windows/absolute/test/path')), 'Din\'t match windows path'); | ||
| Assert(mms[3].match(absolutePath('/some/deep/arbitrary/path/leading/nowhere')), 'Din\'t match arbitrary path'); | ||
| }); | ||
|
|
||
| it('Minimatch can match relative to the project root', () => { | ||
| const paths = ['./relative/**/path', '../parent/*/path', 'no/dot/relative/**/path/*', '.dot/relative/**/path/*']; | ||
| const absPaths = paths.map((path) => absolutePath(path)); | ||
| const mms = createMinimatch(paths); | ||
| const patterns = mms.map(({ pattern }) => pattern); | ||
|
|
||
| Assert(isEqual(patterns, absPaths), `Project root have not been added to paths:\nMMS: ${patterns}\nPaths: ${absPaths}`); | ||
|
|
||
| Assert(mms[0].match(Path.resolve('relative/some/sub/dir/path')), 'Din\'t match relative path'); | ||
| Assert(mms[1].match(Path.resolve('../parent/dir/path')), 'Din\'t match parent path'); | ||
| Assert(mms[2].match(Path.resolve('no/dot/relative/some/sub/dir/path/test')), 'Din\'t match no dot path'); | ||
| Assert(mms[3].match(Path.resolve('.dot/relative/some/sub/dir/path/test')), 'Din\'t match dot path'); | ||
| }); | ||
|
|
||
| it('Minimatch matches dot files', () => { | ||
| const mm = createMinimatch('/some/path/**'); | ||
| Assert(mm.match(absolutePath('/some/path/.dot/dir')), 'Didn\'t match .dot path'); | ||
| Assert(mm.match(absolutePath('/some/path/normal/dir')), 'Didn\'t match normal path'); | ||
| }); | ||
| }); | ||
| }); | ||
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 think that this line is responsible for making typedoc 0.14 a breaking change for my projects.
The path that I use for my config file is something like
node_modules/my-config-project/config/typedoc.config.jsonThis worked fine in 0.13 but breaks in 0.14.
I now need to update the path on 60+ projects..