-
Notifications
You must be signed in to change notification settings - Fork 407
Prepare package for bundling in Atom #640
Changes from all commits
f89dbb6
4c9fafa
a0e9951
517e22e
84d82e7
00de30b
fb996e6
d6484f4
30b9b95
d14450f
4266655
75c05e4
29cbf12
64c822d
2120a10
36217ef
d1ffecd
9ad8629
18e3d7e
5af28e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import {Emitter} from 'atom'; | ||
| import {Emitter} from 'event-kit'; | ||
|
|
||
| import url from 'url'; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,19 @@ | ||
| import path from 'path'; | ||
| import os from 'os'; | ||
|
|
||
| import {CompositeDisposable} from 'atom'; | ||
| import {CompositeDisposable} from 'event-kit'; | ||
|
|
||
| import {GitProcess} from 'git-kitchen-sink'; | ||
| import {GitProcess} from 'dugite'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Thanks for getting this upgrade in here 😄 |
||
| import {parse as parseDiff} from 'what-the-diff'; | ||
|
|
||
| import GitPromptServer from './git-prompt-server'; | ||
| import AsyncQueue from './async-queue'; | ||
| import {readFile, fileExists, writeFile, isFileExecutable} from './helpers'; | ||
| import {getPackageRoot, getDugitePath, readFile, fileExists, writeFile, isFileExecutable} from './helpers'; | ||
| import GitTimingsView from './views/git-timings-view'; | ||
|
|
||
| const LINE_ENDING_REGEX = /\r?\n/; | ||
|
|
||
| const GPG_HELPER_PATH = path.resolve(__dirname, '..', 'bin', 'gpg-no-tty.sh'); | ||
| const DUGITE_PATH = require.resolve('git-kitchen-sink'); | ||
| const GPG_HELPER_PATH = path.resolve(getPackageRoot(), 'bin', 'gpg-no-tty.sh'); | ||
|
|
||
| export class GitError extends Error { | ||
| constructor(message) { | ||
|
|
@@ -100,7 +99,7 @@ export default class GitShellOutStrategy { | |
| env.ATOM_GITHUB_SOCK_PATH = normalizePath(socket); | ||
|
|
||
| env.ATOM_GITHUB_WORKDIR_PATH = this.workingDir; | ||
| env.ATOM_GITHUB_DUGITE_PATH = DUGITE_PATH; | ||
| env.ATOM_GITHUB_DUGITE_PATH = path.join(getDugitePath(), 'git', 'bin', 'git'); | ||
|
|
||
| // "ssh" won't respect SSH_ASKPASS unless: | ||
| // (a) it's running without a tty | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,40 @@ | ||
| import path from 'path'; | ||
| import fs from 'fs-extra'; | ||
| import {ncp} from 'ncp'; | ||
|
|
||
| export function getPackageRoot() { | ||
| const {resourcePath} = atom.getLoadSettings(); | ||
| const currentFileWasRequiredFromSnapshot = !path.isAbsolute(__dirname); | ||
| if (currentFileWasRequiredFromSnapshot) { | ||
| return path.join(resourcePath, 'node_modules', 'github'); | ||
| } else { | ||
| const packageRoot = path.resolve(__dirname, '..'); | ||
| if (path.extname(resourcePath) === '.asar') { | ||
| if (packageRoot.indexOf(resourcePath) === 0) { | ||
| return path.join(`${resourcePath}.unpacked`, 'node_modules', 'github'); | ||
| } | ||
| } | ||
| return packageRoot; | ||
| } | ||
| } | ||
|
|
||
| let DUGITE_PATH; | ||
| export function getDugitePath() { | ||
| if (!DUGITE_PATH) { | ||
| DUGITE_PATH = require.resolve('dugite'); | ||
| if (!path.isAbsolute(DUGITE_PATH)) { | ||
| // Assume we're snapshotted | ||
| const {resourcePath} = atom.getLoadSettings(); | ||
| if (path.extname(resourcePath) === '.asar') { | ||
| DUGITE_PATH = path.join(`${resourcePath}.unpacked`, 'node_modules', 'dugite'); | ||
| } else { | ||
| DUGITE_PATH = path.join(resourcePath, 'node_modules', 'dugite'); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return DUGITE_PATH; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 cool, cool |
||
|
|
||
| function descriptorsFromProto(proto) { | ||
| return Object.getOwnPropertyNames(proto).reduce((acc, name) => { | ||
|
|
@@ -99,9 +133,9 @@ export function writeFile(absoluteFilePath, contents) { | |
| }); | ||
| } | ||
|
|
||
| export function deleteFileOrFolder(path) { | ||
| export function deleteFileOrFolder(fileOrFolder) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😆 I always do this. "path" is such a natural name to use as a variable... |
||
| return new Promise((resolve, reject) => { | ||
| fs.remove(path, err => { | ||
| fs.remove(fileOrFolder, err => { | ||
| if (err) { return reject(err); } else { return resolve(); } | ||
| }); | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,45 +1,26 @@ | ||
| /* eslint-disable no-var */ | ||
| import GithubPackage from './github-package'; | ||
|
|
||
| var semver = require('semver'); | ||
| let pack; | ||
| const entry = { | ||
| activate(...args) { | ||
| pack.activate(...args); | ||
| }, | ||
|
|
||
| var v = semver.parse(atom.appVersion); | ||
| var atomVersion = [v.major, v.minor, v.patch].join('.'); | ||
| var requiredVersion = '>=1.13.0'; | ||
| initialize() { | ||
| pack = new GithubPackage( | ||
| atom.workspace, atom.project, atom.commands, atom.notifications, atom.tooltips, | ||
| atom.styles, atom.config, atom.confirm.bind(atom), | ||
| ); | ||
| }, | ||
| }; | ||
|
|
||
| if (atom.inDevMode() || atom.inSpecMode() || semver.satisfies(atomVersion, requiredVersion)) { | ||
| module.exports = startPackage(); | ||
| } else { | ||
| module.exports = versionMismatch(); | ||
| } | ||
|
|
||
| function versionMismatch() { | ||
| return { | ||
| activate: () => { | ||
| atom.notifications.addWarning('Incompatible Atom Version', { | ||
| description: 'The GitHub packages requires Atom ' + requiredVersion + | ||
| '. You are running ' + atomVersion + '. Please check for updates and try again.', | ||
| dismissable: true, | ||
| }); | ||
| }, | ||
| [ | ||
| 'serialize', 'deserialize', 'deactivate', 'consumeStatusBar', | ||
| 'createGitTimingsView', 'createIssueishPaneItem', | ||
| ].forEach(method => { | ||
| entry[method] = (...args) => { | ||
| return pack[method](...args); | ||
| }; | ||
| } | ||
|
|
||
| function startPackage() { | ||
| var GithubPackage = require('./github-package').default; | ||
|
|
||
| if (atom.inDevMode()) { | ||
| // Let's install some devTools | ||
| try { | ||
| const electronDevtoolsInstaller = require('electron-devtools-installer'); | ||
| const installExtension = electronDevtoolsInstaller.default; | ||
| installExtension(electronDevtoolsInstaller.REACT_DEVELOPER_TOOLS); | ||
| } catch (_e) { | ||
| // Nothing | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| return new GithubPackage( | ||
| atom.workspace, atom.project, atom.commands, atom.notifications, atom.tooltips, | ||
| atom.styles, atom.config, atom.confirm.bind(atom), | ||
| ); | ||
| } | ||
| module.exports = entry; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ import {execFile} from 'child_process'; | |
|
|
||
| import keytar from 'keytar'; | ||
|
|
||
| import {Emitter} from 'atom'; | ||
| import {Emitter} from 'event-kit'; | ||
|
|
||
| export const UNAUTHENTICATED = Symbol('UNAUTHENTICATED'); | ||
|
|
||
|
|
@@ -107,12 +107,22 @@ export default class GithubLoginModel { | |
| return instance; | ||
| } | ||
|
|
||
| constructor(Strategy = strategies.find(strat => strat.isValid())) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving this check to the first time we actually try to use the strategy saves around ~100ms on the package's initial render (called from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another option might be to default to the real Keytar strategy and toggle to the shelling-out one with an environment variable or a (hidden?) config setting. Annoying for those of us who run master, but it'd keep us from paying any performance penalty for the common case of being bundled in a proper Atom distro. |
||
| constructor(Strategy) { | ||
| this._Strategy = Strategy; | ||
| this._strategy = null; | ||
| this.emitter = new Emitter(); | ||
| if (!Strategy) { | ||
| throw new Error('None of the listed GithubLoginModel strategies returned true for `isValid`'); | ||
| } | ||
|
|
||
| get strategy() { | ||
| if (!this._strategy) { | ||
| const Strategy = this._Strategy || strategies.find(strat => strat.isValid()); | ||
| if (!Strategy) { | ||
| throw new Error('None of the listed GithubLoginModel strategies returned true for `isValid`'); | ||
| } | ||
| this._strategy = new Strategy(); | ||
| } | ||
| this.strategy = new Strategy(); | ||
|
|
||
| return this._strategy; | ||
| } | ||
|
|
||
| async getToken(account) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import Relay from 'react-relay'; | ||
|
|
||
| export default class PrInfoByBranchRoute extends Relay.Route { | ||
| static routeName = 'pr-info-route' | ||
|
|
||
| static queries = { | ||
| query: (Component, variables) => Relay.QL` | ||
| query { | ||
| relay { | ||
| ${Component.getFragment('query', variables)} | ||
| } | ||
| } | ||
| `, | ||
| } | ||
|
|
||
| static paramDefinitions = { | ||
| repoOwner: {required: true}, | ||
| repoName: {required: true}, | ||
| branchName: {required: true}, | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| /** @jsx etch.dom */ | ||
| /* eslint react/no-unknown-property: "off" */ | ||
|
|
||
| import {CompositeDisposable, TextEditor} from 'atom'; | ||
| import {TextEditor} from 'atom'; | ||
| import {CompositeDisposable} from 'event-kit'; | ||
|
|
||
| import etch from 'etch'; | ||
| import {autobind} from 'core-decorators'; | ||
|
|
@@ -119,7 +120,7 @@ export default class CommitView { | |
| return '∞'; | ||
| } | ||
| } else { | ||
| return this.props.maximumCharacterLimit; | ||
| return this.props.maximumCharacterLimit || ''; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, this import still works? I thought you'd meant that none of the Atom imports would work properly when bundled. Does it just have to do with the timing of when
EmitterandCompositeDisposableare installed in the Atom global?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use
Pointafter activation, so it works fine. We had to bring inevent-kitforEmitterand friends in other places, so it made sense to replace it package-wide.