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

Windows #858

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
2 participants
@fpopineau
Copy link
Contributor

commented Jan 16, 2017

  • I don't think counsel--git-grep-count need to be set to 0 under windows-nt
  • the MSYS2/MinGW64 version of ag has a problem: it does not search inside the current directory, it is waiting on stdin instead. Under windows-nt, the default-directory argument is mandatory. It is also harmless for other platforms
  • the last change is to tolerate UNC path, aka //server/share/...
@abo-abo

This comment has been minimized.

Copy link
Owner

commented Jan 18, 2017

I don't think counsel--git-grep-count need to be set to 0 under windows-nt

The reason for this is the slowness of starting a new process on Windows, in my experience. Since a new process is spawned each time you press a new key, this makes the UX quite unresponsive at times.
Setting counsel--git-grep-count makes counsel-git-grep believe that the repository is small enough to be loaded into memory fully (i.e. in a single process call), and later Emacs regex can be used to filter each key press, instead of spawning new git-grep processes.

the MSYS2/MinGW64 version of ag has a problem: it does not search inside the current directory, it is waiting on stdin instead. Under windows-nt, the default-directory argument is mandatory. It is also harmless for other platforms

OK, I can merge this.

the last change is to tolerate UNC path, aka //server/share/...

This is an unacceptable UX change for me. // has to move to the root directory. The method for accessing remotes with counsel-find-file is /ssh: RET.

@abo-abo abo-abo closed this in c2b0d30 Jan 18, 2017

@fpopineau

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2017

This is an unacceptable UX change for me. // has to move to the root directory.
The method for accessing remotes with counsel-find-file is /ssh: RET.

Well, the kind of shares UNC path allow to access are not accessible through ssh.
Those shares are more like mount points. Under Unix, Samba provides such shares.
Moreover, UNC paths are acknowledged by POSIX:
http://pubs.opengroup.org/onlinepubs/009695399/xrat/xbd_chap04.html#tag_01_04_11
Emacs itself recognizes this syntax.
It is your right to ignore this syntax, but it is a pity. My patch requires to type /// to move to the root directory. That's not that far from //.

@abo-abo

This comment has been minimized.

Copy link
Owner

commented Jan 18, 2017

My patch requires to type /// to move to the root directory. That's not that far from //.

Yea, except I go to root several times a day, and I haven't heard of UNC even once before.

Why not have /unc: RET provide completion for that?

@fpopineau

This comment has been minimized.

Copy link
Contributor Author

commented Jan 18, 2017

abo-abo added a commit that referenced this pull request Jan 26, 2017

Revert "Add missing parameter for ag"
This reverts commit c2b0d30.

Fixes #861
Re #858
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.