Merge branch 'master' into license

CODE_OF_CONDUCT.md

@@ -0,0 +1,74 @@
+# Contributor Covenant Code of Conduct
+
+## Our Pledge
+
+In the interest of fostering an open and welcoming environment, we as
+contributors and maintainers pledge to making participation in our project and
+our community a harassment-free experience for everyone, regardless of age, body
+size, disability, ethnicity, gender identity and expression, level of experience,
+nationality, personal appearance, race, religion, or sexual identity and
+orientation.
+
+## Our Standards
+
+Examples of behavior that contributes to creating a positive environment
+include:
+
+* Using welcoming and inclusive language
+* Being respectful of differing viewpoints and experiences
+* Gracefully accepting constructive criticism
+* Focusing on what is best for the community
+* Showing empathy towards other community members
+
+Examples of unacceptable behavior by participants include:
+
+* The use of sexualized language or imagery and unwelcome sexual attention or
+advances
+* Trolling, insulting/derogatory comments, and personal or political attacks
+* Public or private harassment
+* Publishing others' private information, such as a physical or electronic
+  address, without explicit permission
+* Other conduct which could reasonably be considered inappropriate in a
+  professional setting
+
+## Our Responsibilities
+
+Project maintainers are responsible for clarifying the standards of acceptable
+behavior and are expected to take appropriate and fair corrective action in
+response to any instances of unacceptable behavior.
+
+Project maintainers have the right and responsibility to remove, edit, or
+reject comments, commits, code, wiki edits, issues, and other contributions
+that are not aligned to this Code of Conduct, or to ban temporarily or
+permanently any contributor for other behaviors that they deem inappropriate,
+threatening, offensive, or harmful.
+
+## Scope
+
+This Code of Conduct applies both within project spaces and in public spaces
+when an individual is representing the project or its community. Examples of
+representing a project or community include using an official project e-mail
+address, posting via an official social media account, or acting as an appointed
+representative at an online or offline event. Representation of a project may be
+further defined and clarified by project maintainers.
+
+## Enforcement
+
+Instances of abusive, harassing, or otherwise unacceptable behavior may be
+reported by contacting the project team at opensource@github.com. All
+complaints will be reviewed and investigated and will result in a response that
+is deemed necessary and appropriate to the circumstances. The project team is
+obligated to maintain confidentiality with regard to the reporter of an incident.
+Further details of specific enforcement policies may be posted separately.
+
+Project maintainers who do not follow or enforce the Code of Conduct in good
+faith may face temporary or permanent repercussions as determined by other
+members of the project's leadership.
+
+## Attribution
+
+This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4,
+available at [http://contributor-covenant.org/version/1/4][version]
+
+[homepage]: http://contributor-covenant.org
+[version]: http://contributor-covenant.org/version/1/4/

app/.npmrc

@@ -1,4 +1,4 @@
 runtime = electron
 disturl = https://atom.io/download/electron
-target = 1.6.5
+target = 1.6.6
 arch = x64

app/src/lib/api.ts

@@ -412,6 +412,21 @@ async function getNote(): Promise<string> {
   return `GitHub Desktop on ${localUsername}@${OS.hostname()}`
 }
 
+/**
+ * Map a repository's URL to the endpoint associated with it. For example:
+ *
+ * https://github.com/desktop/desktop -> https://api.github.com
+ * http://github.mycompany.com/my-team/my-project -> http://github.mycompany.com/api
+ */
+export function getEndpointForRepository(url: string): string {
+  const parsed = URL.parse(url)
+  if (parsed.hostname === 'github.com') {
+    return getDotComAPIEndpoint()
+  } else {
+    return `${parsed.protocol}//${parsed.hostname}/api`
+  }
+}
+
 /**
  * Get the URL for the HTML site. For example:
  *
@@ -456,8 +471,12 @@ export function getDotComAPIEndpoint(): string {
 }
 
 /** Get the user for the endpoint. */
-export function getUserForEndpoint(users: ReadonlyArray<User>, endpoint: string): User {
-  return users.filter(u => u.endpoint === endpoint)[0]
+export function getUserForEndpoint(users: ReadonlyArray<User>, endpoint: string): User | null {
+  const filteredUsers = users.filter(u => u.endpoint === endpoint)
+  if (filteredUsers.length) {
+    return filteredUsers[0]
+  }
+  return null
 }
 
 export function getOAuthAuthorizationURL(endpoint: string, state: string): string {

app/src/lib/dispatcher/app-store.ts

@@ -540,14 +540,15 @@ export class AppStore {
   }
 
   /** This shouldn't be called directly. See `Dispatcher`. */
-  public async _selectRepository(repository: Repository | CloningRepository | null): Promise<void> {
+  public async _selectRepository(repository: Repository | CloningRepository | null): Promise<Repository | null> {
+
     this.selectedRepository = repository
     this.emitUpdate()
 
     this.stopBackgroundFetching()
 
-    if (!repository) { return Promise.resolve() }
-    if (!(repository instanceof Repository)) { return Promise.resolve() }
+    if (!repository) { return Promise.resolve(null) }
+    if (!(repository instanceof Repository)) { return Promise.resolve(null) }
 
     localStorage.setItem(LastSelectedRepositoryIDKey, repository.id.toString())
 
@@ -556,7 +557,7 @@ export class AppStore {
       // ensures we don't accidentally run any Git operations against the
       // wrong location if the user then relocates the `.git` folder elsewhere
       this.removeGitStore(repository)
-      return
+      return Promise.resolve(null)
     }
 
     const gitHubRepository = repository.gitHubRepository
@@ -567,10 +568,12 @@ export class AppStore {
     await this._refreshRepository(repository)
 
     // The selected repository could have changed while we were refreshing.
-    if (this.selectedRepository !== repository) { return }
+    if (this.selectedRepository !== repository) { return null }
 
     this.startBackgroundFetching(repository)
     this.refreshMentionables(repository)
+
+    return repository
   }
 
   public async _updateIssues(repository: GitHubRepository) {
@@ -997,18 +1000,19 @@ export class AppStore {
   }
 
   /** This shouldn't be called directly. See `Dispatcher`. */
-  public async _createBranch(repository: Repository, name: string, startPoint: string): Promise<void> {
+  public async _createBranch(repository: Repository, name: string, startPoint: string): Promise<Repository> {
     const gitStore = this.getGitStore(repository)
     await gitStore.performFailableOperation(() => createBranch(repository, name, startPoint))
     return this._checkoutBranch(repository, name)
   }
 
   /** This shouldn't be called directly. See `Dispatcher`. */
-  public async _checkoutBranch(repository: Repository, name: string): Promise<void> {
+  public async _checkoutBranch(repository: Repository, name: string): Promise<Repository> {
     const gitStore = this.getGitStore(repository)
     await gitStore.performFailableOperation(() => checkoutBranch(repository, name))
 
-    return this._refreshRepository(repository)
+    await this._refreshRepository(repository)
+    return repository
   }
 
   /** This shouldn't be called directly. See `Dispatcher`. */
@@ -1241,8 +1245,8 @@ export class AppStore {
   }
 
   /** This shouldn't be called directly. See `Dispatcher`. */
-  public _clone(url: string, path: string, user: User | null): { promise: Promise<boolean>, repository: CloningRepository } {
-    const promise = this.cloningRepositoriesStore.clone(url, path, user)
+  public _clone(url: string, path: string, options: { user: User | null, branch?: string }): { promise: Promise<boolean>, repository: CloningRepository } {
+    const promise = this.cloningRepositoriesStore.clone(url, path, options)
     const repository = this.cloningRepositoriesStore
                            .repositories
                            .find(r => r.url === url && r.path === path) !
@@ -1274,6 +1278,22 @@ export class AppStore {
     return gitStore.clearContextualCommitMessage()
   }
 
+  /**
+   * Fetch a specific refspec for the repository.
+   *
+   * As this action is required to complete when viewing a Pull Request from
+   * a fork, it does not opt-in to checks that prevent multiple concurrent
+   * network actions. This might require some rework in the future to chain
+   * these actions.
+   *
+   */
+  public async fetchRefspec(repository: Repository, refspec: string, user: User | null): Promise<void> {
+    const gitStore = this.getGitStore(repository)
+    await gitStore.fetchRefspec(user, refspec)
+
+    return this._refreshRepository(repository)
+  }
+
   /** Fetch the repository. */
   public async fetch(repository: Repository, user: User | null): Promise<void> {
     await this.withPushPull(repository, async () => {

app/src/lib/dispatcher/cloning-repositories-store.ts

@@ -2,9 +2,8 @@ import * as Path from 'path'
 
 import { Emitter, Disposable } from 'event-kit'
 
-import { clone as cloneRepo } from '../git'
+import { clone as cloneRepo, CloneOptions } from '../git'
 import { CloneProgressParser } from '../clone-progress-parser'
-import { User } from '../../models/user'
 
 let CloningRepositoryID = 1
 
@@ -68,7 +67,7 @@ export class CloningRepositoriesStore {
    *
    * Returns a {Promise} which resolves to whether the clone was successful.
    */
-  public async clone(url: string, path: string, user: User | null): Promise<boolean> {
+  public async clone(url: string, path: string, options: CloneOptions): Promise<boolean> {
     const repository = new CloningRepository(path, url)
     this._repositories.push(repository)
     this.stateByID.set(repository.id, { output: `Cloning into ${path}`, progressValue: null })
@@ -77,7 +76,7 @@ export class CloningRepositoriesStore {
     let success = true
     const progressParser = new CloneProgressParser()
     try {
-      await cloneRepo(url, path, user, progress => {
+      await cloneRepo(url, path, options, progress => {
         this.stateByID.set(repository.id, {
           output: progress,
           progressValue: progressParser.parse(progress),

app/src/lib/dispatcher/dispatcher.ts

@@ -228,12 +228,14 @@ export class Dispatcher {
   }
 
   /** Select the repository. */
-  public async selectRepository(repository: Repository | CloningRepository): Promise<void> {
-    this.appStore._selectRepository(repository)
+  public async selectRepository(repository: Repository | CloningRepository): Promise<Repository | null> {
+    const repo = this.appStore._selectRepository(repository)
 
     if (repository instanceof Repository) {
       await this.refreshGitHubRepositoryInfo(repository)
     }
+
+    return repo
   }
 
   /** Load the working directory status. */
@@ -302,12 +304,12 @@ export class Dispatcher {
   }
 
   /** Create a new branch from the given starting point and check it out. */
-  public createBranch(repository: Repository, name: string, startPoint: string): Promise<void> {
+  public createBranch(repository: Repository, name: string, startPoint: string): Promise<Repository> {
     return this.appStore._createBranch(repository, name, startPoint)
   }
 
   /** Check out the given branch. */
-  public checkoutBranch(repository: Repository, name: string): Promise<void> {
+  public checkoutBranch(repository: Repository, name: string): Promise<Repository> {
     return this.appStore._checkoutBranch(repository, name)
   }
 
@@ -344,8 +346,15 @@ export class Dispatcher {
     )
   }
 
-  /** Fetch the repository. */
-  public async fetch(repository: Repository): Promise<void> {
+  /** Fetch a specific refspec for the repository. */
+  public fetchRefspec(repository: Repository, fetchspec: string): Promise<void> {
+    return this.withAuthenticatingUser(repository, (repo, user) => {
+      return this.appStore.fetchRefspec(repo, fetchspec, user)
+    })
+  }
+
+  /** Fetch all refs for the repository */
+  public fetch(repository: Repository): Promise<void> {
     return this.withAuthenticatingUser(repository, (repo, user) =>
       this.appStore.fetch(repo, user)
     )
@@ -394,7 +403,7 @@ export class Dispatcher {
    * state in the repository list if the clone completes without error.
    */
   public async cloneAgain(url: string, path: string, user: User | null): Promise<void> {
-    const { promise, repository } = this.appStore._clone(url, path, user)
+    const { promise, repository } = this.appStore._clone(url, path, { user })
     await this.selectRepository(repository)
     const success = await promise
     if (!success) { return }
@@ -412,14 +421,17 @@ export class Dispatcher {
  }
 
   /** Clone the repository to the path. */
-  public async clone(url: string, path: string, user: User | null): Promise<void> {
-    const { promise, repository } = this.appStore._clone(url, path, user)
+  public async clone(url: string, path: string, options: { user: User | null, branch?: string }): Promise<Repository | null> {
+    const { promise, repository } = this.appStore._clone(url, path, options)
     await this.selectRepository(repository)
     const success = await promise
-    if (!success) { return }
+    // TODO: this exit condition is not great, bob
+    if (!success) { return Promise.resolve(null) }
 
     const addedRepositories = await this.addRepositories([ path ])
-    await this.selectRepository(addedRepositories[0])
+    const addedRepository = addedRepositories[0]
+    await this.selectRepository(addedRepository)
+    return addedRepository
   }
 
   /** Rename the branch to a new name. */

app/src/lib/dispatcher/git-store.ts

@@ -17,6 +17,7 @@ import {
   getDefaultRemote,
   getRemotes,
   fetch as fetchRepo,
+  fetchRefspec,
   getRecentBranches,
   getBranches,
   getTip,
@@ -418,6 +419,25 @@ export class GitStore {
     }
   }
 
+  /**
+   * Fetch a given refspec, using the given user for authentication.
+   *
+   * @param user - The user to use for authentication if needed.
+   * @param refspec - The association between a remote and local ref to use as
+   *                  part of this action. Refer to git-scm for more
+   *                  information on refspecs: https://www.git-scm.com/book/tr/v2/Git-Internals-The-Refspec
+   *
+   */
+  public async fetchRefspec(user: User | null, refspec: string): Promise<void> {
+
+    // TODO: we should favour origin here
+    const remotes = await getRemotes(this.repository)
+
+    for (const remote of remotes) {
+      await this.performFailableOperation(() => fetchRefspec(this.repository, user, remote.name, refspec))
+    }
+  }
+
   /** Calculate the ahead/behind for the current branch. */
   public async calculateAheadBehindForCurrentBranch(): Promise<void> {
 

app/src/lib/git/checkout.ts

@@ -6,7 +6,6 @@ export async function checkoutBranch(repository: Repository, name: string): Prom
   await git([ 'checkout', name, '--' ], repository.path, 'checkoutBranch')
 }
 
-
 /** Check out the paths at HEAD. */
 export async function checkoutPaths(repository: Repository, paths: ReadonlyArray<string>): Promise<void> {
   await git([ 'checkout', 'HEAD', '--', ...paths ], repository.path, 'checkoutPaths')

app/src/lib/git/clone.ts

@@ -4,14 +4,30 @@ import { ChildProcess } from 'child_process'
 
 const byline = require('byline')
 
+/** Additional arguments to provide when cloning a repository */
+export type CloneOptions = {
+  /** The optional identity to provide when cloning. */
+  readonly user: User | null
+  /** The branch to checkout after the clone has completed. */
+  readonly branch?: string
+}
+
 /** Clone the repository to the path. */
-export async function clone(url: string, path: string, user: User | null, progress: (progress: string) => void): Promise<void> {
-  const env = envForAuthentication(user)
+export async function clone(url: string, path: string, options: CloneOptions, progress: (progress: string) => void): Promise<void> {
+  const env = envForAuthentication(options.user)
   const processCallback = (process: ChildProcess) => {
     byline(process.stderr).on('data', (chunk: string) => {
       progress(chunk)
     })
   }
 
-  await git([ 'clone', '--recursive', '--progress', '--', url, path ], __dirname, 'clone', { env, processCallback })
+  const args = [ 'clone', '--recursive', '--progress' ]
+
+  if (options.branch) {
+    args.push('-b', options.branch)
+  }
+
+  args.push('--', url, path)
+
+  await git(args, __dirname, 'clone', { env, processCallback })
 }

app/src/lib/git/fetch.ts

@@ -1,4 +1,4 @@
-import { git, envForAuthentication, expectedAuthenticationErrors, GitError } from './core'
+import { git, envForAuthentication } from './core'
 import { Repository } from '../../models/repository'
 import { User } from '../../models/user'
 
@@ -7,15 +7,18 @@ export async function fetch(repository: Repository, user: User | null, remote: s
   const options = {
     successExitCodes: new Set([ 0 ]),
     env: envForAuthentication(user),
-    expectedErrors: expectedAuthenticationErrors(),
   }
 
-  const args = [ 'fetch', '--prune', remote ]
-  const result = await git(args, repository.path, 'fetch', options)
-
-  if (result.gitErrorDescription) {
-    return Promise.reject(new GitError(result, args))
-  }
-
-  return Promise.resolve()
+  await git([ 'fetch', '--prune', remote ], repository.path, 'fetch', options)
 }
+
+/** Fetch a given refspec from the given remote. */
+export async function fetchRefspec(repository: Repository, user: User | null, remote: string, refspec: string): Promise<void> {
+  const options = {
+    successExitCodes: new Set([ 0, 128 ]),
+    env: envForAuthentication(user),
+  }
+
+  await git([ 'fetch', remote, refspec ], repository.path, 'fetchRefspec', options)
+}
+

app/src/lib/parse-url.ts

@@ -1,4 +1,5 @@
 import * as URL from 'url'
+import { testForInvalidChars } from './sanitize-branch'
 
 interface IURLAction<T> {
   name: string
@@ -9,14 +10,25 @@ export interface IOAuthActionArgs {
   readonly code: string
 }
 
+export interface IOpenRepositoryArgs {
+  /** the remote repository location associated with the "Open in Desktop" action */
+  readonly url: string
+  /** the optional branch name which should be checked out. use the default branch otherwise. */
+  readonly branch?: string
+  /** the pull request number, if pull request originates from a fork of the repository */
+  readonly pr?: string
+  /** the file to open after cloning the repository */
+  readonly filepath?: string
+}
+
 export interface IOAuthAction extends IURLAction<IOAuthActionArgs> {
   readonly name: 'oauth'
   readonly args: IOAuthActionArgs
 }
 
-export interface IOpenRepositoryAction extends IURLAction<string> {
+export interface IOpenRepositoryAction extends IURLAction<IOpenRepositoryArgs> {
   readonly name: 'open-repository'
-  readonly args: string
+  readonly args: IOpenRepositoryArgs
 }
 
 export interface IUnknownAction extends IURLAction<{}> {
@@ -35,11 +47,50 @@ export function parseURL(url: string): URLActionType {
   const actionName = hostname.toLowerCase()
   if (actionName === 'oauth') {
     return { name: 'oauth', args: { code: parsedURL.query.code } }
-  } else if (actionName === 'openrepo') {
-    // The `path` will be: /https://github.com/user/repo, so we need to take a
-    // substring from the first character on.
-    return { name: 'open-repository', args: `${parsedURL.path!.substr(1)}.git` }
-  } else {
-    return unknown
   }
+
+  if (actionName === 'openrepo') {
+
+    // we require something resembling a URL first
+    // - bail out if it's not defined
+    // - bail out if you only have `/`
+    const pathName = parsedURL.pathname
+    if (!pathName || pathName.length <= 1) { return unknown }
+
+    // trim the leading / from the parsed URL
+    const probablyAURL = pathName.substr(1)
+
+    // suffix the remote URL with `.git`, for backwards compatibility
+    const url = `${probablyAURL}.git`
+
+    const queryString = parsedURL.query
+
+    const pr = queryString.pr
+    const branch = queryString.branch
+    const filepath = queryString.filepath
+
+    if (pr) {
+      // if anything other than a number is used for the PR value, exit
+      if (!/^\d+$/.test(pr)) { return unknown }
+
+      // we also expect the branch for a forked PR to be a given ref format
+      if (!/^pr\/\d+$/.test(branch)) { return unknown }
+    }
+
+    if (branch) {
+      if (testForInvalidChars(branch)) { return unknown }
+    }
+
+    return {
+      name: 'open-repository',
+      args: {
+        url,
+        branch,
+        pr,
+        filepath,
+      },
+    }
+  }
+
+  return unknown
 }

app/src/lib/sanitize-branch.ts

@@ -0,0 +1,17 @@
+// See https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
+// ASCII Control chars and space, DEL, ~ ^ : ? * [ \
+// | " < and > is technically a valid refname but not on Windows
+// the magic sequence @{, consecutive dots, leading and trailing dot, ref ending in .lock
+const invalidCharacterRegex = /[\x00-\x20\x7F~^:?*\[\\|""<>]|@{|\.\.+|^\.|\.$|\.lock$|\/$/g
+
+/** Sanitize a proposed branch name by replacing illegal characters. */
+export function sanitizedBranchName(name: string): string {
+  return name.replace(invalidCharacterRegex, '-')
+    .replace(/--+/g, '-')
+    .replace(/^-/g, '')
+}
+
+/** Validate a branch does not contain any invalid characters */
+export function testForInvalidChars(name: string): boolean {
+  return invalidCharacterRegex.test(name)
+}

app/src/ui/add-repository/clone-repository.tsx

@@ -179,7 +179,7 @@ export class CloneRepository extends React.Component<ICloneRepositoryProps, IClo
   }
 
   private cloneImpl(url: string, path: string, user: User | null) {
-    this.props.dispatcher.clone(url, path, user)
+    this.props.dispatcher.clone(url, path, { user })
     this.props.onDismissed()
 
     setDefaultDir(Path.resolve(path, '..'))

app/src/ui/create-branch/index.tsx

@@ -2,7 +2,7 @@ import * as React from 'react'
 
 import { Repository } from '../../models/repository'
 import { Dispatcher } from '../../lib/dispatcher'
-import { sanitizedBranchName } from './sanitized-branch-name'
+import { sanitizedBranchName } from '../../lib/sanitize-branch'
 import { Branch } from '../../models/branch'
 import { TextBox } from '../lib/text-box'
 import { Row } from '../lib/row'

app/src/ui/create-branch/sanitized-branch-name.ts

@@ -1,10 +0,0 @@
-/** Sanitize a proposed branch name by replacing illegal characters. */
-export function sanitizedBranchName(name: string): string {
-  // See https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
-  // ASCII Control chars and space, DEL, ~ ^ : ? * [ \
-  // | " < and > is technically a valid refname but not on Windows
-  // the magic sequence @{, consecutive dots, leading and trailing dot, ref ending in .lock
-  return name.replace(/[\x00-\x20\x7F~^:?*\[\\|""<>]|@{|\.\.+|^\.|\.$|\.lock$|\/$/g, '-')
-    .replace(/--+/g, '-')
-    .replace(/^-/g, '')
-}

app/src/ui/index.tsx

@@ -3,11 +3,11 @@ import * as ReactDOM from 'react-dom'
 import * as Path from 'path'
 import * as Url from 'url'
 
-import { ipcRenderer, remote } from 'electron'
+import { ipcRenderer, remote, shell } from 'electron'
 
 import { App } from './app'
 import { Dispatcher, AppStore, GitHubUserStore, GitHubUserDatabase, CloningRepositoriesStore, EmojiStore } from '../lib/dispatcher'
-import { URLActionType } from '../lib/parse-url'
+import { URLActionType, IOpenRepositoryArgs } from '../lib/parse-url'
 import { Repository } from '../models/repository'
 import { getDefaultDir, setDefaultDir } from './lib/default-dir'
 import { SelectionType } from '../lib/app-state'
@@ -18,7 +18,7 @@ import { StatsDatabase, StatsStore } from '../lib/stats'
 import { IssuesDatabase, IssuesStore, SignInStore } from '../lib/dispatcher'
 import { requestAuthenticatedUser, resolveOAuthRequest, rejectOAuthRequest } from '../lib/oauth'
 import { defaultErrorHandler, createMissingRepositoryHandler } from '../lib/dispatcher'
-
+import { getEndpointForRepository, getUserForEndpoint } from '../lib/api'
 import { getLogger } from '../lib/logging/renderer'
 import { installDevGlobals } from './install-globals'
 
@@ -82,56 +82,94 @@ ipcRenderer.on('blur', () => {
 })
 
 ipcRenderer.on('url-action', async (event: Electron.IpcRendererEvent, { action }: { action: URLActionType }) => {
-  if (action.name === 'oauth') {
-    try {
-      const user = await requestAuthenticatedUser(action.args.code)
-      if (user) {
-        resolveOAuthRequest(user)
-      } else {
-        rejectOAuthRequest(new Error('Unable to fetch authenticated user.'))
+  switch (action.name) {
+    case 'oauth':
+      try {
+        const user = await requestAuthenticatedUser(action.args.code)
+        if (user) {
+          resolveOAuthRequest(user)
+        } else {
+          rejectOAuthRequest(new Error('Unable to fetch authenticated user.'))
+        }
+      } catch (e) {
+        rejectOAuthRequest(e)
       }
-    } catch (e) {
-      rejectOAuthRequest(e)
-    }
-  } else if (action.name === 'open-repository') {
-    openRepository(action.args)
-  } else {
-    console.log(`Unknown URL action: ${action.name}`)
+      break
+
+    case 'open-repository':
+      const { pr, url, branch } = action.args
+      // a forked PR will provide both these values, despite the branch not existing
+      // in the repository - drop the branch argument in this case so a clone will
+      // checkout the default branch when it clones
+      const branchToClone = (pr && branch) ? undefined : branch
+      openRepository(url, branchToClone)
+        .then(repository => handleCloneInDesktopOptions(repository, action.args))
+      break
+
+    default:
+      console.log(`Unknown URL action: ${action.name} - payload: ${JSON.stringify(action)}`)
   }
 })
 
-function openRepository(url: string) {
+function cloneRepository(url: string, branch?: string): Promise<Repository | null> {
+  const cloneLocation = getDefaultDir()
+
+  const defaultName = Path.basename(Url.parse(url)!.path!, '.git')
+  const path: string | null = remote.dialog.showSaveDialog({
+    buttonLabel: 'Clone',
+    defaultPath: Path.join(cloneLocation, defaultName),
+  })
+  if (!path) { return Promise.resolve(null) }
+
+  setDefaultDir(Path.resolve(path, '..'))
+
+  const state = appStore.getState()
+  const user = getUserForEndpoint(state.users, getEndpointForRepository(url)) || null
+
+  return dispatcher.clone(url, path, { user, branch })
+}
+
+async function handleCloneInDesktopOptions(repository: Repository | null, args: IOpenRepositoryArgs): Promise<void> {
+  // skip this if the clone failed for whatever reason
+  if (!repository) { return }
+
+  const { filepath, pr, branch } = args
+
+  // we need to refetch for a forked PR and check that out
+  if (pr && branch) {
+    await dispatcher.fetchRefspec(repository, `pull/${pr}/head:${branch}`)
+    await dispatcher.checkoutBranch(repository, branch)
+  }
+
+  if (filepath) {
+    const fullPath = Path.join(repository.path, filepath)
+    // because Windows uses different path separators here
+    const normalized = Path.normalize(fullPath)
+    shell.openItem(normalized)
+  }
+}
+
+function openRepository(url: string, branch?: string): Promise<Repository | null> {
   const state = appStore.getState()
   const repositories = state.repositories
   const existingRepository = repositories.find(r => {
     if (r instanceof Repository) {
       const gitHubRepository = r.gitHubRepository
       if (!gitHubRepository) { return false }
-      return gitHubRepository.htmlURL === url
+      return gitHubRepository.cloneURL === url
     } else {
       return false
     }
   })
 
   if (existingRepository) {
-    return dispatcher.selectRepository(existingRepository)
-  } else {
-    const cloneLocation = getDefaultDir()
-
-    const defaultName = Path.basename(Url.parse(url)!.path!, '.git')
-    const path: string | null = remote.dialog.showSaveDialog({
-      buttonLabel: 'Clone',
-      defaultPath: Path.join(cloneLocation, defaultName),
+    return dispatcher.selectRepository(existingRepository).then(repo => {
+      if (!repo || !branch) { return repo }
+      return dispatcher.checkoutBranch(repo, branch)
     })
-    if (!path) { return }
-
-    setDefaultDir(Path.resolve(path, '..'))
-
-    // TODO: This isn't quite right. We should probably get the user from the
-    // context or URL or something.
-    const user = state.users[0]
-    return dispatcher.clone(url, path, user)
   }
+
+  return cloneRepository(url, branch)
 }
 
 ReactDOM.render(

app/src/ui/rename-branch/index.tsx

@@ -3,7 +3,7 @@ import * as React from 'react'
 import { Dispatcher } from '../../lib/dispatcher'
 import { Repository } from '../../models/repository'
 import { Branch } from '../../models/branch'
-import { sanitizedBranchName } from '../create-branch/sanitized-branch-name'
+import { sanitizedBranchName } from '../../lib/sanitize-branch'
 import { TextBox } from '../lib/text-box'
 import { Button } from '../lib/button'
 import { ButtonGroup } from '../lib/button-group'

app/test/unit/git/checkout-test.ts

@@ -0,0 +1,36 @@
+import { expect, use as chaiUse } from 'chai'
+import { setupEmptyRepository, setupFixtureRepository } from '../../fixture-helper'
+import { Repository } from '../../../src/models/repository'
+import { checkoutBranch, getTip } from '../../../src/lib/git'
+import { TipState, IValidBranch } from '../../../src/models/tip'
+
+chaiUse(require('chai-datetime'))
+
+describe('git/checkout', () => {
+  it('throws when invalid characters are used for branch name', async () => {
+    const repository = await setupEmptyRepository()
+
+    let errorRaised = false
+    try {
+      await checkoutBranch(repository, '..')
+    } catch (error) {
+      errorRaised = true
+      expect(error.message).to.equal('fatal: invalid reference: ..\n')
+    }
+
+    expect(errorRaised).to.be.true
+  })
+
+  it('can checkout a valid branch name in an existing repository', async () => {
+    const path = await setupFixtureRepository('repo-with-many-refs')
+    const repository = new Repository(path, -1, null, false)
+
+    await checkoutBranch(repository, 'commit-with-long-description')
+
+    const tip = await getTip(repository)
+    expect(tip.kind).to.equal(TipState.Valid)
+
+    const validBranch = tip as IValidBranch
+    expect(validBranch.branch.name).to.equal('commit-with-long-description')
+  })
+})

app/test/unit/parse-url-test.ts

@@ -0,0 +1,79 @@
+import * as chai from 'chai'
+const expect = chai.expect
+
+import { parseURL, IOpenRepositoryAction, IOAuthAction } from '../../src/lib/parse-url'
+
+describe('parseURL', () => {
+  it('returns unknown by default', () => {
+    expect(parseURL('').name).to.equal('unknown')
+  })
+
+  describe('oauth', () => {
+    it('returns right name', () => {
+      const expectedArgs = {
+        'code': '18142422',
+      }
+
+      const result = parseURL('x-github-client://oauth?code=18142422&state=e4cd2dea-1567-46aa-8eb2-c7f56e943187')
+      expect(result.name).to.equal('oauth')
+
+      const openRepo = result as IOAuthAction
+      expect(openRepo.args).to.deep.equal(expectedArgs)
+    })
+  })
+
+  describe('openRepo', () => {
+    it('returns right name', () => {
+      const result = parseURL('github-mac://openRepo/https://github.com/desktop/desktop')
+      expect(result.name).to.equal('open-repository')
+
+      const openRepo = result as IOpenRepositoryAction
+      expect(openRepo.args.url).to.equal('https://github.com/desktop/desktop.git')
+    })
+
+    it('returns unknown when no remote defined', () => {
+      const result = parseURL('github-mac://openRepo/')
+      expect(result.name).to.equal('unknown')
+    })
+
+    it('adds branch name if set', () => {
+      const result = parseURL('github-mac://openRepo/https://github.com/desktop/desktop?branch=cancel-2fa-flow')
+      expect(result.name).to.equal('open-repository')
+
+      const openRepo = result as IOpenRepositoryAction
+      expect(openRepo.args.url).to.equal('https://github.com/desktop/desktop.git')
+      expect(openRepo.args.branch).to.equal('cancel-2fa-flow')
+    })
+
+    it('adds pull request ID if found', () => {
+      const result = parseURL('github-mac://openRepo/https://github.com/octokit/octokit.net?branch=pr%2F1569&pr=1569')
+      expect(result.name).to.equal('open-repository')
+
+      const openRepo = result as IOpenRepositoryAction
+      expect(openRepo.args.url).to.equal('https://github.com/octokit/octokit.net.git')
+      expect(openRepo.args.branch).to.equal('pr/1569')
+      expect(openRepo.args.pr).to.equal('1569')
+    })
+
+    it('returns unknown for unexpected pull request input', () => {
+      const result = parseURL('github-mac://openRepo/https://github.com/octokit/octokit.net?branch=bar&pr=foo')
+      expect(result.name).to.equal('unknown')
+    })
+
+    it('returns unknown for invalid branch name', () => {
+      // branch=<>
+      const result = parseURL('github-mac://openRepo/https://github.com/octokit/octokit.net?branch=%3C%3E')
+      expect(result.name).to.equal('unknown')
+    })
+
+    it('adds file path if found', () => {
+      const result = parseURL('github-mac://openRepo/https://github.com/octokit/octokit.net?branch=master&filepath=Octokit.Reactive%2FOctokit.Reactive.csproj')
+      expect(result.name).to.equal('open-repository')
+
+      const openRepo = result as IOpenRepositoryAction
+      expect(openRepo.args.url).to.equal('https://github.com/octokit/octokit.net.git')
+      expect(openRepo.args.branch).to.equal('master')
+      expect(openRepo.args.filepath).to.equal('Octokit.Reactive/Octokit.Reactive.csproj')
+    })
+  })
+})

app/test/unit/sanitize-test-name-test.ts

@@ -1,7 +1,7 @@
 import * as chai from 'chai'
 const expect = chai.expect
 
-import { sanitizedBranchName } from '../../src/ui/create-branch/sanitized-branch-name'
+import { sanitizedBranchName } from '../../src/lib/sanitize-branch'
 
 describe('sanitizedBranchName', () => {
   it('leaves a good branch name alone', () => {

package.json

@@ -43,7 +43,7 @@
     "chai-datetime": "^1.4.1",
     "cross-env": "^3.2.3",
     "css-loader": "^0.26.2",
-    "electron": "1.6.5",
+    "electron": "1.6.6",
     "electron-mocha": "3.3.0",
     "electron-packager": "8.6.0",
     "electron-winstaller": "2.5.2",