GNU bug report logs - #28615
25.3; rgrep, grep-read-files confusingly completes file names

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28615 in the body.
You can then email your comments to 28615 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: ヌエルモリノ <vianchielfaura <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.3; rgrep, grep-read-files confusingly completes file names
Date: Wed, 27 Sep 2017 00:01:56 -0700

rgrep uses grep-read-files to read a filename as the second argument.
However, rgrep cannot actually accept a filename as the second argument.

For example, this is a normal rgrep call

  (rgrep "foo" "*.el" "/some/dir/" nil)

"*.el" is read with grep-read-files, which uses filename completion.
However, actually entering a filename doesn’t really work

  (rgrep "foo" "foo/*.el" "/some/dir/" nil)
  (rgrep "foo" "foo/bar.el" "/some/dir/" nil)

The second argument is passed to the find program as an -iname argument
as a glob, which matches against strictly the filename (e.g., like
file-name-nondirectory) and not the path.

There are a few points that could be improved here.

1. The grep-read-files prompt is confusing.  The prompt is: "Search for
\"foo\" in files: ".  It is not clear that this is asking for a filename
glob.

2. grep-read-files uses filename completion, which suggests that the
user should enter a filename.  This is very misleading as most paths
will fail to work, passing a filename without a slash will likely not
work as the user would expect, and almost certainly a glob would be the
desired input.

In GNU Emacs 25.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.19)
 of 2017-09-16 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.11903000
Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Message #8 received at 28615 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: 28615 <at> debbugs.gnu.org
Subject: [PATCH] Clarify what grep-read-files wants
Date: Wed, 4 Oct 2017 21:15:32 -0700

grep-read-files wants a filename glob, not a filename/path.

* lisp/progmodes/grep.el (grep-read-files): Clarify docstring.
  Use file aliases for completion.
  Clarify completing-read prompt.
---
 lisp/progmodes/grep.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 01bdb04714..0537c76d30 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -903,7 +903,7 @@ grep-read-regexp
   (read-regexp "Search for" 'grep-tag-default 'grep-regexp-history))

 (defun grep-read-files (regexp)
-  "Read files arg for interactive grep."
+  "Read filename glob arg for interactive grep."
   (let* ((bn (or (buffer-file-name)
  (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name))))
  (fn (and bn
@@ -936,10 +936,10 @@ grep-read-files
         (car (car grep-files-aliases))))
  (files (completing-read
  (concat "Search for \"" regexp
- "\" in files"
+ "\" in files matching glob"
  (if default (concat " (default " default ")"))
  ": ")
- 'read-file-name-internal
+ grep-files-aliases
  nil nil nil 'grep-files-history
  (delete-dups
    (delq nil (append (list default default-alias default-extension)
-- 
2.14.2

Message #13 received at 28615-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: 28615-done <at> debbugs.gnu.org
Subject: Re: bug#28615: [PATCH] Clarify what grep-read-files wants
Date: Mon, 09 Oct 2017 16:41:23 +0300

> From: Allen Li <vianchielfaura <at> gmail.com>
> Date: Wed, 4 Oct 2017 21:15:32 -0700
> 
> grep-read-files wants a filename glob, not a filename/path.
> 
> * lisp/progmodes/grep.el (grep-read-files): Clarify docstring.
>   Use file aliases for completion.
>   Clarify completing-read prompt.

Thanks, I installed a slightly different change for the first 2 parts,
but didn't change the function we pass to completing-read.

With that, I'm closing the bug report.

Message #16 received at 28615-done <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 28615-done <at> debbugs.gnu.org
Subject: Re: bug#28615: [PATCH] Clarify what grep-read-files wants
Date: Mon, 9 Oct 2017 21:43:59 -0700

On Mon, Oct 9, 2017 at 6:41 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> Thanks, I installed a slightly different change for the first 2 parts,
> but didn't change the function we pass to completing-read.
>
> With that, I'm closing the bug report.

Why did you not change the collection function?  As I noted in the
bug, most of the completions that would be provided by
read-file-name-internal don't work, and the ones that do work result
in running rgrep against a single file.  grep-files-aliases seems like
a much better choice than read-file-name-internal, given that
grep-files-aliases is intended to provide aliases for this very
purpose.

Message #19 received at 28615 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: 28615 <at> debbugs.gnu.org
Subject: Re: bug#28615: [PATCH] Clarify what grep-read-files wants
Date: Tue, 10 Oct 2017 08:55:26 +0300

> From: Allen Li <vianchielfaura <at> gmail.com>
> Date: Mon, 9 Oct 2017 21:43:59 -0700
> Cc: 28615-done <at> debbugs.gnu.org
> 
> Why did you not change the collection function?

Because there could be a file called literally like one of the
shorthands in grep-files-aliases, or whose name includes those
shorthands as a substring.

> As I noted in the bug, most of the completions that would be
> provided by read-file-name-internal don't work

IMO, that's okay, because wildcards cannot be meaningfully completed
on.

Message #22 received at 28615 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 28615 <at> debbugs.gnu.org
Subject: Re: bug#28615: [PATCH] Clarify what grep-read-files wants
Date: Mon, 9 Oct 2017 23:51:47 -0700

On Mon, Oct 9, 2017 at 10:55 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Allen Li <vianchielfaura <at> gmail.com>
>> Date: Mon, 9 Oct 2017 21:43:59 -0700
>> Cc: 28615-done <at> debbugs.gnu.org
>>
>> Why did you not change the collection function?
>
> Because there could be a file called literally like one of the
> shorthands in grep-files-aliases, or whose name includes those
> shorthands as a substring.

Not sure what you mean.  There is no use case for completing a
file name.  Are you saying that a user might mistake the completion
on grep-files-aliases as completion on file name?  I believe that
with the new prompt change, that is unlikely.

>
>> As I noted in the bug, most of the completions that would be
>> provided by read-file-name-internal don't work
>
> IMO, that's okay, because wildcards cannot be meaningfully completed
> on.

But the aliases from grep-files-aliases can be meaningfully
completed.  I don't see why we shouldn't provide meaningful
completion if the user defines a lot of aliases, seeing as how
the current file name completion is useless.

Message #25 received at 28615 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: 28615 <at> debbugs.gnu.org
Subject: Re: bug#28615: [PATCH] Clarify what grep-read-files wants
Date: Tue, 10 Oct 2017 10:52:01 +0300

> From: Allen Li <vianchielfaura <at> gmail.com>
> Date: Mon, 9 Oct 2017 23:51:47 -0700
> Cc: 28615 <at> debbugs.gnu.org
> 
> Are you saying that a user might mistake the completion on
> grep-files-aliases as completion on file name?

Something like that, yes.  In general, I don't think completing on
wildcards is useful.

> I believe that with the new prompt change, that is unlikely.

People don't always read the prompt paying attention to every word of
it, and "all" is a valid wildcard anyway.

> >> As I noted in the bug, most of the completions that would be
> >> provided by read-file-name-internal don't work
> >
> > IMO, that's okay, because wildcards cannot be meaningfully completed
> > on.
> 
> But the aliases from grep-files-aliases can be meaningfully
> completed.  I don't see why we shouldn't provide meaningful
> completion if the user defines a lot of aliases, seeing as how
> the current file name completion is useless.

I didn't think allowing it to complete on 2 non-file values is
important enough to justify the possible confusion.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.