bug#36644: Git log search

bug#36644: Git log search

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 163 bytes --]

It would be very useful to have the command to grep git logs,
for instance, to search commits by bug numbers in format "bug#36789"
and many other such use cases:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-log-search.patch --]
[-- Type: text/x-diff, Size: 2569 bytes --]

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index eb6d6d331f..d39af516a9 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -337,6 +337,10 @@
 ;;   Insert in BUFFER the revision log for the changes that will be
 ;;   received when performing a pull operation from REMOTE-LOCATION.
 ;;
+;; - log-search (pattern)
+;;
+;;   Search for PATTERN in the revision log.
+;;
 ;; - log-view-mode ()
 ;;
 ;;   Mode to use for the output of print-log.  This defaults to
@@ -2526,6 +2530,16 @@ vc-log-outgoing
     (vc-incoming-outgoing-internal backend (or remote-location "")
                                    "*vc-outgoing*" 'log-outgoing)))
 
+;;;###autoload
+(defun vc-log-search (pattern)
+  "Search a log of changes for PATTERN."
+  (interactive (list (read-regexp "Log search pattern: ")))
+  (let ((backend (vc-deduce-backend)))
+    (unless backend
+      (error "Buffer is not version controlled"))
+    (vc-incoming-outgoing-internal backend pattern
+                                   "*vc-search*" 'log-search)))
+
 ;;;###autoload
 (defun vc-log-mergebase (_files rev1 rev2)
   "Show a log of changes between the merge base of REV1 and REV2 revisions.
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 8b82856332..d1d6a7408e 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1073,6 +1075,13 @@ vc-git-log-incoming
 			"@{upstream}"
 		      remote-location))))
 
+(defun vc-git-log-search (buffer pattern)
+  (vc-setup-buffer buffer)
+  (vc-git-command
+   buffer 'async nil
+   "log"
+   "--no-color" "-i" (format "--grep=%s" pattern)))
+
 (defun vc-git-mergebase (rev1 &optional rev2)
   (unless rev2 (setq rev2 "HEAD"))
   (string-trim-right (vc-git--run-command-string nil "merge-base" rev1 rev2)))
@@ -1089,7 +1100,7 @@ vc-git-log-view-mode
   (set (make-local-variable 'log-view-file-re) regexp-unmatchable)
   (set (make-local-variable 'log-view-per-file-logs) nil)
   (set (make-local-variable 'log-view-message-re)
-       (if (not (eq vc-log-view-type 'long))
+       (if (not (memq vc-log-view-type '(long log-search)))
 	   (cadr vc-git-root-log-format)
 	 "^commit *\\([0-9a-z]+\\)"))
   ;; Allow expanding short log entries.
@@ -1098,7 +1109,7 @@ vc-git-log-view-mode
     (set (make-local-variable 'log-view-expanded-log-entry-function)
 	 'vc-git-expanded-log-entry))
   (set (make-local-variable 'log-view-font-lock-keywords)
-       (if (not (eq vc-log-view-type 'long))
+       (if (not (memq vc-log-view-type '(long log-search)))
 	   (list (cons (nth 1 vc-git-root-log-format)
 		       (nth 2 vc-git-root-log-format)))
 	 (append

bug#36644: Git log search

[Dmitry Gutov]

Hi Juri,

On 14.07.2019 1:27, Juri Linkov wrote:
> It would be very useful to have the command to grep git logs,
> for instance, to search commits by bug numbers in format "bug#36789"
> and many other such use cases:

I like the idea.

> +;; - log-search (pattern)
> +;;
> +;;   Search for PATTERN in the revision log.

Is pattern a regexp or a verbatim string? That should be documented. Git 
supports regexps, but maybe we should look at what other backends can 
support as well.

I wonder if the format of the output should be specified as well. E.g. 
by saying that it's the same as for print-log, long version.

> +(defun vc-git-log-search (buffer pattern)
> +  (vc-setup-buffer buffer)
> +  (vc-git-command
> +   buffer 'async nil
> +   "log"
> +   "--no-color" "-i" (format "--grep=%s" pattern)))

Should this use shell-quote-argument?

bug#36644: Git log search

[Juri Linkov]

>> It would be very useful to have the command to grep git logs,
>> for instance, to search commits by bug numbers in format "bug#36789"
>> and many other such use cases:
>
> I like the idea.

Thanks for the review.  Installed to master after fixing
according to your comments.

>> +;; - log-search (pattern)
>> +;;
>> +;;   Search for PATTERN in the revision log.
>
> Is pattern a regexp or a verbatim string? That should be documented.

Fixed to use string.

> Git supports regexps, but maybe we should look at what other backends
> can support as well.

It seems the most compatible type is string.

> I wonder if the format of the output should be specified as well.
> E.g. by saying that it's the same as for print-log, long version.

Fixed by saying it's long version.

Should it support short format as well?

>> +(defun vc-git-log-search (buffer pattern)
>> +  (vc-setup-buffer buffer)
>> +  (vc-git-command
>> +   buffer 'async nil
>> +   "log"
>> +   "--no-color" "-i" (format "--grep=%s" pattern)))
>
> Should this use shell-quote-argument?

Fixed to use shell-quote-argument.

Should it have a key binding?

For example, `vc-log-incoming' is bound to `C-x v I',
`vc-log-outgoing' is bound to key `C-x v O', so logically
`vc-log-search' would be bound to `C-x v s', but unfortunately
it's already taken by `vc-create-tag'.

bug#36644: Git log search

[Dmitry Gutov]

On 16.07.2019 1:27, Juri Linkov wrote:

> Thanks for the review.  Installed to master after fixing
> according to your comments.

Thanks.

>>> +;; - log-search (pattern)
>>> +;;
>>> +;;   Search for PATTERN in the revision log.
>>
>> Is pattern a regexp or a verbatim string? That should be documented.
> 
> Fixed to use string.

Since --grep expects a regexp, shouldn't PATTERN be passed through 
regexp-quote as well? Though it expects Emacs regexps, so it doesn't 
quote parens or pipes.

>> Git supports regexps, but maybe we should look at what other backends
>> can support as well.
> 
> It seems the most compatible type is string.

OK, if that is your conclusion.

>> I wonder if the format of the output should be specified as well.
>> E.g. by saying that it's the same as for print-log, long version.
> 
> Fixed by saying it's long version.
> 
> Should it support short format as well?

I don't know. How would it be used?

> Should it have a key binding?
> 
> For example, `vc-log-incoming' is bound to `C-x v I',
> `vc-log-outgoing' is bound to key `C-x v O', so logically
> `vc-log-search' would be bound to `C-x v s', but unfortunately
> it's already taken by `vc-create-tag'.

'C-x v S', then?

bug#36644: Git log search

[Andreas Schwab]

On Jul 16 2019, Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 16.07.2019 1:27, Juri Linkov wrote:
>
>> Thanks for the review.  Installed to master after fixing
>> according to your comments.
>
> Thanks.
>
>>>> +;; - log-search (pattern)
>>>> +;;
>>>> +;;   Search for PATTERN in the revision log.
>>>
>>> Is pattern a regexp or a verbatim string? That should be documented.
>>
>> Fixed to use string.
>
> Since --grep expects a regexp, shouldn't PATTERN be passed through
> regexp-quote as well?

You can use --fixed-strings.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#36644: Git log search

[Robert Pluim]

>>>>> On Tue, 16 Jul 2019 17:25:39 +0300, Dmitry Gutov <dgutov@yandex.ru> said:
    >>> Git supports regexps, but maybe we should look at what other backends
    >>> can support as well.
    >> 
    >> It seems the most compatible type is string.

    Dmitry> OK, if that is your conclusion.

If the most compatible type is string, surely emacs should do no
regexp-quoting or anything else on the argument, but just pass it
verbatim to the backend search command. That way you could do eg

Bug.*0

to get all log entries with a bug reference ending in 0.

Robert

bug#36644: Git log search

[Juri Linkov]

>>> Is pattern a regexp or a verbatim string? That should be documented.
>>
>> Fixed to use string.
>
> Since --grep expects a regexp, shouldn't PATTERN be passed through
> regexp-quote as well? Though it expects Emacs regexps, so it doesn't quote
> parens or pipes.

Should this still be used when the need is to pass regexps to the backend
search command verbatim?

>>> Git supports regexps, but maybe we should look at what other backends
>>> can support as well.
>>
>> It seems the most compatible type is string.
>
> OK, if that is your conclusion.

I'm still not sure.  Regexps are more useful.

>>> I wonder if the format of the output should be specified as well.
>>> E.g. by saying that it's the same as for print-log, long version.
>>
>> Fixed by saying it's long version.
>>
>> Should it support short format as well?
>
> I don't know. How would it be used?

Short format displays one line per entry and allows expanding
after pressing RET.  OTOH, when using long format we could highlight
all matches in log entries that will be immediately visible
after the command finishes (like in vc-git-grep output buffers).

However, when allowed to use regexp patterns, I don't know how to
highlight matches in git-log output buffer using Emacs regexps
when pattern uses e.g. Perl-compatible regexp allowed in git-log.

>> Should it have a key binding?
>>
>> For example, `vc-log-incoming' is bound to `C-x v I',
>> `vc-log-outgoing' is bound to key `C-x v O', so logically
>> `vc-log-search' would be bound to `C-x v s', but unfortunately
>> it's already taken by `vc-create-tag'.
>
> 'C-x v S', then?

This is good mnemonic keybinding.  The only doubt when adding a new
keybinding is to think if it could be more useful as a prefix key.
Maybe in this case upper-case shifted 'S' is not good as a prefix key.
Otherwise such prefix key could accommodate other vc search related
commands like grep vc files, etc.

bug#36644: Git log search

[Juri Linkov]

>     >>> Git supports regexps, but maybe we should look at what other backends
>     >>> can support as well.
>     >>
>     >> It seems the most compatible type is string.
>
>     Dmitry> OK, if that is your conclusion.
>
> If the most compatible type is string, surely emacs should do no
> regexp-quoting or anything else on the argument, but just pass it
> verbatim to the backend search command. That way you could do eg
>
> Bug.*0
>
> to get all log entries with a bug reference ending in 0.

Currently when using shell-quote-argument it doesn't allow
using such regexps as Bug.*0.  So I guess we need to remove
shell-quote-argument and also to not use regexp-quote.

bug#36644: Git log search

[Noam Postavsky]

Juri Linkov <juri@linkov.net> writes:

>> Bug.*0
>>
>> to get all log entries with a bug reference ending in 0.
>
> Currently when using shell-quote-argument it doesn't allow
> using such regexps as Bug.*0.  So I guess we need to remove
> shell-quote-argument and also to not use regexp-quote.

I think shell-quote-argument is a mistake regardless, vc-git-command
doesn't call a shell, so there is no need for shell-quote-argument.

bug#36644: Git log search

[Dmitry Gutov]

On 16.07.2019 23:15, Juri Linkov wrote:

> Should this still be used when the need is to pass regexps to the backend
> search command verbatim?

What do you mean, "the need"? If it's a verbatim string, we should 
escape characters that have special meaning in regexps (or do like 
Andreas suggested and pass --fixed-string to 'git log'). If it's a 
regexp, we should document it as such.

>>>> Git supports regexps, but maybe we should look at what other backends
>>>> can support as well.
>>>
>>> It seems the most compatible type is string.
>>
>> OK, if that is your conclusion.
> 
> I'm still not sure.  Regexps are more useful.

That is true. If Hg or Bzr support regexp search as well, we might want 
to choose it to be a regexp. And kind of give up on the rest 
(unsupported, etc).

>>>> I wonder if the format of the output should be specified as well.
>>>> E.g. by saying that it's the same as for print-log, long version.
>>>
>>> Fixed by saying it's long version.
>>>
>>> Should it support short format as well?
>>
>> I don't know. How would it be used?
> 
> Short format displays one line per entry and allows expanding
> after pressing RET. 

That would be a different command, or...? Anyway, the idea sounds nice.

> However, when allowed to use regexp patterns, I don't know how to
> highlight matches in git-log output buffer using Emacs regexps
> when pattern uses e.g. Perl-compatible regexp allowed in git-log.

Maybe we shouldn't allow Perl extensions. Or, IDK, give up on 
highlighting in that particular case.

>>> Should it have a key binding?
>>>
>>> For example, `vc-log-incoming' is bound to `C-x v I',
>>> `vc-log-outgoing' is bound to key `C-x v O', so logically
>>> `vc-log-search' would be bound to `C-x v s', but unfortunately
>>> it's already taken by `vc-create-tag'.
>>
>> 'C-x v S', then?
> 
> This is good mnemonic keybinding.  The only doubt when adding a new
> keybinding is to think if it could be more useful as a prefix key.
> Maybe in this case upper-case shifted 'S' is not good as a prefix key.
> Otherwise such prefix key could accommodate other vc search related
> commands like grep vc files, etc.

FWIW, I prefer commands to have shorter bindings, when possible. Using 
prefix keys too much tends to conflict with that.

bug#36644: Git log search

[Robert Pluim]

>>>>> On Thu, 18 Jul 2019 18:01:12 +0300, Dmitry Gutov <dgutov@yandex.ru> said:

    Dmitry> On 16.07.2019 23:15, Juri Linkov wrote:
    >> Should this still be used when the need is to pass regexps to the backend
    >> search command verbatim?

    Dmitry> What do you mean, "the need"? If it's a verbatim string, we should
    Dmitry> escape characters that have special meaning in regexps (or do like
    Dmitry> Andreas suggested and pass --fixed-string to 'git log'). If it's a
    Dmitry> regexp, we should document it as such.

'verbatim' to me means that emacs should not touch it. If the backend
command such as git chooses to interpret that string as a regexp,
thatʼs up to the command. If emacs starts escaping characters in the
string or using --fixed-string, then we limit the functionality to
*only* fixed strings.

Robert

bug#36644: Git log search

[Dmitry Gutov]

[-- Attachment #1: Type: text/html, Size: 1692 bytes --]

bug#36644: Git log search

[Robert Pluim]

>>>>> On Thu, 18 Jul 2019 21:02:48 +0300, Dmitry Gutov <dgutov@yandex.ru> said:

    Dmitry> We can't really use this approach. VC is a high level
    Dmitry> abstraction, so we try to define the semantics well.

What's wrong with the semantics of 'this string is passed to the
backend as-is'? Iʼm really struggling to understand where the
motivation is coming from to escape characters in the search string,
given that each backend can have completely different regexp support.

Robert

bug#36644: Git log search

[Juri Linkov]

> We can't really use this approach. VC is a high level abstraction,
> so we try to define the semantics well.

I tend to agree with Robert.  A string have to be passed to the backend as is.
It seems such situations when these strings should be compatible between
different backends (such as running the same command on one backend,
and then repeating the same search on another backend by retrieving
a previous argument from the history via M-p) are very rare.

bug#36644: Git log search

[Juri Linkov]

>>> Bug.*0
>>>
>>> to get all log entries with a bug reference ending in 0.
>>
>> Currently when using shell-quote-argument it doesn't allow
>> using such regexps as Bug.*0.  So I guess we need to remove
>> shell-quote-argument and also to not use regexp-quote.
>
> I think shell-quote-argument is a mistake regardless, vc-git-command
> doesn't call a shell, so there is no need for shell-quote-argument.

Right, shell-quote-argument should be removed in any case, fixed.

bug#36644: Git log search

[Dmitry Gutov]

On 17.07.2019 1:31, Noam Postavsky wrote:

> I think shell-quote-argument is a mistake regardless, vc-git-command
> doesn't call a shell, so there is no need for shell-quote-argument.

Right, that is true. Sorry, I got confused by code constructing the 
"full command" value (which intersperses the flags with spaces).

bug#36644: Git log search

[Dmitry Gutov]

On 19.07.2019 1:32, Juri Linkov wrote:
>> We can't really use this approach. VC is a high level abstraction,
>> so we try to define the semantics well.
> 
> I tend to agree with Robert.  A string have to be passed to the backend as is.
> It seems such situations when these strings should be compatible between
> different backends (such as running the same command on one backend,
> and then repeating the same search on another backend by retrieving
> a previous argument from the history via M-p) are very rare.

I might agree with you from the practical standpoint, but vc-log-search 
needs a docstring that actually describes what the function is going to 
do. Including info on how PATTERN is going to be interpreted.

E.g. whether "foo.txt" will only match literally, or whether "." can be 
substituted by any character.

And if PATTERN is a regexp, what kind of regexp it's going to be 
interepreted as: basic RE, extended RE, Emacs RE, or Perl RE (probably 
not the last one anyway).

I suppose we can choose one of these and say e.g. that pattern is 
interpreted as an extended regular expression, except for some backends 
that don't support that. I wonder how we're going to convey the latter 
to the user.

bug#36644: Git log search

[Robert Pluim]

>>>>> On Wed, 24 Jul 2019 18:10:35 +0300, Dmitry Gutov <dgutov@yandex.ru> said:

    Dmitry> On 19.07.2019 1:32, Juri Linkov wrote:
    >>> We can't really use this approach. VC is a high level abstraction,
    >>> so we try to define the semantics well.
    >> 
    >> I tend to agree with Robert.  A string have to be passed to the backend as is.
    >> It seems such situations when these strings should be compatible between
    >> different backends (such as running the same command on one backend,
    >> and then repeating the same search on another backend by retrieving
    >> a previous argument from the history via M-p) are very rare.

    Dmitry> I might agree with you from the practical standpoint, but
    Dmitry> vc-log-search needs a docstring that actually describes what the
    Dmitry> function is going to do. Including info on how PATTERN is going to be
    Dmitry> interpreted.

If it were implemented as 'backend show me all the logs and then emacs
will search through them' then that would be required, but itʼs not,
itʼs implemented as 'backend show me the logs which match STRING'

    Dmitry> E.g. whether "foo.txt" will only match literally, or whether "." can
    Dmitry> be substituted by any character.

That will depend on the backend

    Dmitry> And if PATTERN is a regexp, what kind of regexp it's going to be
    Dmitry> interepreted as: basic RE, extended RE, Emacs RE, or Perl RE (probably
    Dmitry> not the last one anyway).

As will this

    Dmitry> I suppose we can choose one of these and say e.g. that pattern is
    Dmitry> interpreted as an extended regular expression, except for some
    Dmitry> backends that don't support that. I wonder how we're going to convey
    Dmitry> the latter to the user.

Itʼs not a pattern. Itʼs a string that is passed as-is to the backend,
which is free to interpret it as it wishes. From my viewpoint, we can
just say

"Search for STRING, which is passed unsullied to the backend's log
search command.  Consult the documentation for your backend to
understand the matching method it uses to search for STRING."

or similar.

Robert

bug#36644: Git log search

[Dmitry Gutov]

On 24.07.2019 18:46, Robert Pluim wrote:
>>>>>> On Wed, 24 Jul 2019 18:10:35 +0300, Dmitry Gutov <dgutov@yandex.ru> said:
> 
>      Dmitry> On 19.07.2019 1:32, Juri Linkov wrote:
>      >>> We can't really use this approach. VC is a high level abstraction,
>      >>> so we try to define the semantics well.
>      >>
>      >> I tend to agree with Robert.  A string have to be passed to the backend as is.
>      >> It seems such situations when these strings should be compatible between
>      >> different backends (such as running the same command on one backend,
>      >> and then repeating the same search on another backend by retrieving
>      >> a previous argument from the history via M-p) are very rare.
> 
>      Dmitry> I might agree with you from the practical standpoint, but
>      Dmitry> vc-log-search needs a docstring that actually describes what the
>      Dmitry> function is going to do. Including info on how PATTERN is going to be
>      Dmitry> interpreted.
> 
> If it were implemented as 'backend show me all the logs and then emacs
> will search through them' then that would be required, but itʼs not,
> itʼs implemented as 'backend show me the logs which match STRING'

Again, it's about documentation and about commands doing things in a way 
that the user can anticipate.

>      Dmitry> E.g. whether "foo.txt" will only match literally, or whether "." can
>      Dmitry> be substituted by any character.
> 
> That will depend on the backend
> 
>      Dmitry> And if PATTERN is a regexp, what kind of regexp it's going to be
>      Dmitry> interepreted as: basic RE, extended RE, Emacs RE, or Perl RE (probably
>      Dmitry> not the last one anyway).
> 
> As will this
> 
>      Dmitry> I suppose we can choose one of these and say e.g. that pattern is
>      Dmitry> interpreted as an extended regular expression, except for some
>      Dmitry> backends that don't support that. I wonder how we're going to convey
>      Dmitry> the latter to the user.
> 
> Itʼs not a pattern. Itʼs a string that is passed as-is to the backend,
> which is free to interpret it as it wishes. From my viewpoint, we can
> just say
> 
> "Search for STRING, which is passed unsullied to the backend's log
> search command.  Consult the documentation for your backend to
> understand the matching method it uses to search for STRING."

IME this doesn't match the way we try to document commands in Emacs, but 
I wouldn't want to spend much time arguing about this.

Eli, could you weigh in in this discussion? Would you say Robert's 
proposal is acceptable?

bug#36644: Git log search

[Eli Zaretskii]

> Cc: "36644@debbugs.gnu.org" <36644@debbugs.gnu.org>,
>  Juri Linkov <juri@linkov.net>, Eli Zaretskii <eliz@gnu.org>
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Jul 2019 18:53:01 +0300
> 
> > Itʼs not a pattern. Itʼs a string that is passed as-is to the backend,
> > which is free to interpret it as it wishes. From my viewpoint, we can
> > just say
> > 
> > "Search for STRING, which is passed unsullied to the backend's log
> > search command.  Consult the documentation for your backend to
> > understand the matching method it uses to search for STRING."
> 
> IME this doesn't match the way we try to document commands in Emacs, but 
> I wouldn't want to spend much time arguing about this.
> 
> Eli, could you weigh in in this discussion? Would you say Robert's 
> proposal is acceptable?

Yes, I think on balance it's acceptable.

I also see your point: it would be nice to be able to document the
semantics of PATTERN in a backend-independent way.  But I think this
is next to impossible in this case, both because of significant
differences in the backend capabilities (e.g., bzr doesn't have the
equivalent of Git's --fixed-strings, AFAICT), and because some backend
allow great flexibility in interpreting PATTERN, under control of
optional switches passed to the backend.  the only way to make this
backend-independent is to do the search in Emacs Lisp, which I think
will slow down the command too much.

So I think we should treat this as we do in "M-x grep": leave the
semantics of PATTERN backend-dependent, and rely on the user to quote
some characters in it as needed.  Admittedly, 'grep' is lower-level
than 'vc-log-search', but at least we have a precedent.

Note that I still think we should use PATTERN, not STRING in the doc
string, because a literal string here is more an exception than a
rule.  But we should say that the exact semantics of PATTERN is
backend-dependent, and perhaps describe how a couple of the more
popular backends interpret it.

bug#36644: Git log search

[Robert Pluim]

>>>>> On Wed, 24 Jul 2019 18:53:01 +0300, Dmitry Gutov <dgutov@yandex.ru> said:
    >> Itʼs not a pattern. Itʼs a string that is passed as-is to the backend,
    >> which is free to interpret it as it wishes. From my viewpoint, we can
    >> just say
    >> 
    >> "Search for STRING, which is passed unsullied to the backend's log
    >> search command.  Consult the documentation for your backend to
    >> understand the matching method it uses to search for STRING."

    Dmitry> IME this doesn't match the way we try to document commands in Emacs,
    Dmitry> but I wouldn't want to spend much time arguing about this.

    Dmitry> Eli, could you weigh in in this discussion? Would you say Robert's
    Dmitry> proposal is acceptable?

Going back to Juri's original proposal:

+;;;###autoload
+(defun vc-log-search (pattern)
+  "Search a log of changes for PATTERN."
+  (interactive (list (read-regexp "Log search pattern: ")))
+  (let ((backend (vc-deduce-backend)))
+    (unless backend
+      (error "Buffer is not version controlled"))
+    (vc-incoming-outgoing-internal backend pattern
+                                   "*vc-search*" 'log-search)))
+


How about:

"Search a log of changes for PATTERN.

How PATTERN is interpreted will depend on how each individual
backend's log search command is implemented, as some can only match
fixed strings, some have regular expression support, etc."

Robert

bug#36644: Git log search

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  "36644\@debbugs.gnu.org"
>  <36644@debbugs.gnu.org>,  Juri Linkov <juri@linkov.net>
> Date: Wed, 24 Jul 2019 18:13:53 +0200
> 
> "Search a log of changes for PATTERN.
> 
> How PATTERN is interpreted will depend on how each individual
> backend's log search command is implemented, as some can only match
> fixed strings, some have regular expression support, etc."

I think this should also mention that most backends will interpret
PATTERN as a regular expression.  Something like this:

    "Search the log of changes for PATTERN.

  PATTERN is usually interpreted as a regular expression.  However, its
  exact semantics is up to the backend's log search command; some can
  only match fixed strings."

bug#36644: Git log search

[Juri Linkov]

>> "Search a log of changes for PATTERN.
>>
>> How PATTERN is interpreted will depend on how each individual
>> backend's log search command is implemented, as some can only match
>> fixed strings, some have regular expression support, etc."
>
> I think this should also mention that most backends will interpret
> PATTERN as a regular expression.  Something like this:
>
>     "Search the log of changes for PATTERN.
>
>   PATTERN is usually interpreted as a regular expression.  However, its
>   exact semantics is up to the backend's log search command; some can
>   only match fixed strings."

So I fixed this accordingly.

bug#36644: Git log search

[Dmitry Gutov]

On 24.07.2019 19:13, Eli Zaretskii wrote:

>> Eli, could you weigh in in this discussion? Would you say Robert's
>> proposal is acceptable?
> 
> Yes, I think on balance it's acceptable.

OK, thank you.

> I also see your point: it would be nice to be able to document the
> semantics of PATTERN in a backend-independent way.  But I think this
> is next to impossible in this case, both because of significant
> differences in the backend capabilities (e.g., bzr doesn't have the
> equivalent of Git's --fixed-strings, AFAICT), and because some backend
> allow great flexibility in interpreting PATTERN, under control of
> optional switches passed to the backend.

The other option is to standardize on basic or extended regexp, and 
simply give up for backends that can't support that.

Git supports all kinds of regexps. 'hg grep' uses Perl-compatible ones 
(meaning extended regexps are supported, at least). I'm not sure which 
regular expressions are expected by 'bzr log -match', but if it doesn't 
support the extended ones, *shrug*.

As for the older VCS-es, some probably don't support search at all. And 
we've lived without such support for decades, so supporting them can't 
be too important.

Anyway, if people disagree, I'm not going to press the issue.

> So I think we should treat this as we do in "M-x grep": leave the
> semantics of PATTERN backend-dependent, and rely on the user to quote
> some characters in it as needed.  Admittedly, 'grep' is lower-level
> than 'vc-log-search', but at least we have a precedent.

The difference is, 'M-x grep' doesn't use different backends. Although 
I'd be happy to see that capability.

> Note that I still think we should use PATTERN, not STRING in the doc
> string, because a literal string here is more an exception than a
> rule.  But we should say that the exact semantics of PATTERN is
> backend-dependent, and perhaps describe how a couple of the more
> popular backends interpret it.

Makes sense.

bug#36644: Git log search

[Dmitry Gutov]

On 25.07.2019 2:22, Juri Linkov wrote:

> So I fixed this accordingly.

Thank you.

Two points:

- Are you sure we want to use the basic regexps here? Thinking back to 
my experience when I just started using Emacs, I really was only aware 
of two kinds: extended regexps (because most programming languages use 
this syntax nowadays) and Emacs regexps. So I figure extended regexps 
might be the more user-friendly option.

- I wonder how tabs got inside the new function's definition, 
considering the value of indent-tabs-mode we have in .dir-locals.

bug#36644: Git log search

[Eli Zaretskii]

> Cc: rpluim@gmail.com, 36644@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 25 Jul 2019 15:36:54 +0300
> 
> > I also see your point: it would be nice to be able to document the
> > semantics of PATTERN in a backend-independent way.  But I think this
> > is next to impossible in this case, both because of significant
> > differences in the backend capabilities (e.g., bzr doesn't have the
> > equivalent of Git's --fixed-strings, AFAICT), and because some backend
> > allow great flexibility in interpreting PATTERN, under control of
> > optional switches passed to the backend.
> 
> The other option is to standardize on basic or extended regexp, and 
> simply give up for backends that can't support that.

We could simply say "regular expression" and leave the details
unspecified.  But I think Juri said that fixed strings was the lowest
common denominator, which is why I proposed a slightly more vague doc
string.  Juri, which backends don't support regular expressions?  And
Robert, why did you insist on saying STRING?

> Git supports all kinds of regexps. 'hg grep' uses Perl-compatible ones 
> (meaning extended regexps are supported, at least). I'm not sure which 
> regular expressions are expected by 'bzr log -match', but if it doesn't 
> support the extended ones, *shrug*.

I didn't dig deep enough, but since bzr is written in Python, I'd bet
it supports whatever Python supports natively.

> Anyway, if people disagree, I'm not going to press the issue.

If all the backends either support regular expressions or don't
support this feature at all, then we had better mentioned regular
expressions in the doc string.

> > So I think we should treat this as we do in "M-x grep": leave the
> > semantics of PATTERN backend-dependent, and rely on the user to quote
> > some characters in it as needed.  Admittedly, 'grep' is lower-level
> > than 'vc-log-search', but at least we have a precedent.
> 
> The difference is, 'M-x grep' doesn't use different backends.

It actually leaves that to the user: you can invoke any program you
want.  E.g., I invoke 'fgrep' this way very frequently.

bug#36644: Git log search

[Dmitry Gutov]

On 25.07.2019 16:08, Eli Zaretskii wrote:
> It actually leaves that to the user: you can invoke any program you
> want.  E.g., I invoke 'fgrep' this way very frequently.

That's true, but I was thinking more along the lines of using e.g. Ag 
for 'M-x grep' if the user customizes some variable in a particular way.

The "invoke any program" workflow is good and all, but it's much easier 
to describe in the docstring, so it's not the issue.

bug#36644: Git log search

[Robert Pluim]

>>>>> On Thu, 25 Jul 2019 16:08:37 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> Cc: rpluim@gmail.com, 36644@debbugs.gnu.org, juri@linkov.net
    >> From: Dmitry Gutov <dgutov@yandex.ru>
    >> Date: Thu, 25 Jul 2019 15:36:54 +0300
    >> 
    >> > I also see your point: it would be nice to be able to document the
    >> > semantics of PATTERN in a backend-independent way.  But I think this
    >> > is next to impossible in this case, both because of significant
    >> > differences in the backend capabilities (e.g., bzr doesn't have the
    >> > equivalent of Git's --fixed-strings, AFAICT), and because some backend
    >> > allow great flexibility in interpreting PATTERN, under control of
    >> > optional switches passed to the backend.
    >> 
    >> The other option is to standardize on basic or extended regexp, and 
    >> simply give up for backends that can't support that.

    Eli> We could simply say "regular expression" and leave the details
    Eli> unspecified.  But I think Juri said that fixed strings was the lowest
    Eli> common denominator, which is why I proposed a slightly more vague doc
    Eli> string.  Juri, which backends don't support regular expressions?  And
    Eli> Robert, why did you insist on saying STRING?

I donʼt think I did, we can say PATTERN. The thing Iʼm insistent about
is not doing anything to that argument, since its meaning will be
understood only by the backend.

    >> Git supports all kinds of regexps. 'hg grep' uses Perl-compatible ones 
    >> (meaning extended regexps are supported, at least). I'm not sure which 
    >> regular expressions are expected by 'bzr log -match', but if it doesn't 
    >> support the extended ones, *shrug*.

    Eli> I didn't dig deep enough, but since bzr is written in Python, I'd bet
    Eli> it supports whatever Python supports natively.

    >> Anyway, if people disagree, I'm not going to press the issue.

    Eli> If all the backends either support regular expressions or don't
    Eli> support this feature at all, then we had better mentioned regular
    Eli> expressions in the doc string.

Yes, although the regular expression types they support may be
different.

Robert

bug#36644: Git log search

[Juri Linkov]

> - Are you sure we want to use the basic regexps here? Thinking back to my
> experience when I just started using Emacs, I really was only aware of two
> kinds: extended regexps (because most programming languages use this syntax
> nowadays) and Emacs regexps. So I figure extended regexps might be the more
> user-friendly option.

The default version of regular expression syntax is
a basic regular expression in both git-log and grep.
And as Eli has pointed out to grep as a precedent,
most users are accustomed to basic regexps by default
while grepping files or git logs.

> - I wonder how tabs got inside the new function's definition,
> considering the value of indent-tabs-mode we have in .dir-locals.

Sorry, I had a mode with a rule to use indent-tabs-mode
when the file already uses TABs:

  (defun indent-tabs-mode-maybe ()
    (if (save-excursion
          (goto-char (point-min))
          ;; If there are at least 10 lines with a leading TAB, use TABs.
          (re-search-forward "^\t" (+ (point) 100000) t 10))
        (set (make-local-variable 'indent-tabs-mode) t)))
  (add-hook 'find-file-hook 'indent-tabs-mode-maybe)

I'll disable it.

bug#36644: Git log search

[Juri Linkov]

>> So I think we should treat this as we do in "M-x grep": leave the
>> semantics of PATTERN backend-dependent, and rely on the user to quote
>> some characters in it as needed.  Admittedly, 'grep' is lower-level
>> than 'vc-log-search', but at least we have a precedent.
>
> The difference is, 'M-x grep' doesn't use different backends. Although I'd
> be happy to see that capability.

grep backends can be switched implicitly by installing OS packages
like 'agrep', 'pcregrep', 'ripgrep', etc.

bug#36644: Git log search

[Juri Linkov]

>> > I also see your point: it would be nice to be able to document the
>> > semantics of PATTERN in a backend-independent way.  But I think this
>> > is next to impossible in this case, both because of significant
>> > differences in the backend capabilities (e.g., bzr doesn't have the
>> > equivalent of Git's --fixed-strings, AFAICT), and because some backend
>> > allow great flexibility in interpreting PATTERN, under control of
>> > optional switches passed to the backend.
>>
>> The other option is to standardize on basic or extended regexp, and
>> simply give up for backends that can't support that.
>
> We could simply say "regular expression" and leave the details
> unspecified.  But I think Juri said that fixed strings was the lowest
> common denominator, which is why I proposed a slightly more vague doc
> string.  Juri, which backends don't support regular expressions?

The documentation of 'git-log' uses the term 'pattern' for a good reason
since it covers all possible values: basic-regexp, extended-regexp,
perl-regexp, fixed-strings.

bug#36644: Git log search

[Dmitry Gutov]

On 25.07.2019 21:50, Juri Linkov wrote:
> The default version of regular expression syntax is
> a basic regular expression in both git-log and grep.
> And as Eli has pointed out to grep as a precedent,
> most users are accustomed to basic regexps by default
> while grepping files or git logs.

There's also the thing that other backends might not support basic 
regexps. Anyway, up to you.

> I'll disable it.

Thank you.

bug#36644: Git log search

[Dmitry Gutov]

On 25.07.2019 21:55, Juri Linkov wrote:

> grep backends can be switched implicitly by installing OS packages
> like 'agrep', 'pcregrep', 'ripgrep', etc.

Do you mean to say that such packages make 'grep' a symbolic link, or 
similar? Because it doesn't look like 'ripgrep' does that.

bug#36644: Git log search

[Juri Linkov]

>> grep backends can be switched implicitly by installing OS packages
>> like 'agrep', 'pcregrep', 'ripgrep', etc.
>
> Do you mean to say that such packages make 'grep' a symbolic link, or
> similar? Because it doesn't look like 'ripgrep' does that.

Yep, something like this, either by package managers, or by users themselves
creating an alias, etc.

bug#36644: Git log search

[Juri Linkov]

tags 36644 + fixed
close 36644 27.0.50
quit

> So I fixed this accordingly.

I'm closing this now.

Regarding a key binding, I'm not sure how often this command will be used.
Only depending on this measurement, we could decide later whether this command
needs a shorter key sequence or not.