Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Make preset more useful #109

Closed
sindresorhus opened this issue Dec 27, 2013 · 26 comments
Closed

Make preset more useful #109

sindresorhus opened this issue Dec 27, 2013 · 26 comments
Assignees
Labels
Milestone

Comments

@sindresorhus
Copy link
Contributor

re #54

As it stands now preset isn't very useful as it only has the jquery style. Seeing as the config is fairly verbose it would be nice to have some ways to reuse it.

Would be useful if preset could have a searchpath in this order:

  1. Check it with fs.exists if it's a valid path to a preset file
  2. Try to require a node module with that name. Super useful for being able to distribute presets. Since "main" in package.json can be a .json file it could be the preset and requiring the module, would give you the preset config object.
  3. the built-in preset.
@sindresorhus
Copy link
Contributor Author

Relevant JSHint feature: jshint/jshint#1314

@markelog
Copy link
Member

How would one differentiate between real node module and built-in preset config? How about check for the built-in first?

Also not sure about the first step, we could easily check it presence through require method itself.

Browser version should still be able to work with built-in presets, so it might require to disperse preset logic between string-checker and checker modules.

@sindresorhus
Copy link
Contributor Author

How would one differentiate between real node module and built-in preset config? How about check for the built-in first?

Sure, you shouldn't name your own preset the same as the built-in ones anyway.

Also not sure about the first step, we could easily check it presence through require method itself.

Sure, was just a quick braindump.

@mdevils
Copy link
Member

mdevils commented Dec 30, 2013

I suggest using another option name: presetPath. So preset option can be used in StringChecker and presetPath in Checker.

@mikesherov
Copy link
Contributor

I suggest we look at the way PHPCS handles this with standards. Basically, a standard is a reference to a subdirectory of the standards directory. The subdir contains two things:

  1. It's own config file.
  2. A set of custom rules unique to standard.

You can reference rules in your own config by use namespaces:

{
 'jQuery.requireMultiLineChain' : true
}

You can also include multiple full presets. Including presets also runs the config of the preset. Multiple presets are defined in an array with conflicting rules being resolved by having the last declaration win. You can override those values on your own of course.

{
  'preset': [
    'jQuery',
    'node' 
  ],
  'node.customRule' : false,
  'someBasicRuleSetInThePresets': false
}

All standards are defined

@mikesherov
Copy link
Contributor

And to echo what @sindresorhus said, we can also have NPM module lookups. The point I was making is simply about what a preset is, and how to include one (or many!) in your config.

@mikesherov
Copy link
Contributor

Can we get this done in 1.12? /cc @mdevils @zxqfox seems we just need to do the original suggestion and can defer the question of loading multiple presets together to a different issue at a different time.

I'll assign @zxqfox for now just so someone owns this.

@mikesherov mikesherov modified the milestones: 1.12, 1.later Feb 2, 2015
@markelog
Copy link
Member

markelog commented Feb 3, 2015

Can take this one if @zxqfox don't mind

@qfox
Copy link
Member

qfox commented Feb 3, 2015

@markelog Feel free! I wanted to have deep extension here if possible: jscs-dev/jscs-jsdoc#83 . There were idea to split google (with closurecompiler specific tags), wiki (with other set), etc. To mix jscs and plugin presets.

@mikesherov
Copy link
Contributor

Yeah, sounds good to me too. Just trying to get old issues addressed.

@qfox
Copy link
Member

qfox commented Feb 3, 2015

I agree that we can solve original issue (according to current roadmap) and make another issue for future. Just need to know what we should solve here.

@mikesherov
Copy link
Contributor

Ability to load a preset by use of require so that presets can be distributed via npm.

@qfox
Copy link
Member

qfox commented Feb 3, 2015

@mikesherov We already have plugins functionality and it's pretty simple to make it: https://github.com/zxqfox/jscs-bem/blob/master/lib/index.js

I can suggest defining "jscs" or "jscs-plugin" right in package.json with "presets" section if we need something more but it looks pretty complex.

{
  "jscs-plugin": {
    "presets": ["hey-ho", "lets-go"]
  }
}

We already have package.json usage: https://github.com/jscs-dev/node-jscs/blob/master/test/data/configs/mixedWithPkg/package.json

@markelog
Copy link
Member

markelog commented Feb 3, 2015

@zxqfox preset option might be simple alternative to the plugin interface, so it would be easy to share such configs without digging into the API.

But i agree, it looks like creating plugin is easy too.

@mikesherov thoughts?
@sindresorhus would love to hear your opinion about that too.

@qfox
Copy link
Member

qfox commented Feb 3, 2015

@markelog I'm not against some sugar for presets. Feels like it would be very helpful for preset makers.

@markelog
Copy link
Member

markelog commented Feb 3, 2015

I'm not against some sugar for presets. Feels like it would be very helpful for preset makers.

It might be, or it might be duplicative. Not sure

@mikesherov
Copy link
Contributor

My opinion is the same as before, making a preset should be as simple as creating json file and publishing it to NPM.

@qfox
Copy link
Member

qfox commented Feb 4, 2015

So... Consumer's config (.jscsrc file or something) will be the same as before:

{
  "preset": "somepreset"
}

And "somepreset" will be searched on:

  1. Installed node_modules list;
  2. Registered presets list (by plugins or core).

Publisher should name his NPM-package with jscs-preset- (or jscs- + special flag in package.json, to differentiate with usual plugins), create JSON, and publish these 2 files (package.json, and his somepreset.json). I'm not sure which field will aim to preset.json, there is "main" by default, but we can make "jscs-presets" array field to register them all.

upd Here's a caveat with cwd, project root, configs, and testing files. But better to solve this later.

@markelog @mikesherov Please correct me if I wrong.

@markelog And feel free to take this ;-) I'll be back in a couple days.

@mikesherov
Copy link
Contributor

I would think the publisher would name her package jscs-my-wonderful-preset and in .jscsrc put:

{
  preset: "jscs-my-wonderful-preset"
}

This way, there is no magic.

But I'm also fine if we want JSCS to assume jscs-preset- prepended to the name so that the publisher of package jscs-preset-behance could just put the following in .jscsrc:

{
  preset: "behance"
}

@mrjoelkemp
Copy link
Member

I'd modify Mike's example to avoid mentioning jscs all together, just to show that the prefix is unimportant:

{
  preset: "my-npm-module-name"
}

Where my-npm-module-name could be whatever you'd require the module as (ex: require('my-npm-module-name'). We'll try to register the custom preset and if it doesn't adhere to the code standard for custom presets, then it'll fail. Otherwise, it should pass without a problem.

@mikesherov
Copy link
Contributor

@mrjoelkemp yes, that's what I meant in my first example.

@qfox
Copy link
Member

qfox commented Feb 4, 2015

@mrjoelkemp In that case we require some jscs-preset/-plugin field in NPM package.json file as well as main field for main js file. Also we can use some default file name value like jscs-preset.js (like index.js for main). I just don't want to have inconsistencies in package.json usage, but I think we'll fix them at review state.

Okay. I think we need make something to make this talk subjective.

@markelog
Copy link
Member

@zxqfox i'd like to take over this one with #1083.

@phanect
Copy link

phanect commented Apr 1, 2015

@markelog Is this function implemented?
And available on latest release?

@markelog
Copy link
Member

markelog commented Apr 2, 2015

Nope, proposed pull for it, considered as not back-compat, so only in 2.0 :-(

@phanect
Copy link

phanect commented Apr 4, 2015

@markelog OK, thanks.
Then we can get this feature on next major release.

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

No branches or pull requests

7 participants