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

ag and pt not working on a windows box #99

Closed
syl20bnr opened this issue Apr 14, 2015 · 16 comments
Closed

ag and pt not working on a windows box #99

syl20bnr opened this issue Apr 14, 2015 · 16 comments

Comments

@syl20bnr
Copy link
Contributor

@syl20bnr syl20bnr commented Apr 14, 2015

I recently unified the usage of ag, pt and ack under helm-ag in order to get the helm-ag-edit capability.
This does not seems to work on a windows box. I tracked the issue a bit and was able to take the output of helm-ag--construct-command verbatim and execute it on a cmd box and it works.

Do you have any insight about this issue, is it an output encoding issue ?

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented Apr 15, 2015

Thanks for reporting, I'll check later.

NOTE: Now users have to install pt from source code. See also #46 (comment)

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented Apr 15, 2015

I confirmed that helm-ag does not work on Windows.
I suppose patch of monochromegane/the_platinum_searcher#71 is not enough for Windows environment.

@syl20bnr

This comment has been minimized.

Copy link
Contributor Author

@syl20bnr syl20bnr commented Apr 15, 2015

@ralesi What did you do in helm-pt to make it work on Windows ?

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented Apr 16, 2015

@ralesi

This comment has been minimized.

Copy link

@ralesi ralesi commented Apr 16, 2015

For helm-pt, I just ran the command directly through a command process pipe, similar to helm-do-grep. If we can get helm-ag working, it should be better. @syohex, since there is a suggestion in the README to use pt, do you think we can separate out the helm-sources explicitly with a helm-do-pt definition to change the modeline and the buffer name (i.e. instead of *helm-ag*). I imagine it might be possible to adjust these with a let command prior to running helm-do-ag as well.

@syl20bnr

This comment has been minimized.

Copy link
Contributor Author

@syl20bnr syl20bnr commented Apr 29, 2015

@syohex I tried your PR and it does not seem to work. I reinstalled helm-pt and it works, maybe you could explore the way helm-pt calls pt ?

@rorsach

This comment has been minimized.

Copy link

@rorsach rorsach commented May 17, 2015

Regarding ag not working inside Emacs, on Windows 64bit, I think I see the same issue handled over here:

Wilfred/ag.el#26

It's easy to test. Open a command shell in emacs (M-x shell)

Works:

ag sometext .

Fails:

ag sometext

I wasn't able to get pt working using the same workaround (appending a ".")

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented May 25, 2015

Sorry too late reply.
My patch was merged at 1.7.7. And I tried it on my Windows XP 32bit. M-x helm-ag works but M-x helm-do-ag does not work.

@Kludgy

This comment has been minimized.

Copy link

@Kludgy Kludgy commented May 28, 2015

So this still leaves the outstanding issue pointed out by @rorsach where both ag and pt pipes stall unless . is given as the root.

I don't understand what causes this in both cases but can definitely confirm it is still an issue on 64-bit versions of Windows.

What's the correct way to patch helm-do-ag to provide this additional parameter? For instance, lacking familiarity with elisp I can hack a workaround by appending to helm-ag--construct-do-ag-command:

    (setq cmds (append cmds (list ".")))

This hack solves my immediate issue. But how can I tidy it up? Maybe hook into helm-do-ag--default-target somehow?

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented May 29, 2015

I have merged #112. I confirmed that both helm-ag and helm-do-ag(using pt instead of ag) works well on Windows with that change.

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented Jun 1, 2015

@Kludgy Thanks for information. I don't know why, but path arguments must be specified on Windows when helm-do-ag executes. If omitting them, then pt process is killed immediately.

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented Jun 1, 2015

Please try latest helm-ag and pt 1.7.7 or higher version.

@Kludgy

This comment has been minimized.

Copy link

@Kludgy Kludgy commented Jun 1, 2015

All is well with #112. Thank you kindly!

@rorsach

This comment has been minimized.

Copy link

@rorsach rorsach commented Jun 1, 2015

Validated working with pt 1.7.7 and #112, thank you.

@syohex

This comment has been minimized.

Copy link
Owner

@syohex syohex commented Jun 8, 2015

Please reopen the issue if there are any problems about pt on Windows.

@syohex syohex closed this Jun 8, 2015
@syl20bnr

This comment has been minimized.

Copy link
Contributor Author

@syl20bnr syl20bnr commented Jun 8, 2015

@syohex Nice work thank you 👍
P.S. nice catch @rorsach :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.