Fixes for Windows #10
Conversation
There was a problem hiding this comment.
Thanks for the pr.
I am kind of surprised that this is all that is needed for Windows support. :)
I don't use Windows myself and will not officially support anything other than Linux. Patches for other platforms are very welcome though but I can't do any testing.
I will merge this patch after some small changes.
Thanks.
| @@ -207,7 +207,8 @@ for special purposes.") | |||
| (globs (cdr typedef))) | |||
| (mapconcat | |||
| (lambda (glob) | |||
| (concat "--type-add '" name ":" glob "'")) | |||
| (concat "--type-add " | |||
| (shell-quote-argument (concat name ":" glob)))) | |||
There was a problem hiding this comment.
What is the problem on Windows? I assume it's the '*' in the globs that needs to be escaped or is it more? When running this on linux I see that the ':'s are escaped as well.
Elsip documentation (36.2 Shell Arguments) indicates that escaping shall be done in order to make sure file names are detected properly. Maybe it's better to only escape the glob var if it's enough on Wiindows to avoid potential problems?
There was a problem hiding this comment.
BTW, I think this also needs to be done on the custom var in rg-build-template in case you search in a custom file pattern that is not part of rg or rg-custom-type-aliases.
Try invoking rg and chose a custom glob instead of an exisiting aliases at the files prompt, e.g. *.el to see if it works.
There was a problem hiding this comment.
The extra escapes don't hurt (i.e. they are effectively no-ops), and I'm not sure if escaping just the glob piece would work for Windows--I haven't tried, but this way definitely works, and results in a reasonable command line if the user chooses to edit it before invocation via C-u M-x rg; the escaped parameters look like gn\:\*.gn or "gn:*.gn" on Linux and Windows, respectively.
And yes, I think you're right that the custom also needs to be escaped--I tried just now and the argument gets single-quoted, which won't work for Windows, so I'll fix that.
There was a problem hiding this comment.
Yeah, it seems to work on linux so let's keep it.
rg.el
Outdated
| @@ -734,7 +735,7 @@ This command shares argument histories with \\[rgrep] and \\[grep]." | |||
| (setq command nil)) | |||
| (setq dir (file-name-as-directory (expand-file-name dir))) | |||
| (rg-set-case-sensitivity regexp) | |||
| (setq command (rg-build-command regexp files)) | |||
| (setq command (concat (rg-build-command regexp files) " .")) | |||
There was a problem hiding this comment.
Adding the path needs to be done in the rg-build-command defun in order to be applied to rg-rerun-* defuns so please move this to the end of that defun.
|
I pushed a new version with the requested changes. |
|
I can't see any new changes on your branch. |
|
git is harder than actually writing software. Do you see it now? |
|
Yeah, I see it now. Thanks. |
No description provided.