Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve JSON encoding #561

Merged
merged 2 commits into from Jul 30, 2016
Merged

Improve JSON encoding #561

merged 2 commits into from Jul 30, 2016

Conversation

lukechilds
Copy link
Contributor

Allows double quotes in JSON. Currently passing double quotes in will output invalid JSON.

Before this change:

$ json 'Title' 'My PR to "fix" a bug' 'master' 'master'
{
  "title": "Title",
  "body": "My PR to "fix" a bug",
  "head": "master",
  "base": "master"
}

After this change:

$ json 'Title' 'My PR to "fix" a bug' 'master' 'master'
{
  "title": "Title",
  "body": "My PR to \"fix\" a bug",
  "head": "master",
  "base": "master"
}

@nicolaiskogheim
Copy link
Collaborator

nicolaiskogheim commented Jul 30, 2016

Good catch!

Would you mind applying this patch before we merge?

cd $git-extras-repo; patch -p0 < patchfile

--- bin/git-pull-request    2016-07-30 03:35:14.000000000 +0200
+++ bin/git-pull-request    2016-07-30 03:34:35.000000000 +0200
@@ -5,7 +5,7 @@
 #

 abort() {
-  echo $@
+  echo >&2 "$@"
   exit 1
 }

@@ -35,9 +35,7 @@

 # make sure it's pushed

-git push origin $branch || abort "failed to push $branch"
-
-# lame hack to get project
+git push origin "$branch" || abort "failed to push $branch"

 project=$(git config remote.origin.url | sed 's/^.*://' | sed 's/\.git$//')

@@ -46,9 +44,9 @@
 echo
 echo "  create pull-request for $project '$branch'"
 echo
-printf "  title: " && read title
-printf "  body: " && read body
-printf "  base [master]: " && read base
+printf "  title: " && read -r title
+printf "  body: " && read -r body
+printf "  base [master]: " && read -r base
 echo

 # create pull request
@@ -58,7 +56,7 @@
     base="master"
 fi

-body=$(json "$title" "$body" $branch "$base")
+body=$(json "$title" "$body" "$branch" "$base")

 curl -u "$user" "https://api.github.com/repos/$project/pulls" -d "$body"

@nicolaiskogheim
Copy link
Collaborator

Omg. I had some debug stuff in that diff. Cleaning up now.

@lukechilds
Copy link
Contributor Author

Done :)

@hemanth hemanth merged commit 76f23c2 into tj:master Jul 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants