Skip to content

Commit 74cfe73

Browse files
MarshallOfSoundMarshallOfSound
authored
fix: wrap comments with words > 80 chars correctly (#232)
* fix: wrap comments with words > 80 chars correctly Previously we would lose parts of the comment and create a massive multi-line comment instead of just using the whole string. * chore: remove stray log
1 parent 237764b commit 74cfe73

File tree

2 files changed

+95
-70
lines changed

2 files changed

+95
-70
lines changed

‎src/utils.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,21 @@ export const extendArray = <T>(arr1: T[], arr2: T[]): T[] => {
2727
return arr1;
2828
};
2929

30+
const earliest = (a: number, b: number) => {
31+
if (a === -1 && b === -1) return 0;
32+
if (a === -1) return b;
33+
if (b === -1) return a;
34+
return Math.min(a, b);
35+
};
36+
3037
export const wrapComment = (comment: string, additionalTags: DocumentationTag[] = []): string[] => {
3138
if (!comment && !additionalTags.length) return [];
3239
comment = comment.replace(/^\(optional\)(?: - )?/gi, '');
3340
if (!comment && !additionalTags.length) return [];
3441
const result = ['/**'];
3542
while (comment.length > 0) {
36-
let index = 0;
43+
// Default the cut point to be the first "space" or "newline" character
44+
let index = earliest(comment.indexOf(' '), comment.indexOf('\n'));
3745
for (let i = 0; i <= 80; i++) {
3846
if (comment[i] === ' ') index = i;
3947
if (comment[i] === '\n') {
@@ -44,6 +52,11 @@ export const wrapComment = (comment: string, additionalTags: DocumentationTag[]
4452
if (comment.length <= 80 && !comment.includes('\n')) {
4553
index = 80;
4654
}
55+
// If we didn't find a good cut point (i.e. there isn't a good cut point anywhere)
56+
// then let's just take the whole thing it's probably one long word
57+
if (index === 0) {
58+
index = comment.length;
59+
}
4760
result.push(` * ${comment.substring(0, index)}`);
4861
comment = comment.substring(index + 1);
4962
}

‎test/utils_spec.js

Lines changed: 81 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,123 +1,135 @@
1-
const expect = require('chai').expect
2-
const utils = require('../dist/utils')
1+
const expect = require('chai').expect;
2+
const utils = require('../dist/utils');
33

44
describe('utils', () => {
55
describe('extendArray', () => {
66
it('should return an array with all elements added correctly', () => {
7-
expect(utils.extendArray(['foo'], ['bar'])).to.deep.equal(['foo', 'bar'])
8-
})
7+
expect(utils.extendArray(['foo'], ['bar'])).to.deep.equal(['foo', 'bar']);
8+
});
99

1010
it('should return an array with all elements added in the correct oreder', () => {
11-
expect(utils.extendArray([1, 2, 3, 4], [2, 3, 4, 5])).to.deep.equal([1, 2, 3, 4, 2, 3, 4, 5])
12-
})
11+
expect(utils.extendArray([1, 2, 3, 4], [2, 3, 4, 5])).to.deep.equal([1, 2, 3, 4, 2, 3, 4, 5]);
12+
});
1313

1414
it('should mutate the original array', () => {
15-
const primary = [1, 5, 9]
16-
const secondary = [2, 6, 10]
17-
utils.extendArray(primary, secondary)
18-
expect(primary).to.deep.equal([1, 5, 9, 2, 6, 10])
19-
})
20-
})
15+
const primary = [1, 5, 9];
16+
const secondary = [2, 6, 10];
17+
utils.extendArray(primary, secondary);
18+
expect(primary).to.deep.equal([1, 5, 9, 2, 6, 10]);
19+
});
20+
});
2121

2222
describe('wrapComment', () => {
2323
it('should return an array', () => {
24-
expect(utils.wrapComment('Foo Bar')).to.be.a('array')
25-
})
24+
expect(utils.wrapComment('Foo Bar')).to.be.a('array');
25+
});
2626

