Add options for common ag arguments

Wilfred / ag.el

An Emacs frontend to The Silver Searcher

http://agel.readthedocs.org/en/latest/

525 stars 61 forks source link

Add options for common ag arguments #118

Closed albertodonato closed 7 years ago

albertodonato commented 8 years ago

Drop the ag-arguments custom (convertig it to an internal variable), in favor of explicit toggles for stats (ag-search-stats) and smart case (ag-smart-case). This makes configurable options more explicit, reducing the risk of breaking search by twaking the command line.

There are also a few cleanups on the logic to build the ag command line.

albertodonato commented 7 years ago

I fixed conflicts with master, please consider merging.

Wilfred commented 7 years ago

Thanks for your PR, but I don't think this is the right approach.

I'm concerned that making ag-arguments no longer a custom will break configurations for users who have customised it.
I think case-sensitivity depends on the individual search, so requiring users to change a setting repeatedly is an awkward workflow. After v1.0, I hope to look at ways of changing case behaviour interactively (without just using a prefix to customise the raw command).

However, I agree that ag-arguments is brittle. Perhaps we should assert that we always have the minimum required arguments.

I hope you understand. I'm sorry for not communicating sooner.

albertodonato commented 7 years ago

@Wilfred, thanks for looking at the PR.

Agreed that changing ag-argument not to be a custom would break users (I've reverted that change in my branch).

WRT case sensitive searches, adding a separate option for it doesn't really change the behavior compared to the current one. It's basically the default behavior to apply, and it's still possible to use the prefix to be prompted with the ag command line and edit it before each run to toggle it (or any other command line switch).

FWIW the reason behind my change is to minimize the need for command-line customization, to reduce the risk of breaking if ag-arguments is changed in the future.