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

Support for other languages #2

Closed
wants to merge 3 commits into from
Closed

Support for other languages #2

wants to merge 3 commits into from

Conversation

nknapp
Copy link

@nknapp nknapp commented May 2, 2015

This is a pull-request for issue #1.
I've added an options parameter to the extract-function. This parameter is an object that can have a filename-property. If no filename is set, abc.js will be assumed to cover backwards compatibility.
I've used the comment-patterns module to determine the comment-patterns for the programming language determined by the filename.
Tests are green, including my new test-case for a Handlebars-file, and I have included a small example in the .verb.md template (but not in the README).

I have not tested any other languages than JavaScript and Handlebars.

I have not grasped Extracting from files-example in the readme yet. Otherwise I would have adapted it as well.

Finally, I guess having three commits for this issue is not nice as well, so if you comment on my commits, I will combine these three with additional changes to be made and resubmit a pull-request containing just one commit.
Also I would publish comment-patterns as version 1.0.0.
It would be nice if you had a look.

@jonschlinkert
Copy link
Owner

haven't had a chance to review the pr in detail yet, just wanted to say thanks and let you know I didn't forget!

@jonschlinkert
Copy link
Owner

also about the optimization stuff you mentioned, think we can try to do that before merging (if you haven't)? one of the goals of this project is to be fast. fwiw, I also have a lib that extracts comments with esprima, but it's much slower so I always use this one.

Another thing we can do to ensure that the lookups only happen once, regardless of how you're looping, is to catch the results on the first lookup.

var cache = {};

function foo(lang) {
  return cache[lang] || (cache[lang] = lookup(lang));
}

@nknapp
Copy link
Author

nknapp commented May 3, 2015

I've added pre-compiled indexes "nameMatcher"-> "index within the database-array" to the project. The files are not in the repository, but generated on prepublish and maintained in one js-file per language.

Lookup of a language (for a .js-file) is now equivalent to base[byMatcher['.js']]

@nknapp
Copy link
Author

nknapp commented May 5, 2015

The more I think about it... I think I can come up with a better solution to incorporate comment-patterns into extract-comments.

@nknapp
Copy link
Author

nknapp commented May 15, 2015

I would consider this pull-request obsolete. I'll make a new one for discussion of a slightly different approach

@nknapp nknapp closed this May 15, 2015
@nknapp nknapp mentioned this pull request May 17, 2015
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.

2 participants