Merge pull request #4079 from fwalch/vim-patch/review-improvements

vim-patch.sh: Improvements for review functionality, submission option.

1 files changed, 167 insertions, 64 deletions

diff --git a/scripts/vim-patch.sh b/scripts/vim-patch.sh
index bdd3d6209b..7612a2ada0 100755
--- a/scripts/vim-patch.sh
+++ b/scripts/vim-patch.sh
@@ -5,9 +5,12 @@ set -u
 set -o pipefail
 
 readonly NEOVIM_SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
-readonly VIM_SOURCE_DIR_DEFAULT=${NEOVIM_SOURCE_DIR}/.vim-src
+readonly VIM_SOURCE_DIR_DEFAULT="${NEOVIM_SOURCE_DIR}/.vim-src"
 readonly VIM_SOURCE_DIR="${VIM_SOURCE_DIR:-${VIM_SOURCE_DIR_DEFAULT}}"
 readonly BASENAME="$(basename "${0}")"
+readonly BRANCH_PREFIX="vim-"
+
+CREATED_FILES=()
 
 usage() {
   echo "Helper script for porting Vim patches. For more information, see"
@@ -21,6 +24,7 @@ usage() {
   echo "    -p {vim-revision}  Download and apply the Vim patch vim-revision."
   echo "                       vim-revision can be a version number of the "
   echo "                       format '7.4.xxx' or a Git commit hash."
+  echo "    -s                 Submit a vim-patch pull request to Neovim."
   echo "    -r {pr-number}     Review a vim-patch pull request to Neovim."
   echo
   echo "Set VIM_SOURCE_DIR to change where Vim's sources are stored."
@@ -35,6 +39,27 @@ check_executable() {
   fi
 }
 
+clean_files() {
+  if [[ ${#CREATED_FILES[@]} -eq 0 ]]; then
+    return
+  fi
+
+  echo
+  echo "Created files:"
+  local file
+  for file in ${CREATED_FILES[@]}; do
+    echo "  • ${file}"
+  done
+
+  read -p "Delete these files (Y/n)? " -n 1 -r reply
+  echo
+  if [[ "${reply}" =~ ^[Yy]$ ]]; then
+    rm -- ${CREATED_FILES[@]}
+  else
+    echo "You can use 'git clean' to remove these files when you're done."
+  fi
+}
+
 get_vim_sources() {
   check_executable git
 
@@ -66,22 +91,26 @@ assign_commit_details() {
     # Interpret parameter as version number (tag).
     vim_version="${1}"
     vim_tag="v${1}"
-    vim_commit=$( cd "${VIM_SOURCE_DIR}" \
-      && git log -1 --format="%H" ${vim_tag} )
+    vim_commit=$(cd "${VIM_SOURCE_DIR}" \
+      && git log -1 --format="%H" ${vim_tag})
     local strip_commit_line=true
   else
     # Interpret parameter as commit hash.
     vim_version="${1:0:7}"
-    vim_commit="${1}"
+    vim_commit=$(cd "${VIM_SOURCE_DIR}" \
+      && git log -1 --format="%H" ${vim_version})
     local strip_commit_line=false
   fi
 
   vim_commit_url="https://github.com/vim/vim/commit/${vim_commit}"
-  vim_message="$(git log -1 --pretty='format:%B' "${vim_commit}")"
+  vim_message="$(cd "${VIM_SOURCE_DIR}" \
+    && git log -1 --pretty='format:%B' "${vim_commit}" \
+      | sed -e 's/\(#[0-9]*\)/vim\/vim\1/g')"
   if [[ ${strip_commit_line} == "true" ]]; then
     # Remove first line of commit message.
     vim_message="$(echo "${vim_message}" | sed -e '1d')"
   fi
+  patch_file="vim-${vim_version}.patch"
 }
 
 get_vim_patch() {
@@ -96,66 +125,108 @@ get_vim_patch() {
   echo
   echo "✔ Found Vim revision '${vim_commit}'."
 
-  # Collect patch details and store into variables.
-  vim_full="$(git show -1 --pretty=medium "${vim_commit}")"
   # Patch surgery: preprocess the patch.
   #   - transform src/ paths to src/nvim/
-  vim_diff="$(git show -1 "${vim_commit}" \
+  local vim_full="$(git show -1 --pretty=medium "${vim_commit}" \
     | LC_ALL=C sed -e 's/\( [ab]\/src\)/\1\/nvim/g')"
-  neovim_message="$(commit_message)"
-  neovim_pr="
-\`\`\`
-${vim_message}
-\`\`\`
-
-${vim_commit_url}
-
-Original patch:
-
-\`\`\`diff
-${vim_diff}
-\`\`\`"
-  neovim_branch="vim-${vim_version}"
+  local neovim_branch="${BRANCH_PREFIX}${vim_version}"
 
-  echo
-  echo "Creating Git branch."
   cd "${NEOVIM_SOURCE_DIR}"
-  output="$(git checkout -b "${neovim_branch}" 2>&1)" &&
-    echo "✔ ${output}" ||
-    (echo "✘ ${output}"; false)
+  local checked_out_branch="$(git rev-parse --abbrev-ref HEAD)"
+  if [[ "${checked_out_branch}" == ${BRANCH_PREFIX}* ]]; then
+    echo "✔ Current branch '${checked_out_branch}' seems to be a vim-patch"
+    echo "  branch; not creating a new branch."
+  else
+    echo
+    echo "Fetching 'upstream/master'."
+    output="$(git fetch upstream master 2>&1)" &&
+      echo "✔ ${output}" ||
+      (echo "✘ ${output}"; false)
+
+    echo
+    echo "Creating new branch '${neovim_branch}' based on 'upstream/master'."
+    cd "${NEOVIM_SOURCE_DIR}"
+    output="$(git checkout -b "${neovim_branch}" upstream/master 2>&1)" &&
+      echo "✔ ${output}" ||
+      (echo "✘ ${output}"; false)
+  fi
 
   echo
   echo "Creating empty commit with correct commit message."
-  output="$(git commit --allow-empty --file 2>&1 - <<< "${neovim_message}")" &&
+  output="$(commit_message | git commit --allow-empty --file 2>&1 -)" &&
     echo "✔ ${output}" ||
     (echo "✘ ${output}"; false)
 
   echo
   echo "Creating files."
-  echo "${vim_diff}" > "${NEOVIM_SOURCE_DIR}/${neovim_branch}.diff"
-  echo "✔ Saved diff to '${NEOVIM_SOURCE_DIR}/${neovim_branch}.diff'."
-  echo "${vim_full}" > "${NEOVIM_SOURCE_DIR}/${neovim_branch}.patch"
-  echo "✔ Saved full commit details to '${NEOVIM_SOURCE_DIR}/${neovim_branch}.patch'."
-  echo "${neovim_pr}" > "${NEOVIM_SOURCE_DIR}/${neovim_branch}.pr"
-  echo "✔ Saved suggested PR description to '${NEOVIM_SOURCE_DIR}/${neovim_branch}.pr'."
-  echo "You can use 'git clean' to remove these files when you're done."
+  echo "${vim_full}" > "${NEOVIM_SOURCE_DIR}/${patch_file}"
+  echo "✔ Saved full commit details to '${NEOVIM_SOURCE_DIR}/${patch_file}'."
 
   echo
   echo "Instructions:"
   echo
   echo "  Proceed to port the patch."
-  echo "  You might want to try 'patch -p1 < ${neovim_branch}.diff' first."
+  echo "  You might want to try 'patch -p1 < ${patch_file}' first."
+  echo
+  echo "  If the patch contains a new test, consider porting it to Lua."
+  echo "  You might want to try 'scripts/legacy2luatest.pl'."
   echo
   echo "  Stage your changes ('git add ...') and use 'git commit --amend' to commit."
   echo
-  echo "  Push your changes with 'git push origin ${neovim_branch}' and create a"
-  echo "  pull request called '[RFC] vim-patch:${vim_version}'. You might want "
-  echo "  to use the text in '${neovim_branch}.pr' as the description of this pull request."
+  echo "  To port additional patches related to ${vim_version} and add them to the current"
+  echo "  branch, call '${BASENAME} -p' again.  Please use this only if it wouldn't make"
+  echo "  sense to send in each patch individually, as it will increase the size of the"
+  echo "  pull request and make it harder to review."
+  echo
+  echo "  When you are finished, use '${BASENAME} -s' to submit a pull request."
   echo
   echo "  See https://github.com/neovim/neovim/wiki/Merging-patches-from-upstream-vim"
   echo "  for more information."
 }
 
+submit_pr() {
+  check_executable git
+  check_executable hub
+
+  cd "${NEOVIM_SOURCE_DIR}"
+  local checked_out_branch="$(git rev-parse --abbrev-ref HEAD)"
+  if [[ "${checked_out_branch}" != ${BRANCH_PREFIX}* ]]; then
+    echo "✘ Current branch '${checked_out_branch}' doesn't seem to be a vim-patch branch."
+    exit 1
+  fi
+
+  local pr_body="$(git log --reverse --format='#### %s%n%n%b%n' upstream/master..HEAD)"
+  local patches=("$(git log --reverse --format='%s' upstream/master..HEAD)")
+  patches=(${patches[@]//vim-patch:}) # Remove 'vim-patch:' prefix for each item in array.
+  local pr_title="${patches[@]}" # Create space-separated string from array.
+  pr_title="${pr_title// /,}" # Replace spaces with commas.
+
+  local pr_message="$(printf '[RFC] vim-patch:%s\n\n%s\n' "${pr_title#,}" "${pr_body}")"
+
+  echo "Pushing to 'origin/${checked_out_branch}'."
+  output="$(git push origin "${checked_out_branch}" 2>&1)" &&
+    echo "✔ ${output}" ||
+    (echo "✘ ${output}"; git reset --soft HEAD^1; false)
+
+  echo
+  echo "Creating pull request."
+  output="$(hub pull-request -F - 2>&1 <<< "${pr_message}")" &&
+    echo "✔ ${output}" ||
+    (echo "✘ ${output}"; false)
+
+  echo
+  echo "Cleaning up files."
+  local patch_file
+  for patch_file in ${patches[@]}; do
+    patch_file="vim-${patch_file}.patch"
+    if [[ ! -f "${NEOVIM_SOURCE_DIR}/${patch_file}" ]]; then
+      continue
+    fi
+    rm -- "${NEOVIM_SOURCE_DIR}/${patch_file}"
+    echo "✔ Removed '${NEOVIM_SOURCE_DIR}/${patch_file}'."
+  done
+}
+
 list_vim_patches() {
   get_vim_sources
 
@@ -189,7 +260,8 @@ list_vim_patches() {
   echo "Instructions:"
   echo
   echo "  To port one of the above patches to Neovim, execute"
-  echo "  this script with the patch revision as argument."
+  echo "  this script with the patch revision as argument and"
+  echo "  follow the instructions."
   echo
   echo "  Examples: '${BASENAME} -p 7.4.487'"
   echo "            '${BASENAME} -p 1e8ebf870720e7b671f98f22d653009826304c4f'"
@@ -198,32 +270,28 @@ list_vim_patches() {
   echo "        Out-of-order patches increase the possibility of bugs."
 }
 
-review_pr() {
-  check_executable curl
-  check_executable nvim
-
-  get_vim_sources
-
-  local pr="${1}"
-  echo
-  echo "Downloading data for pull request #${pr}."
+review_commit() {
+  local neovim_commit_url="${1}"
+  local neovim_patch_url="${neovim_commit_url}.patch"
 
   local git_patch_prefix='Subject: \[PATCH\] '
-  local neovim_patch="$(curl -Ssf "https://patch-diff.githubusercontent.com/raw/neovim/neovim/pull/${pr}.patch")"
-  echo "${neovim_patch}" > a
+  local neovim_patch="$(curl -Ssf "${neovim_patch_url}")"
   local vim_version="$(head -n 4 <<< "${neovim_patch}" | sed -n "s/${git_patch_prefix}vim-patch:\([a-z0-9.]*\)$/\1/p")"
 
+  echo
   if [[ -n "${vim_version}" ]]; then
     echo "✔ Detected Vim patch '${vim_version}'."
   else
     echo "✘ Could not detect the Vim patch number."
-    echo "  This script assumes that the PR contains a single commit"
-    echo "  with 'vim-patch:XXX' as its title."
+    echo "  This script assumes that the PR contains only commits"
+    echo "  with 'vim-patch:XXX' in their title."
     exit 1
   fi
 
   assign_commit_details "${vim_version}"
 
+  local vim_patch_url="${vim_commit_url}.patch"
+
   local expected_commit_message="$(commit_message)"
   local message_length="$(wc -l <<< "${expected_commit_message}")"
   local commit_message="$(tail -n +4 <<< "${neovim_patch}" | head -n "${message_length}")"
@@ -235,26 +303,57 @@ review_pr() {
     echo "${expected_commit_message}"
     echo "  Actual:"
     echo "${commit_message#${git_patch_prefix}}"
-    exit 1
   fi
 
-  local base_name="vim-${vim_version}"
   echo
   echo "Creating files."
-  curl -Ssfo "${NEOVIM_SOURCE_DIR}/n${base_name}.diff" "https://patch-diff.githubusercontent.com/raw/neovim/neovim/pull/${pr}.diff"
-  echo "✔ Saved pull request diff to '${NEOVIM_SOURCE_DIR}/n${base_name}.diff'."
-  echo "${neovim_patch}" > "${NEOVIM_SOURCE_DIR}/n${base_name}.patch"
-  echo "✔ Saved full pull request commit details to '${NEOVIM_SOURCE_DIR}/n${base_name}.patch'."
-  git show "${vim_commit}" > "${NEOVIM_SOURCE_DIR}/${base_name}.diff"
-  echo "✔ Saved Vim diff to '${NEOVIM_SOURCE_DIR}/${base_name}.diff'."
-  echo "You can use 'git clean' to remove these files when you're done."
+  echo "${neovim_patch}" > "${NEOVIM_SOURCE_DIR}/n${patch_file}"
+  echo "✔ Saved pull request diff to '${NEOVIM_SOURCE_DIR}/n${patch_file}'."
+  CREATED_FILES+=("${NEOVIM_SOURCE_DIR}/n${patch_file}")
+
+  curl -Ssfo "${NEOVIM_SOURCE_DIR}/${patch_file}" "${vim_patch_url}"
+  echo "✔ Saved Vim diff to '${NEOVIM_SOURCE_DIR}/${patch_file}'."
+  CREATED_FILES+=("${NEOVIM_SOURCE_DIR}/${patch_file}")
 
   echo
   echo "Launching nvim."
-  exec nvim -O "${NEOVIM_SOURCE_DIR}/${base_name}.diff" "${NEOVIM_SOURCE_DIR}/n${base_name}.diff"
+  nvim -c "cd ${NEOVIM_SOURCE_DIR}" \
+    -O "${NEOVIM_SOURCE_DIR}/${patch_file}" "${NEOVIM_SOURCE_DIR}/n${patch_file}"
 }
 
-while getopts "hlp:r:" opt; do
+review_pr() {
+  check_executable curl
+  check_executable nvim
+  check_executable jq
+
+  get_vim_sources
+
+  local pr="${1}"
+  echo
+  echo "Downloading data for pull request #${pr}."
+
+  local pr_commit_urls=($(curl -Ssf "https://api.github.com/repos/neovim/neovim/pulls/${pr}/commits" \
+                          | jq -r '.[].html_url'))
+
+  echo "Found ${#pr_commit_urls[@]} commit(s)."
+
+  local pr_commit_url
+  local reply
+  for pr_commit_url in ${pr_commit_urls[@]}; do
+    review_commit "${pr_commit_url}"
+    if [[ "${pr_commit_url}" != "${pr_commit_urls[-1]}" ]]; then
+      read -p "Continue with next commit (Y/n)? " -n 1 -r reply
+      echo
+      if [[ ! "${reply}" =~ ^[Yy]$ ]]; then
+        break
+      fi
+    fi
+  done
+
+  clean_files
+}
+
+while getopts "hlp:r:s" opt; do
   case ${opt} in
     h)
       usage
@@ -272,6 +371,10 @@ while getopts "hlp:r:" opt; do
       review_pr "${OPTARG}"
       exit 0
       ;;
+    s)
+      submit_pr
+      exit 0
+      ;;
     *)
       exit 1
       ;;