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

Fix path with space in command for Aquamacs #24

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

muyinliu
Copy link

@muyinliu muyinliu commented Nov 6, 2017

use shell-quote-argument instead.

@@ -46,7 +46,9 @@
;;; variables

(defvar vr--command-python-default
(format "python %s" (expand-file-name "regexp.py" (file-name-directory load-file-name))))
(format "python %s"
(shell-quote-argument (expand-file-name "regexp.py"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right place to put shell-quote-argument. You should rather call it when the process is called:


and

(like the other arguments that are quoted).

Tip: you can git amend your commit and force push it to the same branch to update an existing pull request. This way, you don't have to open a new PR for every iteration.

Please also drop "for Aquaemacs" from the commit title, as this fix is not about Aquaemacs. Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, those are the wrong places. shell-quote-argument needs to be done before vr--get-command (and vr/command-python) is constructed otherwise you get something like:

(shell-quote-argument (vr--get-command))
"python\\ /Users/nega/Library/Preferences/Aquamacs\\ Emacs/Packages/elpa/visual-regexp-steroids-20170222.253/regexp.py"

which is obviously incorrect.

Copy link
Owner

@benma benma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check out my comments on the PR from November:

#24 (comment)

@@ -297,7 +297,9 @@ and the message line."
(format "%s matches --regexp %s %s %s"
(vr--get-command)
(shell-quote-argument regexp-string)
(when feedback-limit (format "--feedback-limit %s" feedback-limit))
(if feedback-limit
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

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.

3 participants