Merge branch 'master' into track-email-settings

LICENSE

@@ -0,0 +1,19 @@
+Copyright (c) GitHub, Inc.
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

README.md

@@ -7,3 +7,11 @@ Just playin around with some computering.
 ## Documentation
 
 Development documentation is [in the docs directory](docs). Currently the only thing remotely interesting is the [getting started](docs/getting-started.md) docs.
+
+## License
+
+**[MIT](LICENSE)**
+
+The MIT license grant is not for GitHub's trademarks, which include the logo designs. GitHub reserves all trademark and copyright rights in and to all GitHub trademarks. GitHub's logos include, for instance, the stylized Invertocat designs that include "logo" in the file title in the following folder: [logos](app/static/logos).
+
+GitHub® and its stylized versions and the Invertocat mark are GitHub's Trademarks or registered Trademarks. When using GitHub's logos, be sure to follow the GitHub [logo guidelines](https://github.com/logos).

app/src/lib/api.ts

@@ -432,6 +432,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:
  *
@@ -475,9 +490,13 @@ export function getDotComAPIEndpoint(): string {
   return 'https://api.github.com'
 }
 
-/** Get the user for the endpoint. */
-export function getAccountForEndpoint(accounts: ReadonlyArray<Account>, endpoint: string): Account {
-  return accounts.filter(a => a.endpoint === endpoint)[0]
+/** Get the account for the endpoint. */
+export function getAccountForEndpoint(accounts: ReadonlyArray<Account>, endpoint: string): Account | null {
+  const filteredAccounts = accounts.filter(a => a.endpoint === endpoint)
+  if (filteredAccounts.length) {
+    return filteredAccounts[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) {
@@ -991,18 +994,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`. */
@@ -1235,8 +1239,8 @@ export class AppStore {
   }
 
   /** This shouldn't be called directly. See `Dispatcher`. */
-  public _clone(url: string, path: string, account: Account | null): { promise: Promise<boolean>, repository: CloningRepository } {
-    const promise = this.cloningRepositoriesStore.clone(url, path, account)
+  public _clone(url: string, path: string, options: { account: Account | 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) !
@@ -1268,6 +1272,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, account: Account | null): Promise<void> {
+    const gitStore = this.getGitStore(repository)
+    await gitStore.fetchRefspec(account, refspec)
+
+    return this._refreshRepository(repository)
+  }
+
   /** Fetch the repository. */
   public async fetch(repository: Repository, account: Account | 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 { Account } from '../../models/account'
 
 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, account: Account | 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, account, 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, account: Account | null): Promise<void> {
-    const { promise, repository } = this.appStore._clone(url, path, account)
+    const { promise, repository } = this.appStore._clone(url, path, { account })
     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, account: Account | null): Promise<void> {
-    const { promise, repository } = this.appStore._clone(url, path, account)
+  public async clone(url: string, path: string, options: { account: Account | 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 account 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(account: Account | 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, account, 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 account: Account | 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, account: Account | null, progress: (progress: string) => void): Promise<void> {
-  const env = envForAuthentication(account)
+export async function clone(url: string, path: string, options: CloneOptions, progress: (progress: string) => void): Promise<void> {
+  const env = envForAuthentication(options.account)
   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 { Account } from '../../models/account'
 
@@ -7,15 +7,18 @@ export async function fetch(repository: Repository, account: Account | null, rem
   const options = {
     successExitCodes: new Set([ 0 ]),
     env: envForAuthentication(account),
-    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, account: Account | null, remote: string, refspec: string): Promise<void> {
+  const options = {
+    successExitCodes: new Set([ 0, 128 ]),
+    env: envForAuthentication(account),
+  }
+
+  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, account: Account | null) {
-    this.props.dispatcher.clone(url, path, account)
+    this.props.dispatcher.clone(url, path, { account })
     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, getAccountForEndpoint } 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 account = getAccountForEndpoint(state.accounts, getEndpointForRepository(url))
+
+  return dispatcher.clone(url, path, { account, 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.accounts[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/src/ui/repository-settings/no-remote.tsx

@@ -0,0 +1,32 @@
+import * as React from 'react'
+import { DialogContent } from '../dialog'
+import { Button } from '../lib/button'
+import { Row } from '../lib/row'
+import { LinkButton } from '../lib/link-button'
+
+const HelpURL = 'https://help.github.com/articles/about-remote-repositories/'
+
+interface INoRemoteProps {
+  /** The function to call when the users chooses to publish. */
+  readonly onPublish: () => void
+}
+
+/** The component for when a repository has no remote. */
+export class NoRemote extends React.Component<INoRemoteProps, void> {
+  public render() {
+    return (
+      <DialogContent>
+        <Row className='no-remote'>
+          <div>Publish your repository to GitHub. Need help? <LinkButton uri={HelpURL}>Learn more</LinkButton> about remote repositories.</div>
+          <Button type='submit' onClick={this.onPublish}>Publish</Button>
+        </Row>
+      </DialogContent>
+    )
+  }
+
+  private onPublish = (event: React.MouseEvent<HTMLButtonElement>) => {
+    event.preventDefault()
+
+    this.props.onPublish()
+  }
+}

app/src/ui/repository-settings/remote.tsx

@@ -4,17 +4,17 @@ import { TextBox } from '../lib/text-box'
 import { DialogContent } from '../dialog'
 
 interface IRemoteProps {
-  readonly remote: IRemote | null
+  /** The remote being shown. */
+  readonly remote: IRemote
+
+  /** The function to call when the remote URL is changed by the user. */
   readonly onRemoteUrlChanged: (url: string) => void
 }
 
+/** The Remote component. */
 export class Remote extends React.Component<IRemoteProps, void> {
   public render() {
     const remote = this.props.remote
-    if (!remote) {
-      return <div>Nope</div>
-    }
-
     return (
       <DialogContent>
         <div>Primary remote repository ({remote.name})</div>

app/src/ui/repository-settings/repository-settings.tsx

@@ -5,10 +5,12 @@ import { GitIgnore } from './git-ignore'
 import { assertNever } from '../../lib/fatal-error'
 import { IRemote } from '../../models/remote'
 import { Dispatcher } from '../../lib/dispatcher'
+import { PopupType } from '../../lib/app-state'
 import { Repository } from '../../models/repository'
 import { Button } from '../lib/button'
 import { ButtonGroup } from '../lib/button-group'
 import { Dialog, DialogError, DialogFooter } from '../dialog'
+import { NoRemote } from './no-remote'
 
 interface IRepositorySettingsProps {
   readonly dispatcher: Dispatcher
@@ -83,26 +85,43 @@ export class RepositorySettings extends React.Component<IRepositorySettingsProps
         </TabBar>
 
         {this.renderActiveTab()}
-        <DialogFooter>
-          <ButtonGroup>
-            <Button type='submit'>Save</Button>
-            <Button onClick={this.props.onDismissed}>Cancel</Button>
-          </ButtonGroup>
-        </DialogFooter>
+        {this.renderFooter()}
       </Dialog>
     )
   }
 
+  private renderFooter() {
+    const tab = this.state.selectedTab
+    const remote = this.state.remote
+    if (tab === RepositorySettingsTab.Remote && !remote) {
+      return null
+    }
+
+    return (
+      <DialogFooter>
+        <ButtonGroup>
+          <Button type='submit'>Save</Button>
+          <Button onClick={this.props.onDismissed}>Cancel</Button>
+        </ButtonGroup>
+      </DialogFooter>
+    )
+  }
+
   private renderActiveTab() {
     const tab = this.state.selectedTab
     switch (tab) {
       case RepositorySettingsTab.Remote: {
-        return (
-          <Remote
-            remote={this.state.remote}
-            onRemoteUrlChanged={this.onRemoteUrlChanged}
-          />
-        )
+        const remote = this.state.remote
+        if (remote) {
+          return (
+            <Remote
+              remote={remote}
+              onRemoteUrlChanged={this.onRemoteUrlChanged}
+            />
+          )
+        } else {
+          return <NoRemote onPublish={this.onPublish}/>
+        }
       }
       case RepositorySettingsTab.IgnoredFiles: {
         return <GitIgnore
@@ -116,6 +135,10 @@ export class RepositorySettings extends React.Component<IRepositorySettingsProps
     return assertNever(tab, `Unknown tab type: ${tab}`)
   }
 
+  private onPublish = () => {
+    this.props.dispatcher.showPopup({ type: PopupType.PublishRepository, repository: this.props.repository })
+  }
+
   private onShowGitIgnoreExamples = () => {
     this.props.dispatcher.openInBrowser('https://git-scm.com/docs/gitignore')
   }

app/styles/ui/_dialog.scss

@@ -1,6 +1,7 @@
 @import "../mixins";
 @import "dialogs/merge";
 @import "dialogs/publish-repository";
+@import "dialogs/repository-settings";
 
 // The styles herein attempt to follow a flow where margins are only applied
 // to the bottom of elements (with the exception of the last child). This to
@@ -246,10 +247,6 @@ dialog {
     }
   }
 
-  // Repository settings has long phrasing content in the gitignore
-  // tab so we'll constrain it to 400px.
-  &#repository-settings { width: 400px; }
-
   &#app-error {
     .dialog-content {
       p {

app/styles/ui/dialogs/_repository-settings.scss

@@ -0,0 +1,14 @@
+#repository-settings {
+  width: 450px;
+
+  .no-remote {
+    justify-content: space-between;
+
+    button {
+      flex-shrink: 0;
+      flex-grow: 0;
+      min-width: 120px;
+      align-self: center;
+    }
+  }
+}

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', () => {

script/build

@@ -36,7 +36,7 @@ const options = {
   arch: 'x64',
   asar: false, // TODO: Probably wanna enable this down the road.
   out: path.join(projectRoot, 'dist'),
-  icon: path.join(projectRoot, 'app', 'static', 'icon'),
+  icon: path.join(projectRoot, 'app', 'static', 'logos', 'icon-logo'),
   dir: outRoot,
   overwrite: true,
   tmpdir: false,

script/package

@@ -37,7 +37,7 @@ function packageWindows () {
     cp.execSync(`powershell ${setupCertificatePath}`)
   }
 
-  const iconSource = path.join(__dirname, '..', 'app', 'static', 'icon.ico')
+  const iconSource = path.join(__dirname, '..', 'app', 'static', 'logos', 'icon-logo.ico')
 
   if (!fs.existsSync(iconSource)) {
     console.error(`expected setup icon not found at location: ${iconSource}`)
@@ -45,7 +45,7 @@ function packageWindows () {
   }
 
   // TODO: change this when the repository is public
-  // 'https://raw.githubusercontent.com/desktop/desktop/master/app/static/icon.ico'
+  // 'https://raw.githubusercontent.com/desktop/desktop/master/app/static/logos/icon-logo.ico'
   const iconUrl = 'https://www.dropbox.com/s/6n9wuyqhy9xhow4/icon.ico?dl=1'
 
   const options = {