Code review comment for lp:~parthm/bzr/503670-vila-grep-as-builtin

Revision history for this message
Parth Malwankar (parthm) wrote :

> As a windows user, I have no problem with the idea of a `bzr grep` that
> depends on an external grep tool, I'd just want you to lose the xargs
> dependency.
>

Having grep.exe shipped with bzr is one option. But for something like history
and view support, we would still need to do use bzrlib API to filter files,
get text of older versions and pipe it to grep. And as you mentioned below, I also
think '--' looks quite ugly so it might be good to process options via bzr's
command line processing.

I feel going with 'bzr ls [opts] | grep' may be a better option than the above. For history we would probably need to use 'bzr cat'?

Considering the above, IMO going with a plugin gives us more flexibility (e.g the --from-root option in the plugin) along with the possibility of easily enhancing it in the future.

> The -- thing is ugly, why not just pull out the bazaar-specific arguments from
> the list given and pass the rest onto the external tool?

I am wondering if there is a consensus on the approach to bug #503670.
The options which seem ok to me are:
 (1) 'bzr ls [opts] | grep': bzr could [based on user selection] install grep.exe on windows. 'bzr cat' could be used for history search.
 (2) self contained grep [a]: This should probably be a plugin (at least to start with).
 (3) ???
Whatever option is chosen should have support for views and history IMO.

Once there is some agreement on the approach thats preferred, we should be
able to close bug #503670

[a] https://code.launchpad.net/~parthm/bzr/503670-grep-plugin/+merge/20067

« Back to merge proposal