2727
it('should be a correctly formatted JS multi-line comment', () => {
28-
const wrapped = utils.wrapComment('Foo bar')
29-
expect(wrapped[0]).to.be.equal('/**')
30-
expect(wrapped[wrapped.length - 1]).to.be.equal(' */')
31-
})
28+
const wrapped = utils.wrapComment('Foo bar');
29+
expect(wrapped[0]).to.be.equal('/**');
30+
expect(wrapped[wrapped.length - 1]).to.be.equal(' */');
31+
});
3232

3333
it('should wrap each line to be a max of 80 chars', () => {
34-
const reallyLongString = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec pulvinar nibh eu orci fringilla interdum. In mi arcu, accumsan nec justo eget, pharetra egestas mauris. Quisque nisl tellus, sagittis lobortis commodo nec, tincidunt a arcu. Donec congue lacus a lacus euismod, in hendrerit nunc faucibus. Praesent ac libero eros. Nunc lorem turpis, elementum vel pellentesque vitae, aliquet et erat. In tempus, nulla vitae cursus congue, massa dui pretium eros, eget ornare ipsum diam a velit. Aliquam ac iaculis dui. Phasellus mollis augue volutpat turpis posuere scelerisque. Donec a rhoncus nisl, eu viverra massa. Suspendisse rutrum fermentum diam, posuere tempus turpis accumsan in. Pellentesque commodo in leo vitae aliquet. Vestibulum id justo ac odio mollis fringilla ac a odio. Quisque rhoncus pretium risus, tristique convallis urna.'
35-
const wrapped = utils.wrapComment(reallyLongString)
34+
const reallyLongString =
35+
'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec pulvinar nibh eu orci fringilla interdum. In mi arcu, accumsan nec justo eget, pharetra egestas mauris. Quisque nisl tellus, sagittis lobortis commodo nec, tincidunt a arcu. Donec congue lacus a lacus euismod, in hendrerit nunc faucibus. Praesent ac libero eros. Nunc lorem turpis, elementum vel pellentesque vitae, aliquet et erat. In tempus, nulla vitae cursus congue, massa dui pretium eros, eget ornare ipsum diam a velit. Aliquam ac iaculis dui. Phasellus mollis augue volutpat turpis posuere scelerisque. Donec a rhoncus nisl, eu viverra massa. Suspendisse rutrum fermentum diam, posuere tempus turpis accumsan in. Pellentesque commodo in leo vitae aliquet. Vestibulum id justo ac odio mollis fringilla ac a odio. Quisque rhoncus pretium risus, tristique convallis urna.';
36+
const wrapped = utils.wrapComment(reallyLongString);
3637
wrapped.forEach(line => {
3738
// Subtract 3 due to commend prefix " * "
38-
expect(line.length - 3).to.be.lte(80)
39-
})
40-
})
39+
expect(line.length - 3).to.be.lte(80);
40+
});
41+
});
4142

4243
it('should not split words unless it needs to', () => {
43-
const wrapped = utils.wrapComment('Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword')
44+
const wrapped = utils.wrapComment(
45+
'Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword Thisisalongword',
46+
);
4447
wrapped.forEach((line, index) => {
45-
if (index === 0 || index === wrapped.length - 1) return
46-
expect(line.endsWith('Thisisalongword')).to.eq(true)
47-
})
48-
})
49-
})
48+
if (index === 0 || index === wrapped.length - 1) return;
49+
expect(line.endsWith('Thisisalongword')).to.eq(true);
50+
});
51+
});
52+
53+
it('should handle long urls and not create needless empty lines', () => {
54+
const wrapped = utils.wrapComment(
55+
'Unregisters the app from notifications received from APNS. See: https://developer.apple.com/documentation/appkit/nsapplication/1428747-unregisterforremotenotifications?language=objc',
56+
);
57+
expect(wrapped.length).equal(4);
58+
});
59+
});
5060

5161
describe('typify', () => {
5262
it('should lower case known types', () => {
53-
expect(utils.typify('String')).to.equal('string')
54-
expect(utils.typify('Number')).to.equal('number')
55-
})
63+
expect(utils.typify('String')).to.equal('string');
64+
expect(utils.typify('Number')).to.equal('number');
65+
});
5666

