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

use smart case instead of ignore case #13

Closed mbunkus closed 7 years ago

mbunkus commented 7 years ago

Please consider using rg's "smart case" mode (-S or --smart-case). It searches case-insensitively if the pattern is all-lowercase and case-sensitively if there's at least one upper-case letter in the pattern. Or make that configurable.

The advantage is not having to search multiple times if I know that I want exact case matching in advance.

At the moment I have to replace rg-set-case-sensitivity and rg-rerun-toggle-case. I cannot even use rg-command-line-flags because they're inserted before rg-toggle-command-line-flags, and therefore rg-toggle-command-line-flags (which includes -i) overrides my -S in rg-command-line-flags.

Smart case searching is the default in other similar implementations, notably ack-and-a-half from where I'm migrating.

Thanks for considering it.

dajva commented 7 years ago

Thanks for your report.

rg-set-case-sensitivity implements something that to me seems identical to --smart-case. As long as case-fold-search is non nil an upper case letter will trigger case sensitive search an all lower case will trigger case insensitive search. Could you elaborate a bit on why this implementation does not work for you since I fail to see the difference?

mbunkus commented 7 years ago

Oh… I wasn't aware of that. I have to admit that I simply assumed that smart-matching wasn't used as there was no -S in the sources. I didn't consider rg.el might be the smart one here.

I've now given it a proper try with the original definitions of the two functions I had overwritten, and I can confirm that it works nicely and just as you've described. Sorry for the noise.

dajva commented 7 years ago

No problem. It's an indication that the implementation could improve. The reuse of case-fold-search is and inheritance from grep.el which works well in my personal setup but is not very intuitive. I actually plan to replace this with a dedicated setting for this package. Some related documentation could make the behaviour more obvious.