-
Notifications
You must be signed in to change notification settings - Fork 80
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
Rethink plugin config #141
Comments
Idea 1: use different plugin keysInstead of just dumping plugins into
That would make: /* docpress.json */
"transforms": {
"docpress-jsdoc": {}, /* hypothetical support for .js comments */
},
"generator": {
"docpress-base": {},
}
"addons": {
"docpress-search": {},
"docpress-disqus": {},
"docpress-edit-in-github": {}
} Pros:
Cons:
Pending questions:
|
Instead of: "addons": {
"docpress-search": {},
"docpress-disqus": {},
"docpress-edit-in-github": {}
} maybe a better markup could be an array and just providing specific options when is necessary: "addons": [
"docpress-search",
{ "docpress-disqus": { shortname: 'my-shortname' }},
"docpress-edit-in-github"
] I have a similar plugin pipeline at bumped and I decide use CSON. Also is possible provide a JSON file but with CSON it lloks more legible: 'addons': [
'docpress-search'
{ 'docpress-disqus': shortname: 'my-shortname' }
'docpress-edit-in-github'
] |
that can work. I chose the object notation because that's the convention metalsmith uses (example). In the spirit of keeping up with the Metalsmith community, I'd like to continue support for it. (We can support both object notation and array notation.) For the array syntax, I think it's better for options to be passed through array tuples, like so: "addons": [
"docpress-search",
[ "docpress-disqus", { shortname: 'my-shortname' } ],
"docpress-edit-in-github"
] This is browserify's convention, and I think it's sensible because it even allows you to specify multiple options to pass if need be. |
I added you to the docpress org by the way. Thanks for all your help! |
I like browserify convention; I understand your support to metalsmith convention but maybe for this case don't have much sense pass a empty object the major part of time. |
I think the argument for the Metalsmith convention is that it's easier to parse (not just by MS but by other things as well), and the shape is uniform without any "smart" rules involved. The array convention is also used by Babel, which typically turns into a mess when auto-formatted:
Nonetheless I'm cool with both; they're both widely-used conventions (metalsmith/postcss on the object notation, browserify/babel on the array convention). |
what do you all think about the |
Btw, in the future {
"generators": [
"docpress-base", /* html website */
["docpress-epub": { filename: "myapp.epub" } ],
["docpress-pdf": { filename: "myapp.pdf" } ]
}
} |
(And I just realized |
I like the rename to docpress-html. Where would themes go? generators or addons? |
Haha. I'd add another It'd be in |
Might be a pain to try and override the existing css in |
Hm, probably not. Here's how I'd approach a totally custom theme:
Of course I'd also refactor docpress-html to have its stylus a bit more easily-extensible (separate it into multiple .styl files so people can cherry-pick what they wanna use) |
Ah, sounds good. We'd need to document some of this once it has been implemented. |
Yeah, and I'll make https://github.com/rstacruz/docpress-rsc to be an official theme ( |
Docpress is simply metalsmith with 2 plugins: docpress-core and docpress-base.
You can add plugins with the
plugins
configuration:This convention is not ideal. As a new user, my first instinct is just add
docpress-disqus
to the plugins list, but that's not the case. You need to repeatdocpress-core
anddocpress-base
in there.My first instinct is to configure it this way because:
docpress-base
, it should be easy to remove itdocpress-base
(or-core
), so it should be possible to do thatNo idea what the best plugin API would be. Opening the discussion here on that.
(ps: @Kikobeats what's your npm email?)
The text was updated successfully, but these errors were encountered: