`pull-request` aborts if unpushed commits; opens editor for title/body

2011-10-24 01:12:26 +02:00 · 2011-10-24 01:12:26 +02:00 · 132eb6f274
--- a/README.md
+++ b/README.md
@ -190,11 +190,12 @@ superpowers:
 ### git pull-request

    # while on a topic branch called "feature":
-    $ git pull-request "I've implemented feature X"
+    $ git pull-request
+    [ opens text editor to edit title & body for the request ]
    [ opened pull request on GitHub for "YOUR_USER:feature" ]

-    # explicit pull base & head:
-    $ git pull-request -b defunkt:master -h mislav:feature
+    # explicit title, pull base & head:
+    $ git pull-request "I've implemented feature X" -b defunkt:master -h mislav:feature

    $ git pull-request #123
    [ attached pull request to issue #123 ]
--- a/lib/hub/commands.rb
+++ b/lib/hub/commands.rb
@ -73,11 +73,13 @@ module Hub
    def pull_request(args)
      args.shift
      options = { }
-      explicit_owner = false
+      force = explicit_owner = false
      base_repo = origin_repo

      while arg = args.shift
        case arg
+        when '-f'
+          force = true
        when '-b'
          options[:base] = Context::Ref.from_github_ref(args.shift, origin_repo)
        when '-h'
@ -105,6 +107,21 @@ module Hub
        options[:head].repo = head_repo.from_owner(github_user)
      end

+      if not force and not explicit_owner and local_commits = Context::GIT_CONFIG["rev-list --cherry #{options[:head].to_local_ref}..."]
+        $stderr.puts "Aborted: #{local_commits.split("\n").size} commits are not yet pushed to #{options[:head].to_local_ref}"
+        warn "(use `-f` to force submit a pull request anyway)"
+        abort
+      end
+
+      unless options[:title] or options[:issue]
+        changes = Context::GIT_CONFIG["log --no-color --pretty=medium --cherry %s...%s" %
+          [options[:base].to_local_ref, options[:head].to_local_ref]]
+
+        options[:title], options[:body] = pullrequest_editmsg(changes) { |msg|
+          msg.puts "# You're requesting a pull to #{options[:base].to_github_ref} from #{options[:head].to_github_ref}"
+        }
+      end
+
      pull = create_pullrequest(options)

      args.executable = 'echo'
@ -805,6 +822,39 @@ help
      YAML.load(response.body)['pull']
    end

+    def pullrequest_editmsg(changes)
+      message_file = File.join(git_dir, 'PULLREQ_EDITMSG')
+      File.open(message_file, 'w') { |msg|
+        msg.puts
+        yield msg
+        if changes
+          msg.puts "#\n# Changes:\n#"
+          msg.puts changes.gsub(/^/, '# ')
+        end
+      }
+      edit_cmd = Array(git_editor).dup << message_file
+      system(*edit_cmd)
+      abort "can't open text editor for pull request message" unless $?.success?
+      title, body = read_editmsg(message_file)
+      abort "Aborting due to empty pull request title" unless title
+      [title, body]
+    end
+
+    def read_editmsg(file)
+      title, body = '', ''
+      File.open(file, 'r') { |msg|
+        msg.each_line do |line|
+          next if line.index('#') == 0
+          ((body.empty? and line =~ /\S/) ? title : body) << line
+        end
+      }
+      title.tr!("\n", ' ')
+      title.strip!
+      body.strip!
+      
+      [title =~ /\S/ ? title : nil, body =~ /\S/ ? body : nil]
+    end
+
    def expand_alias(cmd)
      if expanded = git_alias_for(cmd)
        if expanded.index('!') != 0
--- a/lib/hub/context.rb
+++ b/lib/hub/context.rb
@ -279,6 +279,14 @@ module Hub
      !!git_dir
    end

+    def git_editor
+      # possible: ~/bin/vi, $SOME_ENVIRONMENT_VARIABLE, "C:\Program Files\Vim\gvim.exe" --nofork
+      editor = GIT_CONFIG['var GIT_EDITOR']
+      editor = ENV[$1] if editor =~ /^\$(\w+)$/
+      editor = File.expand_path editor if (editor =~ /^[~.]/ or editor.index('/')) and editor !~ /["']/
+      editor.shellsplit
+    end
+
    # Cross-platform web browser command; respects the value set in $BROWSER.
    # 
    # Returns an array, e.g.: ['open']
--- a/man/hub.1
+++ b/man/hub.1
@ -55,7 +55,7 @@
 \fBgit fork\fR [\fB\-\-no\-remote\fR]
 .
 .br
-\fBgit pull\-request\fR [\fITITLE\fR] [\fB\-b\fR \fIBASE\fR] [\fB\-h\fR \fIHEAD\fR]
+\fBgit pull\-request\fR [\fB\-f\fR] [\fITITLE\fR] [\fB\-b\fR \fIBASE\fR] [\fB\-h\fR \fIHEAD\fR]
 .
 .SH "DESCRIPTION"
 \fBhub\fR enhances various \fBgit\fR commands with GitHub remote expansion\. The alias command displays information on configuring your environment:
@ -124,10 +124,13 @@ Submodule repository "git://github\.com/\fIUSER\fR/\fIREPOSITORY\fR\.git" into \
 \fBgit fork\fR [\fB\-\-no\-remote\fR]: Forks the original project (referenced by "origin" remote) on GitHub and adds a new remote for it under your username\. Requires \fBgithub\.token\fR to be set (see CONFIGURATION)\.
 .
 .IP "\(bu" 4
-\fBgit pull\-request\fR [\fITITLE\fR] [\fB\-b\fR \fIBASE\fR] [\fB\-h\fR \fIHEAD\fR]:
+\fBgit pull\-request\fR [\fB\-f\fR] [\fITITLE\fR] [\fB\-b\fR \fIBASE\fR] [\fB\-h\fR \fIHEAD\fR]:
 .
 .br
-Opens a pull request on GitHub for the project that the "origin" remote points to\. The default head of the pull request is the current branch\. Both base and head of the pull request can be explicitly given in one of the following formats: "branch", "owner:branch", "owner/repo:branch"\.
+Opens a pull request on GitHub for the project that the "origin" remote points to\. The default head of the pull request is the current branch\. Both base and head of the pull request can be explicitly given in one of the following formats: "branch", "owner:branch", "owner/repo:branch"\. This command will abort operation if it detects that the current topic branch has local commits that are not yet pushed to its upstream branch on the remote\. To skip this check, use \fB\-f\fR\.
+.
+.IP
+If \fITITLE\fR is omitted, a text editor will open in which title and body of the pull request can be entered in the same manner as git commit message\.
 .
 .IP
 If instead of normal \fITITLE\fR a string in "#\fINUMBER\fR" format is given, the pull request will be attached to an existing GitHub issue whose ID corresponds to \fINUMBER\fR\.
@ -292,11 +295,12 @@ $ git fork
 .nf

 # while on a topic branch called "feature":
-$ git pull\-request "I\'ve implemented feature X"
+$ git pull\-request
+[ opens text editor to edit title & body for the request ]
 [ opened pull request on GitHub for "YOUR_USER:feature" ]

-# explicit pull base & head:
-$ git pull\-request \-b defunkt:master \-h mislav:feature
+# explicit title, pull base & head:
+$ git pull\-request "I\'ve implemented feature X" \-b defunkt:master \-h mislav:feature

 $ git pull\-request #123
 [ attached pull request to issue #123 ]
--- a/man/hub.1.html
+++ b/man/hub.1.html
@ -93,7 +93,7 @@
 <code>git compare</code> [<code>-u</code>] [<var>USER</var>] [<var>START</var>...]<var>END</var><br />
 <code>git submodule add</code> [<code>-p</code>] <var>OPTIONS</var> [<var>USER</var>/]<var>REPOSITORY</var> <var>DIRECTORY</var><br />
 <code>git fork</code> [<code>--no-remote</code>]<br />
-<code>git pull-request</code> [<var>TITLE</var>] [<code>-b</code> <var>BASE</var>] [<code>-h</code> <var>HEAD</var>]</p>
+<code>git pull-request</code> [<code>-f</code>] [<var>TITLE</var>] [<code>-b</code> <var>BASE</var>] [<code>-h</code> <var>HEAD</var>]</p>

 <h2 id="DESCRIPTION">DESCRIPTION</h2>

@ -175,11 +175,17 @@ your GitHub login. With <code>-p</code>, use private remote
 Forks the original project (referenced by "origin" remote) on GitHub and
 adds a new remote for it under your username. Requires <code>github.token</code> to
 be set (see CONFIGURATION).</p></li>
-<li><p><code>git pull-request</code> [<var>TITLE</var>] [<code>-b</code> <var>BASE</var>] [<code>-h</code> <var>HEAD</var>]:<br />
+<li><p><code>git pull-request</code> [<code>-f</code>] [<var>TITLE</var>] [<code>-b</code> <var>BASE</var>] [<code>-h</code> <var>HEAD</var>]:<br />
 Opens a pull request on GitHub for the project that the "origin" remote
 points to. The default head of the pull request is the current branch.
 Both base and head of the pull request can be explicitly given in one of
-the following formats: "branch", "owner:branch", "owner/repo:branch".</p>
+the following formats: "branch", "owner:branch", "owner/repo:branch".
+This command will abort operation if it detects that the current topic
+branch has local commits that are not yet pushed to its upstream branch
+on the remote. To skip this check, use <code>-f</code>.</p>
+
+<p>If <var>TITLE</var> is omitted, a text editor will open in which title and body of
+the pull request can be entered in the same manner as git commit message.</p>

 <p>If instead of normal <var>TITLE</var> a string in "#<var>NUMBER</var>" format is given,
 the pull request will be attached to an existing GitHub issue whose ID
@ -307,11 +313,12 @@ $ git apply https://gist.github.com/8da7fb575debd88c54cf
 <h3 id="git-pull-request">git pull-request</h3>

 <pre><code># while on a topic branch called "feature":
-$ git pull-request "I've implemented feature X"
+$ git pull-request
+[ opens text editor to edit title &amp; body for the request ]
 [ opened pull request on GitHub for "YOUR_USER:feature" ]

-# explicit pull base &amp; head:
-$ git pull-request -b defunkt:master -h mislav:feature
+# explicit title, pull base &amp; head:
+$ git pull-request "I've implemented feature X" -b defunkt:master -h mislav:feature

 $ git pull-request #123
 [ attached pull request to issue #123 ]
--- a/man/hub.1.ronn
+++ b/man/hub.1.ronn
@ -20,7 +20,7 @@ hub(1) -- git + hub = github
 `git compare` [`-u`] [<USER>] [<START>...]<END>  
 `git submodule add` [`-p`] <OPTIONS> [<USER>/]<REPOSITORY> <DIRECTORY>  
 `git fork` [`--no-remote`]  
-`git pull-request` [<TITLE>] [`-b` <BASE>] [`-h` <HEAD>]  
+`git pull-request` [`-f`] [<TITLE>] [`-b` <BASE>] [`-h` <HEAD>]  

 ## DESCRIPTION

@ -116,11 +116,17 @@ alias command displays information on configuring your environment:
    adds a new remote for it under your username. Requires `github.token` to
    be set (see CONFIGURATION).

-  * `git pull-request` [<TITLE>] [`-b` <BASE>] [`-h` <HEAD>]:  
+  * `git pull-request` [`-f`] [<TITLE>] [`-b` <BASE>] [`-h` <HEAD>]:  
    Opens a pull request on GitHub for the project that the "origin" remote
    points to. The default head of the pull request is the current branch.
    Both base and head of the pull request can be explicitly given in one of
    the following formats: "branch", "owner:branch", "owner/repo:branch".
+    This command will abort operation if it detects that the current topic
+    branch has local commits that are not yet pushed to its upstream branch
+    on the remote. To skip this check, use `-f`.
+
+    If <TITLE> is omitted, a text editor will open in which title and body of
+    the pull request can be entered in the same manner as git commit message.

    If instead of normal <TITLE> a string in "#<NUMBER>" format is given,
    the pull request will be attached to an existing GitHub issue whose ID
--- a/test/hub_test.rb
+++ b/test/hub_test.rb
@ -45,7 +45,7 @@ class HubTest < Test::Unit::TestCase
      'config github.token'  => 'abc123',
      'config --get-all remote.origin.url' => 'git://github.com/defunkt/hub.git',
      'config --get-all remote.mislav.url' => 'git://github.com/mislav/hub.git',
-      "name-rev refs/heads/master@{upstream} --name-only --refs='refs/remotes/*' --no-undefined" => 'remotes/origin/master',
+      "name-rev master@{upstream} --name-only --refs='refs/remotes/*' --no-undefined" => 'remotes/origin/master',
      'config --bool hub.http-clone' => 'false',
      'rev-parse --git-dir' => '.git'
    )
@ -638,6 +638,14 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
+    assert_output expected, "pull-request hereyougo -f"
+  end
+
+  def test_pullrequest_with_checks
+    @git["rev-list --cherry origin/master..."] = "+abcd1234\n+bcde2345"
+
+    expected = "Aborted: 2 commits are not yet pushed to origin/master\n" <<
+      "(use `-f` to force submit a pull request anyway)\n"
    assert_output expected, "pull-request hereyougo"
  end

@ -649,7 +657,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
-    assert_output expected, "pull-request hereyougo"
+    assert_output expected, "pull-request hereyougo -f"
  end

  def test_pullrequest_from_tracking_branch
@ -660,7 +668,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
-    assert_output expected, "pull-request hereyougo"
+    assert_output expected, "pull-request hereyougo -f"
  end

  def test_pullrequest_explicit_head
@ -669,7 +677,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
-    assert_output expected, "pull-request hereyougo -h yay-feature"
+    assert_output expected, "pull-request hereyougo -h yay-feature -f"
  end

  def test_pullrequest_explicit_head_with_owner
@ -678,7 +686,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
-    assert_output expected, "pull-request hereyougo -h mojombo:feature"
+    assert_output expected, "pull-request hereyougo -h mojombo:feature -f"
  end

  def test_pullrequest_explicit_base
@ -687,7 +695,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
-    assert_output expected, "pull-request hereyougo -b feature"
+    assert_output expected, "pull-request hereyougo -b feature -f"
  end

  def test_pullrequest_explicit_base_with_owner
@ -696,7 +704,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
-    assert_output expected, "pull-request hereyougo -b mojombo:feature"
+    assert_output expected, "pull-request hereyougo -b mojombo:feature -f"
  end

  def test_pullrequest_explicit_base_with_repo
@ -705,7 +713,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(1))

    expected = "https://github.com/defunkt/hub/pull/1\n"
-    assert_output expected, "pull-request hereyougo -b mojombo/hubbub:feature"
+    assert_output expected, "pull-request hereyougo -b mojombo/hubbub:feature -f"
  end

  def test_pullrequest_existing_issue
@ -714,7 +722,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(92))

    expected = "https://github.com/defunkt/hub/pull/92\n"
-    assert_output expected, "pull-request #92"
+    assert_output expected, "pull-request #92 -f"
  end

  def test_pullrequest_existing_issue_url
@ -723,7 +731,7 @@ class HubTest < Test::Unit::TestCase
      to_return(:body => mock_pullreq_response(92, 'mojombo/hub'))

    expected = "https://github.com/mojombo/hub/pull/92\n"
-    assert_output expected, "pull-request https://github.com/mojombo/hub/issues/92#comment_4"
+    assert_output expected, "pull-request https://github.com/mojombo/hub/issues/92#comment_4 -f"
  end

  def test_version
@ -1005,7 +1013,7 @@ config

    def stub_tracking(from, remote_name, remote_branch)
      value = remote_branch ? "remotes/#{remote_name}/#{remote_branch}" : nil
-      @git["name-rev refs/heads/#{from}@{upstream} --name-only --refs='refs/remotes/*' --no-undefined"] = value
+      @git["name-rev #{from}@{upstream} --name-only --refs='refs/remotes/*' --no-undefined"] = value
    end

    def stub_tracking_nothing(from = 'master')