[Fix #1044] Prefer call-process to shell-command-to-string
#1045
Conversation
colonelpanic8
force-pushed
the
avoid_shell_command_to_string
branch
2 times, most recently
from
99e5797
to
bc74472
Compare
|
The tests are failing. |
| @@ -1058,9 +1059,22 @@ they are excluded from the results of this function." | |||
| (let ((cmd (projectile-get-ext-ignored-command))) | |||
| (projectile-files-via-ext-command cmd))) | |||
|
|
|||
| (defun projectile-call-process-to-string (program &rest args) | |||
| (apply 'call-process program nil (current-buffer) nil args) | ||
| (buffer-string))) | ||
|
|
||
| (defun projectile-shell-command-to-string (command) |
| (require 'compile) | ||
| (require 'dash) |
There was a problem hiding this comment.
Rebase on top of the current master - we're no longer using dash.el.
| (let ((command "command arg1 arg2") | ||
| (command-path "/path/to/command") | ||
| shell-command-args call-process-args) | ||
| (noflet ((shell-command-to-string (&rest args) |
There was a problem hiding this comment.
The noflet indentation seems broken. Probably you didn't load noflet before indenting this buffer.
colonelpanic8
force-pushed
the
avoid_shell_command_to_string
branch
from
bc74472
to
3c7c287
Compare
This change makes it so that a direct `call-process` invocation is used when it is possible to determine what binary should be used for the command. This avoids the cost of shell startup time that is incurred when `shell-command-to-string` is invoked instead.
colonelpanic8
force-pushed
the
avoid_shell_command_to_string
branch
from
48a882e
to
1713d62
Compare
|
Thanks for the review. Things should be fixed now! |
|
👍 |
|
Been debugging why listing of submodule project files stopped working in Executing this Gives me this in Messages This MR parses commands naively by splitting via spaces creating an argument list like |
|
File a bug report for this. On Wednesday, 28 September 2016, Brian Suh notifications@github.com wrote:
Best Regards, |
Yeah makes sense. Actually in this case, it seems that thats not the only issue, since the pipe at the end of this command will likely also fail when passed to call process. I wasn't really aware that projectile ran commands that actually rely on shell features like this, and the only obvious way to fix this is to revert this change. The other alternative is to make it so that projectile commands are always specified as lists of arguments, but that seems like a much larger change. |
|
Not all commands rely on this behaviour. I guess only a few commands rely on pipes and probably we can rewrite some on them - e.g. they can be a sequence of two commands or some processing can be done in Elisp instead. |
This change makes it so that a direct
call-processinvocation is usedwhen it is possible to determine what binary should be used for the
command. This avoids the cost of shell startup time that is incurred
when
shell-command-to-stringis invoked instead.Fixes #1044