diff --git a/keymaps/git.cson b/keymaps/git.cson index 6fd6175699..17c774719d 100644 --- a/keymaps/git.cson +++ b/keymaps/git.cson @@ -54,3 +54,16 @@ '.github-Dialog input': 'enter': 'core:confirm' + +'.github-CommitView-coAuthorEditor': + 'enter': 'github:co-author:enter' + 'down': 'github:co-author:down' + 'up': 'github:co-author:up' + 'tab': 'github:co-author:tab' + 'backspace': 'github:co-author:backspace' + 'escape': 'github:co-author:escape' + 'pageup': 'github:co-author:pageup' + 'pagedown': 'github:co-author:pagedown' + 'home': 'github:co-author:home' + 'end': 'github:co-author:end' + 'delete': 'github:co-author:delete' diff --git a/lib/controllers/commit-controller.js b/lib/controllers/commit-controller.js index 120822d2ff..78254d1c41 100644 --- a/lib/controllers/commit-controller.js +++ b/lib/controllers/commit-controller.js @@ -31,6 +31,10 @@ export default class CommitController extends React.Component { stagedChangesExist: PropTypes.bool.isRequired, lastCommit: PropTypes.object.isRequired, currentBranch: PropTypes.object.isRequired, + mentionableUsers: PropTypes.arrayOf(PropTypes.shape({ + email: PropTypes.string.isRequired, + name: PropTypes.string.isRequired, + })).isRequired, prepareToCommit: PropTypes.func.isRequired, commit: PropTypes.func.isRequired, @@ -110,6 +114,7 @@ export default class CommitController extends React.Component { onChangeMessage={this.handleMessageChange} toggleExpandedCommitMessageEditor={this.toggleExpandedCommitMessageEditor} deactivateCommitBox={!!this.getCommitMessageEditors().length > 0} + mentionableUsers={this.props.mentionableUsers} /> ); } @@ -137,16 +142,27 @@ export default class CommitController extends React.Component { } @autobind - async commit(message) { + async commit(message, coAuthors = []) { + + let msg; if (this.getCommitMessageEditors().length > 0) { - await this.props.commit({filePath: this.getCommitMessagePath()}); + msg = this.getCommitMessageEditors()[0].getText(); } else { - let formattedMessage = message; - if (this.props.config.get('github.automaticCommitMessageWrapping')) { - formattedMessage = wrapCommitMessage(formattedMessage); - } - await this.props.commit(formattedMessage); + const wrapMessage = this.props.config.get('github.automaticCommitMessageWrapping'); + msg = wrapMessage ? wrapCommitMessage(message) : message; + } + + if (coAuthors.length) { + // TODO: ensure that expanded editor commit functionality still works + const trailers = coAuthors.map(author => { + return { + token: 'Co-Authored-By', + value: `${author.name} <${author.email}>`, + }; + }); + msg = await this.props.repository.addTrailersToCommitMessage(msg, trailers); } + return this.props.commit(msg.trim()); } getCommitMessage() { diff --git a/lib/controllers/git-tab-controller.js b/lib/controllers/git-tab-controller.js index 51eee44bf0..5394ffe97d 100644 --- a/lib/controllers/git-tab-controller.js +++ b/lib/controllers/git-tab-controller.js @@ -9,6 +9,7 @@ import yubikiri from 'yubikiri'; import GitTabView from '../views/git-tab-view'; import ObserveModelDecorator from '../decorators/observe-model'; +import UserStore from '../models/user-store'; import {nullBranch} from '../models/branch'; import {nullCommit} from '../models/commit'; @@ -103,6 +104,15 @@ export default class GitTabController extends React.Component { this.lastFocus = GitTabView.focus.STAGING; this.refView = null; + + this.state = {mentionableUsers: []}; + + this.userStore = new UserStore({ + repository: this.props.repository, + onDidUpdate: users => { + this.setState({mentionableUsers: users}); + }, + }); } serialize() { @@ -132,6 +142,7 @@ export default class GitTabController extends React.Component { mergeConflicts={this.props.mergeConflicts} workingDirectoryPath={this.props.workingDirectoryPath} mergeMessage={this.props.mergeMessage} + mentionableUsers={this.state.mentionableUsers} resolutionProgress={this.props.resolutionProgress} workspace={this.props.workspace} @@ -171,6 +182,15 @@ export default class GitTabController extends React.Component { componentWillReceiveProps(newProps) { this.refreshResolutionProgress(false, false); + + if (this.props.repository !== newProps.repository) { + this.userStore = new UserStore({ + repository: newProps.repository, + onDidUpdate: users => { + this.setState({mentionableUsers: users}); + }, + }); + } } componentWillUnmount() { diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 859606fad1..09c8414dae 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -581,7 +581,7 @@ export default class GitShellOutStrategy { // There's probably a better way. I tried finding a regex to do it in one fell swoop but had no luck const coAuthors = body.split(LINE_ENDING_REGEX).reduce((emails, line) => { - const match = line.match(/\s*Co-authored-by: .*<(.*)>\s*/); + const match = line.match(/^co-authored-by: .*<(.*)>\s*/i); if (match && match[1]) { emails.push(match[1]); } return emails; }, []); @@ -598,6 +598,65 @@ export default class GitShellOutStrategy { return commits; } + async getAuthors(options = {}) { + const {max, ref} = {max: 1, ref: 'HEAD', ...options}; + + // https://git-scm.com/docs/git-log#_pretty_formats + // %x1F - field separator byte + // %an - author name + // %ae - author email + // %cn - committer name + // %ce - committer email + // %(trailers:unfold,only) - the commit message trailers, separated + // by newlines and unfolded (i.e. properly + // formatted and one trailer per line). + + const delimiter = '1F'; + const delimiterString = String.fromCharCode(parseInt(delimiter, 16)); + const fields = ['%an', '%ae', '%cn', '%ce', '%(trailers:unfold,only)']; + const format = fields.join(`%x${delimiter}`); + + try { + const output = await this.exec([ + 'log', `--format=${format}`, '-z', '-n', max, ref, '--', + ]); + + return output.split('\0') + .reduce((acc, line) => { + if (line.length === 0) { return acc; } + + const [an, ae, cn, ce, trailers] = line.split(delimiterString); + trailers + .split('\n') + .map(trailer => trailer.match(/^co-authored-by. (.+?) <(.+?)>$/i)) + .filter(match => match !== null) + .forEach(([_, name, email]) => { acc[email] = name; }); + + acc[ae] = an; + acc[ce] = cn; + + return acc; + }, {}); + } catch (err) { + if (/unknown revision/.test(err.stdErr) || /bad revision 'HEAD'/.test(err.stdErr)) { + return []; + } else { + throw err; + } + } + } + + mergeTrailers(commitMessage, trailers, unfold) { + const args = ['interpret-trailers']; + if (unfold) { + args.push('--unfold'); + } + for (const trailer of trailers) { + args.push('--trailer', `${trailer.token}=${trailer.value}`); + } + return this.exec(args, {stdin: commitMessage}); + } + readFileFromIndex(filePath) { return this.exec(['show', `:${toGitPathSep(filePath)}`]); } diff --git a/lib/models/repository-states/present.js b/lib/models/repository-states/present.js index 28b07bfeb6..d1f871d31c 100644 --- a/lib/models/repository-states/present.js +++ b/lib/models/repository-states/present.js @@ -151,6 +151,7 @@ export default class Present extends State { keys.add(Keys.recentCommits); keys.add(Keys.statusBundle); keys.add(Keys.headDescription); + keys.add(Keys.authors); continue; } @@ -159,6 +160,7 @@ export default class Present extends State { keys.add(Keys.lastCommit); keys.add(Keys.recentCommits); keys.add(Keys.headDescription); + keys.add(Keys.authors); continue; } @@ -262,6 +264,15 @@ export default class Present extends State { }, message, options); } + async addTrailersToCommitMessage(rawMessage, trailers) { + // Ensure that message ends with newline for git-interpret trailers to work + const message = `${rawMessage}\n`.replace(/\s+$/, '\n'); + + return trailers !== undefined && trailers.length > 0 + ? await this.git().mergeTrailers(message, trailers) + : message; + } + // Merging @invalidate(() => [ @@ -307,6 +318,7 @@ export default class Present extends State { Keys.stagedChangesSinceParentCommit, Keys.lastCommit, Keys.recentCommits, + Keys.authors, Keys.statusBundle, Keys.index.all, ...Keys.filePatch.eachWithOpts({staged: true, amending: true}), @@ -611,6 +623,17 @@ export default class Present extends State { }); } + // Author information + + getAuthors(options) { + // For now we'll do the naive thing and invalidate anytime HEAD moves. This ensures that we get new authors + // introduced by newly created commits or pulled commits. + // This means that we are constantly re-fetching data. If performance becomes a concern we can optimize + return this.cache.getOrSet(Keys.authors, () => { + return this.git().getAuthors(options); + }); + } + // Branches getBranches() { @@ -965,6 +988,8 @@ const Keys = { recentCommits: new CacheKey('recent-commits'), + authors: new CacheKey('authors'), + branches: new CacheKey('branches'), headDescription: new CacheKey('head-description'), @@ -1010,6 +1035,7 @@ const Keys = { Keys.stagedChangesSinceParentCommit, Keys.lastCommit, Keys.recentCommits, + Keys.authors, Keys.statusBundle, ], }; diff --git a/lib/models/repository-states/state.js b/lib/models/repository-states/state.js index db17bb9ffa..196dc0790e 100644 --- a/lib/models/repository-states/state.js +++ b/lib/models/repository-states/state.js @@ -178,6 +178,11 @@ export default class State { return unsupportedOperationPromise(this, 'commit'); } + @shouldDelegate + addTrailersToCommitMessage(message, trailers) { + return unsupportedOperationPromise(this, 'addTrailersToCommitMessage'); + } + // Merging @shouldDelegate @@ -352,6 +357,13 @@ export default class State { return Promise.resolve([]); } + // Author information + + @shouldDelegate + getAuthors() { + return Promise.resolve({}); + } + // Branches @shouldDelegate diff --git a/lib/models/repository.js b/lib/models/repository.js index 6aa356c799..0e3f0dcc9a 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -256,6 +256,7 @@ const delegates = [ 'applyPatchToWorkdir', 'commit', + 'addTrailersToCommitMessage', 'merge', 'abortMerge', @@ -292,6 +293,8 @@ const delegates = [ 'getLastCommit', 'getRecentCommits', + 'getAuthors', + 'getBranches', 'getCurrentBranch', 'getHeadDescription', diff --git a/lib/models/user-store.js b/lib/models/user-store.js new file mode 100644 index 0000000000..532edb5926 --- /dev/null +++ b/lib/models/user-store.js @@ -0,0 +1,63 @@ +// This is a guess about what a reasonable value is. Can adjust if performance is poor. +const MAX_COMMITS = 5000; + +export default class UserStore { + constructor({repository, onDidUpdate}) { + this.repository = repository; + this.repository.onDidUpdate(() => { + this.loadUsers(); + }); + this.onDidUpdate = onDidUpdate || (() => {}); + // TODO: [ku 3/2018] Consider using Dexie (indexDB wrapper) like Desktop and persist users across sessions + this.users = {}; + this.populate(); + } + + populate() { + if (this.repository.isPresent()) { + this.loadUsers(); + } else { + this.repository.onDidChangeState(({from, to}) => { + if (!from.isPresent() && to.isPresent()) { + this.loadUsers(); + } + }); + } + } + + loadUsers() { + this.loadUsersFromLocalRepo(); + } + + async loadUsersFromLocalRepo() { + const users = await this.repository.getAuthors({max: MAX_COMMITS}); + this.addUsers(users); + } + + addUsersFromGraphQL(response) { + // TODO: [ku 3/2018] also get users from GraphQL API if available. Will need to reshape the data accordingly + // This will get called in relay query renderer callback + // this.addUsers(users); + } + + addUsers(users) { + this.users = {...this.users, ...users}; + this.didUpdate(); + } + + didUpdate() { + this.onDidUpdate(this.getUsers()); + } + + getUsers() { + // TODO: [ku 3/2018] consider sorting based on most recent authors or commit frequency + // This is obviously not the most performant. Test out on repo with many users to see if we need to optimize + return Object.keys(this.users) + .map(email => ({email, name: this.users[email]})) + .sort((a, b) => { + if (a.name < b.name) { return -1; } + if (a.name > b.name) { return 1; } + return 0; + }); + } +} diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index 1d60ab0aca..8fd762ebec 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import {CompositeDisposable} from 'event-kit'; import {autobind} from 'core-decorators'; import cx from 'classnames'; +import Select from 'react-select'; import Tooltip from './tooltip'; import AtomTextEditor from './atom-text-editor'; @@ -10,6 +11,13 @@ import {shortenSha} from '../helpers'; const LINE_ENDING_REGEX = /\r?\n/; +class FakeKeyDownEvent extends CustomEvent { + constructor(keyCode) { + super('keydown'); + this.keyCode = keyCode; + } +} + export default class CommitView extends React.Component { static focus = { EDITOR: Symbol('commit-editor'), @@ -33,6 +41,7 @@ export default class CommitView extends React.Component { deactivateCommitBox: PropTypes.bool.isRequired, maximumCharacterLimit: PropTypes.number.isRequired, message: PropTypes.string.isRequired, + mentionableUsers: PropTypes.arrayOf(PropTypes.object).isRequired, commit: PropTypes.func.isRequired, abortMerge: PropTypes.func.isRequired, @@ -45,7 +54,12 @@ export default class CommitView extends React.Component { constructor(props, context) { super(props, context); - this.state = {showWorking: false}; + this.state = { + showWorking: false, + selectedCoAuthors: [], + showCoAuthorInput: false, + }; + this.timeoutHandle = null; this.subscriptions = new CompositeDisposable(); @@ -54,6 +68,22 @@ export default class CommitView extends React.Component { this.refAmendCheckbox = null; this.refHardWrapButton = null; this.refAbortMergeButton = null; + this.refCoAuthorSelect = null; + } + + proxyKeyCode(keyCode) { + return e => { + if (!this.refCoAuthorSelect) { + return; + } + + const fakeEvent = new FakeKeyDownEvent(keyCode); + this.refCoAuthorSelect.handleKeyDown(fakeEvent); + + if (!fakeEvent.defaultPrevented) { + e.abortKeyBinding(); + } + }; } componentWillMount() { @@ -63,6 +93,17 @@ export default class CommitView extends React.Component { this.props.commandRegistry.add('atom-workspace', { 'github:commit': this.commit, 'github:toggle-expanded-commit-message-editor': this.toggleExpandedCommitMessageEditor, + + 'github:co-author:down': this.proxyKeyCode(40), + 'github:co-author:up': this.proxyKeyCode(38), + 'github:co-author:enter': this.proxyKeyCode(13), + 'github:co-author:tab': this.proxyKeyCode(9), + 'github:co-author:backspace': this.proxyKeyCode(8), + 'github:co-author:pageup': this.proxyKeyCode(33), + 'github:co-author:pagedown': this.proxyKeyCode(34), + 'github:co-author:end': this.proxyKeyCode(35), + 'github:co-author:home': this.proxyKeyCode(36), + 'github:co-author:delete': this.proxyKeyCode(46), }), this.props.config.onDidChange('github.automaticCommitMessageWrapping', () => this.forceUpdate()), ); @@ -98,6 +139,17 @@ export default class CommitView extends React.Component { didChange={this.didChangeCommitMessage} didChangeCursorPosition={this.didMoveCursor} /> + + this.refCoAuthorToggle} + title="Toggle co-authors" + />