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

Correct escaping of quotes and double quotes. (#89, #86) #93

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
9 participants
@aucl

aucl commented Nov 19, 2012

This fixes an issue that occured when the commit message/annotation
contains a double-quote.

For some reason the original code replaced a ' with '\''.
the changed code replaces a ' with \' and a " with \".

this is related to issue #89, #86

Correct escaping of single- and double-quotes.
This fixes an issue that occured when the commit message/annotation
contains a double-quote.

For some reason the original code replaced a `'` with `'\''`.
the changed code replaces a `'` with `\'` and a `"` with `\"`.
@@ -330,7 +330,7 @@ def verify_working_tree_is_clean
end
def escape_quotes(str)
str.gsub("'", "'\\\\''")
str.gsub("'", "\\\\'").gsub('"', '\\\\"')

This comment has been minimized.

@aucl

aucl Nov 19, 2012

alternatively it could be also used:
str.gsub(/'|"/) { |c| "\\#{c}" }
which is probably nicer looking.

should backslashes \ also be escaped?

@aucl

aucl Nov 19, 2012

alternatively it could be also used:
str.gsub(/'|"/) { |c| "\\#{c}" }
which is probably nicer looking.

should backslashes \ also be escaped?

This comment has been minimized.

@nirvdrum

nirvdrum Mar 10, 2014

Owner

I'm going to use this for now. I tried to replace everything with Shellwords and even backported it for Ruby 1.8 support, but Shellwords doesn't work on Windows. I'm still evaluating options there, but in the meanwhile I'll fix the quote escaping.

@nirvdrum

nirvdrum Mar 10, 2014

Owner

I'm going to use this for now. I tried to replace everything with Shellwords and even backported it for Ruby 1.8 support, but Shellwords doesn't work on Windows. I'm still evaluating options there, but in the meanwhile I'll fix the quote escaping.

@leandro-lucarella-sociomantic

This comment has been minimized.

Show comment
Hide comment
@leandro-lucarella-sociomantic

leandro-lucarella-sociomantic Jan 23, 2013

Any reasons why this is not merged? I just hit this bug.

leandro-lucarella-sociomantic commented Jan 23, 2013

Any reasons why this is not merged? I just hit this bug.

@mesozoic

This comment has been minimized.

Show comment
Hide comment
@mesozoic

mesozoic Jan 25, 2013

I just hit this problem as well.

mesozoic commented Jan 25, 2013

I just hit this problem as well.

@jhkrischel

This comment has been minimized.

Show comment
Hide comment
@jhkrischel

jhkrischel Mar 7, 2013

I found I needed this (ruby 1.8.7 (2012-02-08 patchlevel 358) [universal-darwin12.0]), git version 1.8.1.3):

str.gsub!("\\", "\\\\\\").gsub!("'", "\\\\'").gsub!('"', '\\\\"')

Not sure why the "!" was necessary, but that's what ended up working for me.

jhkrischel commented Mar 7, 2013

I found I needed this (ruby 1.8.7 (2012-02-08 patchlevel 358) [universal-darwin12.0]), git version 1.8.1.3):

str.gsub!("\\", "\\\\\\").gsub!("'", "\\\\'").gsub!('"', '\\\\"')

Not sure why the "!" was necessary, but that's what ended up working for me.

@jsdalton

This comment has been minimized.

Show comment
Hide comment
@jsdalton

jsdalton Mar 19, 2013

I hit this bug as well when double quote is in a repository commit message. I have a patch that's almost exactly like this one and it works now.

jsdalton commented Mar 19, 2013

I hit this bug as well when double quote is in a repository commit message. I have a patch that's almost exactly like this one and it works now.

@cfoellmann

This comment has been minimized.

Show comment
Hide comment
@cfoellmann

cfoellmann May 23, 2013

I can say that the inital code from the pull request
str.gsub("'", "\\\\'").gsub('"', '\\\\"')
is working for me, too.

Should be integrated in the release.

cfoellmann commented May 23, 2013

I can say that the inital code from the pull request
str.gsub("'", "\\\\'").gsub('"', '\\\\"')
is working for me, too.

Should be integrated in the release.

@svscorp

This comment has been minimized.

Show comment
Hide comment
@svscorp

svscorp Jun 17, 2013

works for me:

str.gsub("'", "\\\\'").gsub('"', '\\\\"')

Ship it!

svscorp commented Jun 17, 2013

works for me:

str.gsub("'", "\\\\'").gsub('"', '\\\\"')

Ship it!

@Pluies

This comment has been minimized.

Show comment
Hide comment
@Pluies

Pluies Nov 24, 2013

Just ran into the same issue, the pull requests fixes it. It would be great if it could be merged. :)

Pluies commented Nov 24, 2013

Just ran into the same issue, the pull requests fixes it. It would be great if it could be merged. :)

@nirvdrum

This comment has been minimized.

Show comment
Hide comment
@nirvdrum

nirvdrum Mar 10, 2014

Owner

I ended up fixing this slightly differently than this PR did, so I'm going to close this out. But thanks for the contribution, since it spurred the solution. I also added a little test suite to make sure this doesn't regress. Check out svn2git 2.2.5 for the fix.

Owner

nirvdrum commented Mar 10, 2014

I ended up fixing this slightly differently than this PR did, so I'm going to close this out. But thanks for the contribution, since it spurred the solution. I also added a little test suite to make sure this doesn't regress. Check out svn2git 2.2.5 for the fix.

@nirvdrum nirvdrum closed this Mar 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment