-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add support for file: syntax #20
base: master
Are you sure you want to change the base?
Conversation
Not needed now that bit-docs makes all file: paths absolute: bit-docs/bit-docs#26
build/static_dist.js
Outdated
for(var depName in siteConfig.html.dependencies) { | ||
var possibleFilePath = siteConfig.dependencies[depName]; | ||
|
||
if (_.startsWith(possibleFilePath,'file:')) { |
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.
Any reason to use Lodash instead of startsWith
or indexOf
?
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'm sorry, I don't understand what you're asking. I literally wanted to know what the code is saying: Does it start with this string or not, and lodash was already required in the file. Otherwise I would have used indexOf
.
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.
Sorry, I should’ve been more clear; any reason not to use the native String APIs here? Not a big deal, just asking if there’s a reason to use Lodash.
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.
For readability it's good.
e9486ea
to
8be8b34
Compare
Is there a reason that plugins registering themselves with the bit-docs/bit-docs#25 (comment) If plugins were expected to use If it doesn't break anything, I suggest we change the plugins that need to load front-end JS/Less through |
Before this PR, the file:
Would have {
"name": "bit-docs-site",
"version": "0.0.1",
"description": "A site to be built",
"main": "static.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "Bitovi",
"license": "MIT",
"dependencies": {
"steal": "0.16.X",
"bit-docs-html-toc": "0.6.2",
"bit-docs-prettify": "0.1.1",
"bit-docs-html-highlight-line": "0.2.3",
"bit-docs-tag-demo": "0.3.0"
},
"system": {
"npmAlgorithm": "flat"
}
} After this PR (or, alternatively, using
{
"name": "bit-docs-site",
"version": "0.0.1",
"description": "A site to be built",
"main": "static.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "Bitovi",
"license": "MIT",
"dependencies": {
"steal": "0.16.X",
"bit-docs-html-toc": "file:\/Users\/leoj\/bitovi\/,+\/bd\/gm\/bit-docs-website\/docs\/modules\/bit-docs-html-toc",
"bit-docs-prettify": "file:\/Users\/leoj\/bitovi\/,+\/bd\/gm\/bit-docs-website\/docs\/modules\/bit-docs-prettify",
"bit-docs-html-highlight-line": "file:\/Users\/leoj\/bitovi\/,+\/bd\/gm\/bit-docs-website\/docs\/modules\/bit-docs-html-highlight-line",
"bit-docs-tag-demo": "file:\/Users\/leoj\/bitovi\/,+\/bd\/gm\/bit-docs-website\/docs\/modules\/bit-docs-tag-demo"
},
"system": {
"npmAlgorithm": "flat"
}
} Note: We would probably want to use |
Regardless of supporting Currently, for getting an arbitrary plugin's front-end code into the generated website, The problem is that all plugins currently specify an actual version number like One solution is to have the plugin specify its bit-docs/bit-docs-js#4 However, there may be a better option, as pointed out by @justinbmeyer, that involves passing the plugin name and version to the plugin's https://github.com/bit-docs/bit-docs/blob/master/lib/configure/add-package-configuration.js#L8 That would essentially be the same as the |
To use However, I have thought of another and very important reason to support It has to do with the case when someone specifies a GitHub repository as the version string in the So, for a website "bit-docs": {
"dependencies": {
"bit-docs-dev": "bit-docs/bit-docs-dev",
"bit-docs-generate-html": "bit-docs/bit-docs-generate-html",
"bit-docs-glob-finder": "bit-docs/bit-docs-glob-finder",
"my-cool-plugin": "dude/my-cool-plugin",
"bit-docs-js": "bit-docs/bit-docs-js"
}, We most definitely need to use something like |
Note that in conversation I've had with Justin he agrees this is a usability win for end users, but it isn't "correct". Using |
Some commits of this PR depend on bit-docs/bit-docs#26 being merged.
Alternative is to use
__dirname
in the plugins'bit-docs.js
: bit-docs/bit-docs#25Useful if you want to have
bit-docs
use local copies of plugins:Notice that the
file:
dependencies will not pull from GitHub like the others, but now will instead use the local copy thru and thru. In this case, items indocs/modules
are git submodules.The changes in this PR are necessary specifically for working with local
file:
plugins that register themselves to thehtml
hook (a hook that is handled bybit-docs-generate-html
).Such a plugins are:
Without this PR and when using
file:
to include them, any changes to files such as:Won't come through, because currently
bit-docs-generate-html
will download from remote npm.