5767
it('should convert specific number types to typescript types', () => {
58-
expect(utils.typify('Integer')).to.equal('number')
59-
expect(utils.typify('Float')).to.equal('number')
60-
expect(utils.typify('Double')).to.equal('number')
61-
expect(utils.typify('Number')).to.equal('number')
62-
})
68+
expect(utils.typify('Integer')).to.equal('number');
69+
expect(utils.typify('Float')).to.equal('number');
70+
expect(utils.typify('Double')).to.equal('number');
71+
expect(utils.typify('Number')).to.equal('number');
72+
});
6373

6474
it('should correctly convert a void function', () => {
65-
expect(utils.typify('VoidFunction')).to.equal('(() => void)')
66-
})
75+
expect(utils.typify('VoidFunction')).to.equal('(() => void)');
76+
});
6777

6878
it('should lower case known array types', () => {
69-
expect(utils.typify('String[]')).to.equal('string[]')
70-
expect(utils.typify('Number[]')).to.equal('number[]')
71-
})
79+
expect(utils.typify('String[]')).to.equal('string[]');
80+
expect(utils.typify('Number[]')).to.equal('number[]');
81+
});
7282

7383
it('should map an array of types through typify as well', () => {
74-
expect(utils.typify(['String', 'Float', 'Boolean'])).to.deep.equal('(string) | (number) | (boolean)')
75-
})
84+
expect(utils.typify(['String', 'Float', 'Boolean'])).to.deep.equal(
85+
'(string) | (number) | (boolean)',
86+
);
87+
});
7688

7789
it('should map an array of types through typify as well and remove duplicates', () => {
78-
expect(utils.typify(['String', 'Float', 'Double'])).to.deep.equal('(string) | (number)')
79-
})
90+
expect(utils.typify(['String', 'Float', 'Double'])).to.deep.equal('(string) | (number)');
91+
});
8092

8193
it('should map node objects to the correct type', () => {
82-
expect(utils.typify('buffer')).to.equal('Buffer')
83-
})
84-
})
94+
expect(utils.typify('buffer')).to.equal('Buffer');
95+
});
96+
});
8597

8698
describe('paramify', () => {
8799
it('should pass through most param names', () => {
88-
expect(utils.paramify('foo')).to.equal('foo')
89-
})
100+
expect(utils.paramify('foo')).to.equal('foo');
101+
});
90102

91103
it('should clean reserved words', () => {
92-
expect(utils.paramify('switch')).to.equal('the_switch')
93-
})
94-
})
104+
expect(utils.paramify('switch')).to.equal('the_switch');
105+
});
106+
});
95107

96108
describe('isEmitter', () => {
97109
it('should return true on most modules', () => {
98-
expect(utils.isEmitter({ name: 'app' })).to.eq(true)
99-
})
110+
expect(utils.isEmitter({ name: 'app' })).to.eq(true);
111+
});
100112

101113
it('should return false for specific non-emitter modules', () => {
102-
expect(utils.isEmitter({ name: 'menuitem' })).to.eq(false)
103-
})
104-
})
114+
expect(utils.isEmitter({ name: 'menuitem' })).to.eq(false);
115+
});
116+
});
105117

106118
describe('isOptional', () => {
107119
it('should return true if param is not required', () => {
108-
expect(utils.isOptional({})).to.eq(true)
109-
})
120+
expect(utils.isOptional({})).to.eq(true);
121+
});
110122

111123
it('should return false if param is required', () => {
112-
expect(utils.isOptional({ required: true })).to.eq(false)
113-
})
124+
expect(utils.isOptional({ required: true })).to.eq(false);
125+
});
114126

115127
it('should default to true if param is a non-function', () => {
116-
expect(utils.isOptional({ type: 'Foo' })).to.eq(true)
117-
})
128+
expect(utils.isOptional({ type: 'Foo' })).to.eq(true);
129+
});
118130

119131
it('should default to false if param is a function', () => {
120-
expect(utils.isOptional({ type: 'Function' })).to.eq(false)
121-
})
122-
})
123-
})
132+
expect(utils.isOptional({ type: 'Function' })).to.eq(false);
133+
});
134+
});
135+
});

0 commit comments

Comments
 (0)