Skip to content
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

TypeDoc now also honors the --exclude option on a list of files #387

Merged
merged 2 commits into from
Feb 13, 2017
Merged

TypeDoc now also honors the --exclude option on a list of files #387

merged 2 commits into from
Feb 13, 2017

Conversation

dploeger
Copy link
Contributor

This pull request lets typedoc honor the --exclude argument also for typedoc calls, that include a list of files. Previously --exclude only worked if you specify a directory.

As one could possibly argue, running typedoc with a list of files already excludes the files we don't want to use, but typedoc seems to also check tsconfig.json for input files and overwrites the list of the provided files.

@kondei
Copy link

kondei commented Feb 9, 2017

I want this to be merged

@aciccarello
Copy link
Collaborator

@kondei Have you tested this PR yourself?

@dploeger
Copy link
Contributor Author

Travis is actually green on this.

Copy link
Collaborator

@aciccarello aciccarello left a comment

Choose a reason for hiding this comment

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

@dploeger Thanks for the fix! Could you please add a few more tests? I gave one suggestion but any more you can think of to help sort through the open issues regarding exclude would be helpful.


Assert.equal(expanded.indexOf(Path.join(inputFiles, 'class.ts')), -1);
Assert.equal(expanded.indexOf(inputFiles), -1);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add a few more test cases? There are quite a few GitHub issues regarding exclude so better testing will help us sort out what problems actually exist. For example, could you add a test with multiple excludes (see #170 comment)?

@@ -272,7 +272,7 @@ export class Application extends ChildableComponent<Application, AbstractCompone
file = Path.resolve(file);
if (FS.statSync(file).isDirectory()) {
add(file);
} else {
} else if (exclude && !exclude.match(file)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just wanted to note that #395 suggested there was a problem here. Thanks for the fix!

@dploeger
Copy link
Contributor Author

Done. I've scanned through the exclude-issues, and could also only think about the multiple excludes test.

@blakeembrey blakeembrey merged commit 6ddb7f5 into TypeStrong:master Feb 13, 2017
@dploeger
Copy link
Contributor Author

Thanks for the merge. When will this be available in an official release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants