dajva / rg.el

Emacs search tool based on ripgrep
https://rgel.readthedocs.io
GNU General Public License v3.0
475 stars 41 forks source link

Fixes for Windows (continued) #11

Closed drvink closed 7 years ago

drvink commented 7 years ago

When rg is invoked on Windows via Emacs, its stdin is not a console, which causes rg to suppress line numbers by default, causing our regexes to not match. Pass -n so that line numbers are always included in the output.

drvink commented 7 years ago

This also fixes the fact that I am an idiot: I think I must have pushed an older commit by mistake in #10, since it was missing the diff chunk that added quoting of the custom --type glob that you had mentioned would also need fixing.

dajva commented 7 years ago

Thanks. Obviously I failed equally hard checking your follow up commit. :)

Tests needs to be fixed though. Should be as simple as adding shell-qoute-argument at corresponding places in rg.el-test.el.

drvink commented 7 years ago

Unfortunately, not quite as easy as that due to the regexes not matching against the full arguments that we escape, but the relevant tests should now pass even on command lines using the Windows-style escaping.

dajva commented 7 years ago

Ah, you probably need to use regexp-quote as well, i.e. something like: (regexp-quote (shell-quote-argument "custom:")).

But I think these tests need some cleanup and fixing anyway so unless you really want to dig into this I can fix this myself later.

drvink commented 7 years ago

Quoting the regex isn't the problem; it's that (shell-quote-argument "custom:") produces custom\: for Unix, but "custom:" for Windows, so unless we were quoting an entire argument, the resulting regex would have a stray " in the middle (and therefore those tests would fail on Windows, if someone were to ever run them). Probably not a big deal, but that's why I hand-wrote some of the regexes--I did use regexp-quote and shell-quote-argument where possible.

dajva commented 7 years ago

Ah, yeah. The tests would need to change a bit as well. Anyway, thanks for the pr.