new rule: requireHyphenBeforeDescription #52
Conversation
| } | ||
|
|
||
| if (tag.description.substring(0, 2) !== '- ') { | ||
| err('Missing hyphen before description', tag.name.loc.column); |
There was a problem hiding this comment.
Not sure about the location for the error.
There was a problem hiding this comment.
Seems like there are no location for description property ;-( Probably better to left here tag.loc for now.
There was a problem hiding this comment.
Doesn't look jscs has any way of testing the position of the error pointer right now? Since all of our tests in jscs and here are checking if error = 0 or 1.
There was a problem hiding this comment.
Can you check location? Something tells me that column should be passed as third parameter and easier to pass here .loc property
hzoo
force-pushed
the
require-hyphen-before-@param-description
branch
from
b4ee9f5
to
e9346f0
Compare
| }).to.throws(/accepted value/i); | ||
| }); | ||
|
|
||
| it('with undefined should throws', function() { |
There was a problem hiding this comment.
test with object named with undefined ;-\
There was a problem hiding this comment.
Haha I copied it from the other ones - I think we need to change it for a lot of them. Only leading-underscore-access.js says it('with object should throws'.
Maybe reword all to - should throw with an object and should throw with undefined.
There was a problem hiding this comment.
@hzoo Agreed. I'm not native english speaker so If you find some mistakes feel free to fix that. This project need a lot of rules to systemize and formalize generic jsdoc stylistic requirements so any ideas are welcome. It needs meat ;-D
Btw. Fix them all here if you can with separated commits, it would be great
I think about configured word in describe. Probably it also should be changed
There was a problem hiding this comment.
Maybe change configured to should throws, and in it blocks left cases: with undefined, with object, and so on?
There was a problem hiding this comment.
Yeah the test cases in jscs itself probably aren't completely consistent but I see a lot of should report on and the describe usually the value of the option like true
There was a problem hiding this comment.
Totally agreed, it would be much better to have describe('should VERB', ... it('VALUE', ... instead of ...configured...with VALUE should VERB
There was a problem hiding this comment.
I'll fix later other tests and will put this note to CONTRIBUTION.md
|
Nice work. ;-D |
hzoo
force-pushed
the
require-hyphen-before-@param-description
branch
from
e9346f0
to
e317b03
Compare
|
Changed the wording for all the tests in |
|
Thanks! Merging? |
|
Yea sounds good. I wonder if jscs could let you check style for stuff like parameter names, and test case strings so it has the same format. |
|
Oh - I didn't change the readme yet - just remembered.. |
…tion new rule: requireHyphenBeforeDescription
|
Nice catch! I'll wait for README changes before publishing new version. Btw, jscs has deprecated rule |
|
Oh yeah I just meant whether it made sense to make a new rule to check for the correct naming like |
|
@hzoo Oh, probably |
#50
This might need some more tests?
I'm assuming most people want the space so its like
- descriptionnot-descriptionso it works like
if (tag.description.substring(0, 2) !== '- ') {