Skip to content

Commit

Permalink
BREAKING CHANGE: changed generator to an HTML based one (#157)
Browse files Browse the repository at this point in the history
*BREAKING CHANGE*

* style(generate): changed generator to an HTML based one [WIP]

I added and set the HTML generator for resolving #154 (which works when testing manually but doesn't
when running the cli in dev mode).

fix #154

* refactor(generate): (re)moved files to keep it DRY

* refactor: refactored/updated tests

Update the contributors test (which wasn't already committed somehow) and tweaked the
`format-contribution-type` test.

* docs(readme): removed doctoc

BREAKING CHANGE: (in 2babe26) The resulting contributors table is in HTML/CSS instead of being in Markdown.

* refactor(generate): removed the style from the generation

Removed the `<style>` block from the generated HTML code as it's redundant on Github (since it's one
of the non-whitelisted tags). The `README.md` was also updated reflecting the breaking changes.

* refactor(generate): image height and tweaks

Added `height` to images for avatars, quoted some `<table>` attributes and updated `README.md`

* docs: drop TOC
  • Loading branch information
Berkmann18 authored and jakebolam committed Feb 6, 2019
1 parent 9ebc25f commit b0c3376
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 60 deletions.
8 changes: 1 addition & 7 deletions README.md

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
"lint": "kcd-scripts lint",
"test": "kcd-scripts test",
"validate": "kcd-scripts validate",
"commit": "npx git-cz"
"commit": "git-cz",
"start": "./dist/cli.js",
"dev": "./src/cli.js"
},
"husky": {
"hooks": {
Expand Down
12 changes: 6 additions & 6 deletions src/contributors/__tests__/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ test('add new contributor at the end of the list of contributors', () => {

return addContributor(options, username, contributions, mockInfoFetcher).then(
contributors => {
expect(contributors.length).toBe(options.contributors.length + 1)
expect(contributors).toHaveLength(options.contributors.length + 1)
expect(contributors[options.contributors.length]).toEqual({
login: 'login3',
name: 'Some name',
Expand All @@ -70,7 +70,7 @@ test('add new contributor at the end of the list of contributors with a url link

return addContributor(options, username, contributions, mockInfoFetcher).then(
contributors => {
expect(contributors.length).toBe(options.contributors.length + 1)
expect(contributors).toHaveLength(options.contributors.length + 1)
expect(contributors[options.contributors.length]).toEqual({
login: 'login3',
name: 'Some name',
Expand Down Expand Up @@ -112,7 +112,7 @@ test(`should update an existing contributor's contributions if a new type is add
const contributions = ['bug']
return addContributor(options, username, contributions, mockInfoFetcher).then(
contributors => {
expect(contributors.length).toBe(options.contributors.length)
expect(contributors).toHaveLength(options.contributors.length)
expect(contributors[0]).toEqual({
login: 'login1',
name: 'Some name',
Expand All @@ -130,7 +130,7 @@ test(`should update an existing contributor's contributions if a new type is add
const contributions = ['bug']
return addContributor(options, username, contributions, mockInfoFetcher).then(
contributors => {
expect(contributors.length).toBe(1)
expect(contributors).toHaveLength(1)
expect(contributors[0]).toEqual({
login: 'Login1',
name: 'Some name',
Expand All @@ -150,7 +150,7 @@ test(`should update an existing contributor's contributions if a new type is add

return addContributor(options, username, contributions, mockInfoFetcher).then(
contributors => {
expect(contributors.length).toBe(options.contributors.length)
expect(contributors).toHaveLength(options.contributors.length)
expect(contributors[0]).toEqual({
login: 'login1',
name: 'Some name',
Expand All @@ -169,7 +169,7 @@ test(`should update an existing contributor's contributions if an existing type

return addContributor(options, username, contributions, mockInfoFetcher).then(
contributors => {
expect(contributors.length).toBe(options.contributors.length)
expect(contributors).toHaveLength(options.contributors.length)
expect(contributors[1]).toEqual({
login: 'login2',
name: 'Some name',
Expand Down
2 changes: 1 addition & 1 deletion src/contributors/__tests__/addWithDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ test('add new contributor without going to the network', async () => {
profile: userDetails.profile,
})

expect(contributors.length).toBe(options.contributors.length + 1)
expect(contributors).toHaveLength(options.contributors.length + 1)
expect(contributors[options.contributors.length]).toEqual(userDetails)
})
2 changes: 1 addition & 1 deletion src/contributors/__tests__/prompt.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ test(`should filter valid contribution types from user inserted types`, () => {
const contributions =
'invalidContributionType1,code,invalidContributionType2,bug'
return prompt(options, username, contributions).then(answers => {
expect(answers.contributions.length).toBe(2)
expect(answers.contributions).toHaveLength(2)
expect(answers.contributions).toEqual(['code', 'bug'])
})
})
7 changes: 2 additions & 5 deletions src/generate/__tests__/__snapshots__/index.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ Description
These people contributed to the project:
<!-- ALL-CONTRIBUTORS-LIST:START -->
<!-- prettier-ignore -->
| Kent C. Dodds is awesome! | Divjot Singh is awesome! | Jeroen Engels is awesome! |
| :---: | :---: | :---: |
<table cellspacing=\\"0\\" cellpadding=\\"1\\"><tr><td>Kent C. Dodds is awesome!</td><td>Divjot Singh is awesome!</td><td>Jeroen Engels is awesome!</td></tr></table>
<!-- ALL-CONTRIBUTORS-LIST:END -->
Thanks a lot everyone!"
Expand All @@ -25,9 +24,7 @@ Description
These people contributed to the project:
<!-- ALL-CONTRIBUTORS-LIST:START -->
<!-- prettier-ignore -->
| Kent C. Dodds is awesome! | Kent C. Dodds is awesome! | Kent C. Dodds is awesome! | Kent C. Dodds is awesome! | Kent C. Dodds is awesome! |
| :---: | :---: | :---: | :---: | :---: |
| Kent C. Dodds is awesome! | Kent C. Dodds is awesome! |
<table cellspacing=\\"0\\" cellpadding=\\"1\\"><tr><td>Kent C. Dodds is awesome!</td><td>Kent C. Dodds is awesome!</td><td>Kent C. Dodds is awesome!</td><td>Kent C. Dodds is awesome!</td><td>Kent C. Dodds is awesome!</td></tr><tr><td>Kent C. Dodds is awesome!</td><td>Kent C. Dodds is awesome!</td></tr></table>
<!-- ALL-CONTRIBUTORS-LIST:END -->
Thanks a lot everyone!"
Expand Down
30 changes: 15 additions & 15 deletions src/generate/__tests__/format-contribution-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ test('return corresponding symbol', () => {
const {options} = fixtures()

expect(formatContributionType(options, contributor, 'tool')).toBe(
'[🔧](#tool-kentcdodds "Tools")',
'<a href="#tool-kentcdodds" title="Tools">🔧</a>',
)
expect(formatContributionType(options, contributor, 'question')).toBe(
'[💬](#question-kentcdodds "Answering Questions")',
'<a href="#question-kentcdodds" title="Answering Questions">💬</a>',
)
})

Expand All @@ -31,21 +31,21 @@ test('return link to commits', () => {
'https://github.com/all-contributors/all-contributors-cli/commits?author=kentcdodds'

expect(formatContributionType(options, contributor, 'code')).toBe(
`[💻](${expectedLink} "Code")`,
`<a href="${expectedLink}" title="Code">💻</a>`,
)
expect(formatContributionType(options, contributor, 'doc')).toBe(
`[📖](${expectedLink} "Documentation")`,
`<a href="${expectedLink}" title="Documentation">📖</a>`,
)
expect(formatContributionType(options, contributor, 'test')).toBe(
`[⚠️](${expectedLink} "Tests")`,
`<a href="${expectedLink}" title="Tests">⚠️</a>`,
)
})

test('return link to issues', () => {
const contributor = contributors.kentcdodds
const {options} = fixtures()
const expected =
'[🐛](https://github.com/all-contributors/all-contributors-cli/issues?q=author%3Akentcdodds "Bug reports")'
'<a href="https://github.com/all-contributors/all-contributors-cli/issues?q=author%3Akentcdodds" title="Bug reports">🐛</a>'

expect(formatContributionType(options, contributor, 'bug')).toBe(expected)
})
Expand All @@ -59,7 +59,7 @@ test('make any symbol into a link if contribution is an object', () => {
}

expect(formatContributionType(options, contributor, contribution)).toBe(
'[🔧](www.foo.bar "Tools")',
'<a href="www.foo.bar" title="Tools">🔧</a>',
)
})

Expand All @@ -72,7 +72,7 @@ test('override url for given types', () => {
}

expect(formatContributionType(options, contributor, contribution)).toBe(
'[💻](www.foo.bar "Code")',
'<a href="www.foo.bar" title="Code">💻</a>',
)
})

Expand All @@ -84,14 +84,14 @@ test('be able to add types to the symbol list', () => {
}

expect(formatContributionType(options, contributor, 'cheerful')).toBe(
'[:smiley:](#cheerful-kentcdodds "")',
'<a href="#cheerful-kentcdodds" title="">:smiley:</a>',
)
expect(
formatContributionType(options, contributor, {
type: 'cheerful',
url: 'www.foo.bar',
}),
).toBe('[:smiley:](www.foo.bar "")')
).toBe('<a href="www.foo.bar" title="">:smiley:</a>')
})

test('be able to add types with template to the symbol list', () => {
Expand All @@ -105,7 +105,7 @@ test('be able to add types with template to the symbol list', () => {
}

expect(formatContributionType(options, contributor, 'web')).toBe(
'[:web:](www.kentcdodds.com "")',
'<a href="www.kentcdodds.com" title="">:web:</a>',
)
})

Expand All @@ -117,14 +117,14 @@ test('be able to override existing types', () => {
}

expect(formatContributionType(options, contributor, 'code')).toBe(
'[:smiley:](#code-kentcdodds "")',
'<a href="#code-kentcdodds" title="">:smiley:</a>',
)
expect(
formatContributionType(options, contributor, {
type: 'code',
url: 'www.foo.bar',
}),
).toBe('[:smiley:](www.foo.bar "")')
).toBe('<a href="www.foo.bar" title="">:smiley:</a>')
})

test('be able to override existing templates', () => {
Expand All @@ -138,14 +138,14 @@ test('be able to override existing templates', () => {
}

expect(formatContributionType(options, contributor, 'code')).toBe(
'[:web:](www.kentcdodds.com "")',
'<a href="www.kentcdodds.com" title="">:web:</a>',
)
expect(
formatContributionType(options, contributor, {
type: 'code',
url: 'www.foo.bar',
}),
).toBe('[:web:](www.foo.bar "")')
).toBe('<a href="www.foo.bar" title="">:web:</a>')
})

test('throw a helpful error on unknown type', () => {
Expand Down
10 changes: 5 additions & 5 deletions src/generate/__tests__/format-contributor.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ test('format a simple contributor', () => {
const {options} = fixtures()

const expected =
'[<img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" alt="Kent C. Dodds"/><br /><sub><b>Kent C. Dodds</b></sub>](http://kentcdodds.com)<br />[👀](#review-kentcdodds "Reviewed Pull Requests")'
'<a href="http://kentcdodds.com"><img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" height="150px;" alt="Kent C. Dodds"/><br /><sub><b>Kent C. Dodds</b></sub></a><br /><a href="#review-kentcdodds" title="Reviewed Pull Requests">👀</a>'

expect(formatContributor(options, contributor)).toBe(expected)
})
Expand All @@ -30,7 +30,7 @@ test('format contributor with complex contribution types', () => {
const {options} = fixtures()

const expected =
'[<img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" alt="Kent C. Dodds"/><br /><sub><b>Kent C. Dodds</b></sub>](http://kentcdodds.com)<br />[📖](https://github.com/all-contributors/all-contributors-cli/commits?author=kentcdodds "Documentation") [👀](#review-kentcdodds "Reviewed Pull Requests") [💬](#question-kentcdodds "Answering Questions")'
'<a href="http://kentcdodds.com"><img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" height="150px;" alt="Kent C. Dodds"/><br /><sub><b>Kent C. Dodds</b></sub></a><br /><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=kentcdodds" title="Documentation">📖</a> <a href="#review-kentcdodds" title="Reviewed Pull Requests">👀</a> <a href="#question-kentcdodds" title="Answering Questions">💬</a>'

expect(formatContributor(options, contributor)).toBe(expected)
})
Expand All @@ -53,7 +53,7 @@ test('default image size to 100', () => {
delete options.imageSize

const expected =
'[<img src="https://avatars1.githubusercontent.com/u/1500684" width="100px;" alt="Kent C. Dodds"/><br /><sub><b>Kent C. Dodds</b></sub>](http://kentcdodds.com)<br />[👀](#review-kentcdodds "Reviewed Pull Requests")'
'<a href="http://kentcdodds.com"><img src="https://avatars1.githubusercontent.com/u/1500684" width="100px;" height="100px;" alt="Kent C. Dodds"/><br /><sub><b>Kent C. Dodds</b></sub></a><br /><a href="#review-kentcdodds" title="Reviewed Pull Requests">👀</a>'

expect(formatContributor(options, contributor)).toBe(expected)
})
Expand All @@ -63,7 +63,7 @@ test('format contributor with pipes in their name', () => {
const {options} = fixtures()

const expected =
'[<img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" alt="Who &#124; Needs &#124; Pipes?"/><br /><sub><b>Who &#124; Needs &#124; Pipes?</b></sub>](http://github.com/chrisinajar)<br />[📖](https://github.com/all-contributors/all-contributors-cli/commits?author=pipey "Documentation")'
'<a href="http://github.com/chrisinajar"><img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" height="150px;" alt="Who &#124; Needs &#124; Pipes?"/><br /><sub><b>Who &#124; Needs &#124; Pipes?</b></sub></a><br /><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=pipey" title="Documentation">📖</a>'

expect(formatContributor(options, contributor)).toBe(expected)
})
Expand All @@ -73,7 +73,7 @@ test('format contributor with no github account', () => {
const {options} = fixtures()

const expected =
'<img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" alt="No Github Account"/><br /><sub><b>No Github Account</b></sub><br />[🌍](#translation "Translation")'
'<img src="https://avatars1.githubusercontent.com/u/1500684" width="150px;" height="150px;" alt="No Github Account"/><br /><sub><b>No Github Account</b></sub><br /><a href="#translation" title="Translation">🌍</a>'

expect(formatContributor(options, contributor)).toBe(expected)
})
2 changes: 1 addition & 1 deletion src/generate/format-contribution-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const _ = require('lodash/fp')
const util = require('../util')

const linkTemplate = _.template(
'[<%= symbol %>](<%= url %> "<%= description %>")',
'<a href="<%= url %>" title="<%= description %>"><%= symbol %></a>',
)

function getType(options, contribution) {
Expand Down
4 changes: 2 additions & 2 deletions src/generate/format-contributor.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ const _ = require('lodash/fp')
const formatContributionType = require('./format-contribution-type')

const avatarTemplate = _.template(
'<img src="<%= contributor.avatar_url %>" width="<%= options.imageSize %>px;" alt="<%= name %>"/>',
'<img src="<%= contributor.avatar_url %>" width="<%= options.imageSize %>px;" height="<%= options.imageSize %>px;" alt="<%= name %>"/>',
)
const avatarBlockTemplate = _.template(
'[<%= avatar %><br /><sub><b><%= name %></b></sub>](<%= contributor.profile %>)',
'<a href="<%= contributor.profile %>"><%= avatar %><br /><sub><b><%= name %></b></sub></a>',
)
const avatarBlockTemplateNoProfile = _.template(
'<%= avatar %><br /><sub><b><%= name %></b></sub>',
Expand Down
23 changes: 7 additions & 16 deletions src/generate/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const _ = require('lodash/fp')
const injectContentBetween = require('../util').markdown.injectContentBetween
const formatBadge = require('./format-badge')
const formatContributor = require('./format-contributor')

Expand Down Expand Up @@ -37,12 +36,7 @@ function injectListBetweenTags(newContent) {
}

function formatLine(contributors) {
return `| ${contributors.join(' | ')} |`
}

function createColumnLine(options, contributors) {
const nbColumns = Math.min(options.contributorsPerLine, contributors.length)
return `${_.repeat(nbColumns, '| :---: ')}|`
return `<td>${contributors.join('</td><td>')}</td>`
}

function generateContributorsList(options, contributors) {
Expand All @@ -52,13 +46,9 @@ function generateContributorsList(options, contributors) {
}),
_.chunk(options.contributorsPerLine),
_.map(formatLine),
function insertColumns(lines) {
const columnLine = createColumnLine(options, contributors)
return injectContentBetween(lines, columnLine, 1, 1)
},
_.join('\n'),
_.join('</tr><tr>'),
newContent => {
return `\n${newContent}\n`
return `\n<table cellspacing="0" cellpadding="1"><tr>${newContent}</tr></table>\n`
},
)(contributors)
}
Expand All @@ -83,7 +73,8 @@ module.exports = function generate(options, contributors, fileContent) {
? '\n'
: generateContributorsList(options, contributors)
const badge = formatBadge(options, contributors)
return _.flow(injectListBetweenTags(contributorsList), replaceBadge(badge))(
fileContent,
)
return _.flow(
injectListBetweenTags(contributorsList),
replaceBadge(badge),
)(fileContent)
}

0 comments on commit b0c3376

Please sign in to comment.