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.

Sign up for GitHub

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
Open

Fix path with space in command for Aquamacs #24

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c2d08f
Fix path with space in command for Aquamacs
Nov 6, 2017
fa99f06
Fix path with space in command for Aquamacs _2
Apr 3, 2018
e3e496a
Fix command when feedback-limit is nil
Apr 3, 2018
8fb57d9
Revert "Fix path with space in command for Aquamacs _0"
Apr 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions visual-regexp-steroids.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:

visual-regexp-steroids.el/visual-regexp-steroids.el

Line 296 in a6420b2

(vr--get-command)

and

visual-regexp-steroids.el/visual-regexp-steroids.el

Line 315 in a6420b2

(vr--get-command)

(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.

(file-name-directory load-file-name)))))

(defcustom vr/command-python vr--command-python-default
"External command used for the Python engine."
Expand Down Expand Up @@ -295,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!

(format "--feedback-limit %s" feedback-limit)
"")
(if forward "" "--backwards"))
(lambda (output)
(vr--parse-matches
Expand Down