bug#34621: [PATCH] lisp/progmodes/grep.el (grep-read-files): Add file-directory-p check

bug#34621: [PATCH] lisp/progmodes/grep.el (grep-read-files): Add file-directory-p check

[Christopher Thorne]

Hello,

This patch fixes a bug in the rgrep command which causes certain
directory names to be mistaken for files with extensions. For example,
when running rgrep in a directory called "django-1.11", rgrep will
prompt with 'Search for "x" in files (default *.11):', under the
assumption that .11 is a file extension. Similarly, creating a
directory called "test.c" and running rgrep inside it results in the
prompt 'Search for "x" in files (default *.[ch])'. With this patch,
the default file extension for directories is either taken from the
rgrep history or set to "all".

Changelog entry:
* lisp/progmodes/grep.el (grep-read-files): Add file-directory-p check

Patch:

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 3fd2a7e701..fe0fb5b30c 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -963,6 +963,7 @@ grep-read-files
                   (file-name-nondirectory bn)))
          (default-alias
            (and fn
+               (not (file-directory-p (concat "../" fn)))
                 (let ((aliases (remove (assoc "all" grep-files-aliases)
                                        grep-files-aliases))
                       alias)
@@ -979,6 +980,7 @@ grep-read-files
                   (cdr alias))))
          (default-extension
            (and fn
+               (not (file-directory-p (concat "../" fn)))
                 (let ((ext (file-name-extension fn)))
                   (and ext (concat "*." ext)))))
          (default

Regards,
Christopher Thorne

bug#34621: Patch Update

[Christopher Thorne]

I've changed the patch to check if the buffer is in dired mode. I think 
this makes more sense than looking at the file system and addresses the 
edge case described in the previous email.

Also, I think I could have been clearer in explaining how to reproduce:
1. Create directory called django-1.11 on the filesystem
2. Open this directory in dired buffer
3. Run rgrep with this dired buffer open
4. Search for any string
5. Observe unexpected default extension suggestion of (*.11)

Patch below:

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 3fd2a7e701..22e7ec11d8 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -963,6 +963,7 @@ grep-read-files
                   (file-name-nondirectory bn)))
          (default-alias
            (and fn
+               (not (eq major-mode 'dired-mode))
                 (let ((aliases (remove (assoc "all" grep-files-aliases)
                                        grep-files-aliases))
                       alias)
@@ -979,6 +980,7 @@ grep-read-files
                   (cdr alias))))
          (default-extension
            (and fn
+               (not (eq major-mode 'dired-mode))
                 (let ((ext (file-name-extension fn)))
                   (and ext (concat "*." ext)))))
          (default

Changelog entry:
* lisp/progmodes/grep.el (grep-read-files): Disable default extension in 
dired buffers

bug#34621: Patch Update

[Drew Adams]

> +               (not (eq major-mode 'dired-mode))
> +               (not (eq major-mode 'dired-mode))

I haven't followed this thread at all - dunno
what the problem is that you're trying to solve.
But I'm a bit surprised that Dired needs to be
special-cased for grep in any way.

Anyway, the reasom I'm writing is to suggest
that you probably don't want
(eq major-mode 'dired-mode).  You probably
want (derived-mode-p 'dired-mode).

bug#34621: Patch Update

[Christopher Thorne]

Thanks Drew, I agree derived-mode-p is better.

The reason for special casing dired is to stop rgrep taking extension 
suggestions from dired buffers, e.g. suggesting "*.11" as the rgrep file 
pattern in a dired buffer called "django-1.11".

Here is the same fix but using derived-mode-p:

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 3fd2a7e701..83154872aa 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -963,6 +963,7 @@ grep-read-files
                   (file-name-nondirectory bn)))
          (default-alias
            (and fn
+               (not (derived-mode-p 'dired-mode))
                 (let ((aliases (remove (assoc "all" grep-files-aliases)
                                        grep-files-aliases))
                       alias)
@@ -979,6 +980,7 @@ grep-read-files
                   (cdr alias))))
          (default-extension
            (and fn
+               (not (derived-mode-p 'dired-mode))
                 (let ((ext (file-name-extension fn)))
                   (and ext (concat "*." ext)))))
          (default

Changelog entry:
* lisp/progmodes/grep.el (grep-read-files): Disable default extension in
dired buffers


On 2019-03-04 15:26, Drew Adams wrote:
>> +               (not (eq major-mode 'dired-mode))
>> +               (not (eq major-mode 'dired-mode))
> 
> I haven't followed this thread at all - dunno
> what the problem is that you're trying to solve.
> But I'm a bit surprised that Dired needs to be
> special-cased for grep in any way.
> 
> Anyway, the reasom I'm writing is to suggest
> that you probably don't want
> (eq major-mode 'dired-mode).  You probably
> want (derived-mode-p 'dired-mode).

bug#34621: Patch Update

[Drew Adams]

> The reason for special casing dired is to stop rgrep taking extension
> suggestions from dired buffers, e.g. suggesting "*.11" as the rgrep
> file pattern in a dired buffer called "django-1.11".

I see.  It sounds like `rgrep' should itself be changed
to be more flexible or smarter, or to let users more
easily control the default file-name pattern.

It doesn't sound to me like this has anything, per se,
to do with Dired.  It has to do with how the default is
determined, and that's apparently now being picked up
from the buffer name.

bug#34621: Patch Update

[Christopher Thorne]

> It doesn't sound to me like this has anything, per se,
> to do with Dired.  It has to do with how the default is
> determined, and that's apparently now being picked up
> from the buffer name.

Hmm, I think you're right that this isn't just isolated to dired. For 
example, I can start a shell-mode buffer, rename it to shell.txt and 
rgrep will now suggest "*.txt" as the default extension even though my 
buffer is unrelated to .txt files.

An alternative I considered is only showing extension suggestions for 
buffers that are associated with a file (i.e. buffer-file-name is a 
non-empty string). Can you think of any cases where this would fall 
down?

bug#34621: Patch Update

[Drew Adams]

> > It doesn't sound to me like this has anything, per se,
> > to do with Dired.  It has to do with how the default is
> > determined, and that's apparently now being picked up
> > from the buffer name.
> 
> Hmm, I think you're right that this isn't just isolated to dired. For
> example, I can start a shell-mode buffer, rename it to shell.txt and
> rgrep will now suggest "*.txt" as the default extension even though my
> buffer is unrelated to .txt files.
> 
> An alternative I considered is only showing extension suggestions for
> buffers that are associated with a file (i.e. buffer-file-name is a
> non-empty string). Can you think of any cases where this would fall
> down?

I don't think it's a question of falling down.

It's not obvious what a reasonable or smart default
filename pattern is in most cases.  Just because
your current buffer is visiting a file does not at
all imply that you want to search files with the
same extension.

I think you need to (for yourself) specify just
what relation (if any) you want between the current
buffer and the default filename pattern.

If it's a question of improving `rgrep` then the
determination of the default needs to be such that
it's useful generally and typically, and perhaps
user configurable.

As an example, but for the search pattern only
(not the filename pattern(s)), in `grep+.el' there
is user option `grepp-default-regexp-fn` and a
default function of the same name, as follows.
(If the option value is not a function then the
default function is used.)

https://www.emacswiki.org/emacs/grep%2b.el

I mention this only as an example of how you can
provide some flexibility and user control.  For
filename pattern(s) obviously the ways of picking
a default would be different.

------------8<--------------

grepp-default-regexp-fn is a variable defined in `grep+.el'.
Its value is non-nil-symbol-name-nearest-point

Documentation:
Function of 0 args called to provide default search regexp to \\[grep].
Some reasonable choices are defined in `thingatpt+.el':
`word-nearest-point', `non-nil-symbol-name-nearest-point',
`region-or-non-nil-symbol-name-nearest-point', `sexp-nearest-point'.

This is ignored if Transient Mark mode is on and the region is active
and non-empty.  In that case, the quoted (") region text is used as
the default regexp.

If `grepp-default-regexp-fn' is nil and no prefix arg is given to
`grep', then no defaulting is done.

Otherwise, if the value is not a function, then function
`grepp-default-regexp-fn' does the defaulting.

You can customize this variable.

------------8<--------------

(defun grepp-default-regexp-fn ()
  "*Function of 0 args called to provide default search regexp to \\[grep].
This is used only if both of the following are true:
- Transient Mark mode is off or the region is inactive or empty.
- The value of option `grepp-default-regexp-fn' is
  `grepp-default-regexp-fn'.

When this is used, the default regexp is provided by calling the
first of these that references a defined function:
  - variable `grepp-default-regexp-fn'
  - variable `find-tag-default-function'
  - the `find-tag-default-function' property of the `major-mode'
  - function `non-nil-symbol-name-nearest-point', if bound
  - function `grep-tag-default'"
  (cond ((functionp grepp-default-regexp-fn) grepp-default-regexp-fn)
        (find-tag-default-function)
        ((get major-mode 'find-tag-default-function))
        ((fboundp 'non-nil-symbol-name-nearest-point) 'non-nil-symbol-name-nearest-point)
        (t 'find-tag-default)))

bug#34621: Patch Update

[Christopher Thorne]

Thanks for the input, Drew.

> It's not obvious what a reasonable or smart default
> filename pattern is in most cases.  Just because
> your current buffer is visiting a file does not at
> all imply that you want to search files with the
> same extension.
> 
> I think you need to (for yourself) specify just
> what relation (if any) you want between the current
> buffer and the default filename pattern.

Personally I think the current behaviour of searching files with the 
same extension works quite well in most cases. There is also some 
flexibility with aliases which means that if you're in a "*.c" file the 
default pattern becomes "*.[ch]".

However, I like the idea of providing more flexibility like 
`grepp-default-regexp-fn`. This would mean a buffer in c-mode could 
always suggest *.[ch] regardless of its buffer/file name. More 
importantly, dired could implement this so that no default patterns are 
suggested for dired buffers.

I will consider both (1) using only buffer-file-name instead of 
buffer-name (2) enabling rgrep to delegate extension suggestions to 
major modes. (1) would fix the case I'm personally encountering with 
dired, but (2) may be better in the long term and could cover more cases 
that I'm currently imagining.

bug#34621: [PATCH] lisp/progmodes/grep.el (grep-read-files): Add file-directory-p check

[Juri Linkov]

> More importantly, dired could implement this so that no default
> patterns are suggested for dired buffers.

For Dired, it would make more sense to use a default based on the
filename under point by using dired-get-filename, so for example
when in the Dired buffer in a directory called "django-1.11"
the cursor is on a file called "test.c" then the default could be
*.[ch]

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Christopher Thorne]

I've updated the patch following input from Drew and Juri.
This now adds 'grep-default-file-pattern-function' which major modes can 
implement.
Dired implements this to take the extension of the file at point, with a 
default of 'all',
thus solving the issue I was having with the django-1.11 directory 
mentioned earlier in the thread.
Behaviour for major modes that don't implement 
'grep-default-file-pattern-function' will remain as before.

diff --git a/lisp/dired.el b/lisp/dired.el
index 3cb645eea7..219acbf148 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -4138,6 +4138,19 @@ dired-restore-desktop-buffer
  (add-to-list 'desktop-buffer-mode-handlers
              '(dired-mode . dired-restore-desktop-buffer))

+(defun dired-grep-default-file-pattern ()
+  "Use extension of file at point as the default file pattern for grep.
+If a directory or nothing is found at point, fallback to 'all'."
+  (let* ((dired-file-name (ignore-errors (dired-get-filename)))
+        (dired-file-extension (if (and dired-file-name
+                                       (not (file-directory-p 
dired-file-name)))
+                                  (file-name-extension 
dired-file-name))))
+    (if dired-file-extension
+       (concat "*." dired-file-extension)
+       "all")))
+(put 'dired-mode 'grep-default-file-pattern-function 
'dired-grep-default-file-pattern)
+
+
  (provide 'dired)

  (run-hooks 'dired-load-hook)           ; for your customizations
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 3fd2a7e701..54d1412a66 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -961,6 +961,8 @@ grep-read-files
          (fn (and bn
                   (stringp bn)
                   (file-name-nondirectory bn)))
+        (default-file-pattern-function
+          (get major-mode 'grep-default-file-pattern-function))
          (default-alias
            (and fn
                 (let ((aliases (remove (assoc "all" grep-files-aliases)
@@ -982,10 +984,12 @@ grep-read-files
                 (let ((ext (file-name-extension fn)))
                   (and ext (concat "*." ext)))))
          (default
-          (or default-alias
-              default-extension
-              (car grep-files-history)
-              (car (car grep-files-aliases))))
+          (if default-file-pattern-function
+            (funcall default-file-pattern-function)
+            (or default-alias
+                default-extension
+                (car grep-files-history)
+                (car (car grep-files-aliases)))))
          (files (completing-read
                  (concat "Search for \"" regexp
                          "\" in files matching wildcard"

Changelog entry:
* lisp/progmodes/grep.el (grep-read-files): Allow major modes to
define default search file pattern
* lisp/dired.el (dired-grep-default-file-pattern): Define default search
file pattern for grep

Thanks for the input so far. Any further suggestions are welcome.

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Juri Linkov]

> I've updated the patch following input from Drew and Juri.
> This now adds 'grep-default-file-pattern-function' which major modes
> can implement.
> Dired implements this to take the extension of the file at point, with
> a default of 'all',
> thus solving the issue I was having with the django-1.11 directory
> mentioned earlier in the thread.
> Behaviour for major modes that don't implement
> 'grep-default-file-pattern-function' will remain as before.

Thanks, this is better than previous versions.

Even more, you don't need any changes in dired.el.  Just use in grep.el

  (run-hook-with-args-until-success 'file-name-at-point-functions)

because 'file-name-at-point-functions' already has the right value in Dired.

You can call it at the very beginning of grep-read-files in the same 'or'
before buffer-file-name, so the existing code will take care about getting
its file extension.

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Christopher Thorne]

> Even more, you don't need any changes in dired.el.  Just use in grep.el
> 
>   (run-hook-with-args-until-success 'file-name-at-point-functions)
> 
> because 'file-name-at-point-functions' already has the right value in 
> Dired.
> 
> You can call it at the very beginning of grep-read-files in the same 
> 'or'
> before buffer-file-name, so the existing code will take care about 
> getting
> its file extension.

Thanks, Juri. I tried this:

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 3fd2a7e701..4fb4490104 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -956,11 +956,17 @@ grep-read-files
  The pattern can include shell wildcards.  As whitespace triggers
  completion when entering a pattern, including it requires
  quoting, e.g. `\\[quoted-insert]<space>'."
-  (let* ((bn (or (buffer-file-name)
+  (let* ((file-name-at-point (run-hook-with-args-until-success 
'file-name-at-point-functions))
+        (bn (or (if (and (stringp file-name-at-point)
+                         (not (file-directory-p file-name-at-point)))
+                    file-name-at-point)
+                (buffer-file-name)
                  (replace-regexp-in-string "<[0-9]+>\\'" "" 
(buffer-name))))
          (fn (and bn

However, it doesn't cover the case where emacs is in a dired buffer 
called django-1.11 with nothing at point. In this case, *.11 will be 
suggested because buffer-name is used. I'm not sure how to fix this in 
grep.el without an "if is dired" check.
That said, I do like the fact that this approach preserves the existing 
aliasing behaviour (e.g. *.c -> *.[ch]), so I've changed the approach of 
the previous patch to let major modes define the file-name grep should 
use rather than extension:

diff --git a/lisp/dired.el b/lisp/dired.el
index 3cb645eea7..d66f12b3a6 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -4138,6 +4138,16 @@ dired-restore-desktop-buffer
  (add-to-list 'desktop-buffer-mode-handlers
              '(dired-mode . dired-restore-desktop-buffer))

+(defun dired-grep-file-name-for-default-pattern ()
+  "Use file at point as the file for grep's default file-name pattern 
suggestion.
+If a directory or nothing is found at point, return nil."
+  (let ((dired-file-name (run-hook-with-args-until-success 
'file-name-at-point-functions)))
+    (if (and dired-file-name
+            (not (file-directory-p dired-file-name)))
+       dired-file-name)))
+(put 'dired-mode 'grep-file-name-for-default-pattern-function 
'dired-grep-file-name-for-default-pattern)
+
+
  (provide 'dired)

  (run-hooks 'dired-load-hook)           ; for your customizations
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 3fd2a7e701..5fb56a33be 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -956,8 +956,12 @@ grep-read-files
  The pattern can include shell wildcards.  As whitespace triggers
  completion when entering a pattern, including it requires
  quoting, e.g. `\\[quoted-insert]<space>'."
-  (let* ((bn (or (buffer-file-name)
-                (replace-regexp-in-string "<[0-9]+>\\'" "" 
(buffer-name))))
+  (let* ((file-name-for-default-pattern-function
+          (get major-mode 
'grep-file-name-for-default-pattern-function))
+        (bn (if file-name-for-default-pattern-function
+                (funcall file-name-for-default-pattern-function)
+              (or (buffer-file-name)
+                (replace-regexp-in-string "<[0-9]+>\\'" "" 
(buffer-name)))))
          (fn (and bn
                   (stringp bn)
                   (file-name-nondirectory bn)))

Changelog entry:
* lisp/progmodes/grep.el (grep-read-files): Allow major modes to
define file name to use for default search pattern
* lisp/dired.el (dired-grep-file-name-for-default-pattern): Use file at 
point
for grep file name pattern

Let me know if there are any further improvements I can make.

Regards,
Chris

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Noam Postavsky]

> +  (let* ((file-name-at-point (run-hook-with-args-until-success
> 'file-name-at-point-functions))

Just a minor suggestion meta suggestion: post your patches by attaching
the output from 'git format-patch', rather than inlining the output of
git diff.  That will avoid patch corruption due to line-wrapping, like
in the above quote (although that line is also a bit long, and you
should break it manually).

> Changelog entry:
> * lisp/progmodes/grep.el (grep-read-files): Allow major modes to
> define file name to use for default search pattern
> * lisp/dired.el (dired-grep-file-name-for-default-pattern): Use file
> at point
> for grep file name pattern

And the ChangeLog entry then goes in the git commit message.  It's much
easier for other people to apply your patch that way.

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Christopher Thorne]

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

> Just a minor suggestion meta suggestion: post your patches by attaching
> the output from 'git format-patch', rather than inlining the output of
> git diff.  That will avoid patch corruption due to line-wrapping, like
> in the above quote (although that line is also a bit long, and you
> should break it manually).

Thanks, Noam. Trying this now. Patch should be attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rgrep-in-dired-using-directory-for-search-file-p.patch --]
[-- Type: text/x-diff; name=0001-Fix-rgrep-in-dired-using-directory-for-search-file-p.patch, Size: 2316 bytes --]

From af551683f9ff6c9bcdd967135a72a268eda6177a Mon Sep 17 00:00:00 2001
From: Christopher Thorne <c.thorne@reckondigital.com>
Date: Tue, 9 Apr 2019 13:12:39 +0100
Subject: [PATCH] Fix rgrep in dired using directory for search file pattern
 (Bug#34621)

* lisp/progmodes/grep.el (grep-read-files): Allow major modes to
define file name to use for default search pattern
* lisp/dired.el (dired-grep-file-name-for-default-pattern): Use file
at point for grep file name pattern
---
 lisp/dired.el          | 11 +++++++++++
 lisp/progmodes/grep.el |  8 ++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lisp/dired.el b/lisp/dired.el
index fc0b71238b..33abca550a 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -4162,6 +4162,17 @@ dired-restore-desktop-buffer
 (add-to-list 'desktop-buffer-mode-handlers
 	     '(dired-mode . dired-restore-desktop-buffer))
 
+(defun dired-grep-file-name-for-default-pattern ()
+  "Use file at point as the file for grep's default file-name pattern suggestion.
+If a directory or nothing is found at point, return nil."
+  (let ((dired-file-name
+	  (run-hook-with-args-until-success 'file-name-at-point-functions)))
+    (if (and dired-file-name
+	     (not (file-directory-p dired-file-name)))
+	dired-file-name)))
+(put 'dired-mode 'grep-file-name-for-default-pattern-function 'dired-grep-file-name-for-default-pattern)
+
+
 (provide 'dired)
 
 (run-hooks 'dired-load-hook)		; for your customizations
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index c0f47159c9..b0bb8e6924 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -959,8 +959,12 @@ grep-read-files
 The pattern can include shell wildcards.  As whitespace triggers
 completion when entering a pattern, including it requires
 quoting, e.g. `\\[quoted-insert]<space>'."
-  (let* ((bn (or (buffer-file-name)
-		 (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name))))
+  (let* ((file-name-for-default-pattern-function
+	   (get major-mode 'grep-file-name-for-default-pattern-function))
+	 (bn (if file-name-for-default-pattern-function
+		 (funcall file-name-for-default-pattern-function)
+	       (or (buffer-file-name)
+		 (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name)))))
 	 (fn (and bn
 		  (stringp bn)
 		  (file-name-nondirectory bn)))
-- 
2.11.0

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Eli Zaretskii]

> Date: Tue, 09 Apr 2019 13:23:11 +0100
> From: Christopher Thorne <c.thorne@reckondigital.com>
> Cc: Noam Postavsky <npostavs@gmail.com>
> 
> > Just a minor suggestion meta suggestion: post your patches by attaching
> > the output from 'git format-patch', rather than inlining the output of
> > git diff.  That will avoid patch corruption due to line-wrapping, like
> > in the above quote (although that line is also a bit long, and you
> > should break it manually).
> 
> Thanks, Noam. Trying this now. Patch should be attached.

One more nit to a well-formatted patch:

> * lisp/progmodes/grep.el (grep-read-files): Allow major modes to
> define file name to use for default search pattern
> * lisp/dired.el (dired-grep-file-name-for-default-pattern): Use file
> at point for grep file name pattern

Please end each sentence with a period.

Also, it is better to have the bug number as part of the detailed
descriptions, not as part of the header line (in Subject), because
that leaves you more space for the header.

Thanks.

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Christopher Thorne]

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

> One more nit to a well-formatted patch:

Thanks, Eli. I've updated the commit message as requested.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rgrep-in-dired-using-directory-for-search-file-p.patch --]
[-- Type: text/x-diff; name=0001-Fix-rgrep-in-dired-using-directory-for-search-file-p.patch, Size: 2318 bytes --]

From f4c6496e66de13438938515719709511f8036a96 Mon Sep 17 00:00:00 2001
From: Christopher Thorne <c.thorne@reckondigital.com>
Date: Tue, 9 Apr 2019 13:12:39 +0100
Subject: [PATCH] Fix rgrep in dired using directory for search file pattern

* lisp/progmodes/grep.el (grep-read-files): Allow major modes to
define file name to use for default search pattern.
* lisp/dired.el (dired-grep-file-name-for-default-pattern): Use file
at point for grep file name pattern.  (Bug#34621)
---
 lisp/dired.el          | 11 +++++++++++
 lisp/progmodes/grep.el |  8 ++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lisp/dired.el b/lisp/dired.el
index fc0b71238b..33abca550a 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -4162,6 +4162,17 @@ dired-restore-desktop-buffer
 (add-to-list 'desktop-buffer-mode-handlers
 	     '(dired-mode . dired-restore-desktop-buffer))
 
+(defun dired-grep-file-name-for-default-pattern ()
+  "Use file at point as the file for grep's default file-name pattern suggestion.
+If a directory or nothing is found at point, return nil."
+  (let ((dired-file-name
+	  (run-hook-with-args-until-success 'file-name-at-point-functions)))
+    (if (and dired-file-name
+	     (not (file-directory-p dired-file-name)))
+	dired-file-name)))
+(put 'dired-mode 'grep-file-name-for-default-pattern-function 'dired-grep-file-name-for-default-pattern)
+
+
 (provide 'dired)
 
 (run-hooks 'dired-load-hook)		; for your customizations
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index c0f47159c9..b0bb8e6924 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -959,8 +959,12 @@ grep-read-files
 The pattern can include shell wildcards.  As whitespace triggers
 completion when entering a pattern, including it requires
 quoting, e.g. `\\[quoted-insert]<space>'."
-  (let* ((bn (or (buffer-file-name)
-		 (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name))))
+  (let* ((file-name-for-default-pattern-function
+	   (get major-mode 'grep-file-name-for-default-pattern-function))
+	 (bn (if file-name-for-default-pattern-function
+		 (funcall file-name-for-default-pattern-function)
+	       (or (buffer-file-name)
+		 (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name)))))
 	 (fn (and bn
 		  (stringp bn)
 		  (file-name-nondirectory bn)))
-- 
2.11.0

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Juri Linkov]

> However, it doesn't cover the case where emacs is in a dired buffer called
> django-1.11 with nothing at point. In this case, *.11 will be suggested
> because buffer-name is used. I'm not sure how to fix this in grep.el
> without an "if is dired" check.

When I tried the change that I suggested to use, I forgot that I have
this customization:

  (add-hook 'dired-after-readin-hook
            (lambda ()
              ;; Set name of dired buffers to absolute directory names.
              (when (stringp dired-directory)
                (rename-buffer (generate-new-buffer-name dired-directory)))))

that renames dired buffers to e.g. "django-1.11/", and due to the
trailing slash, grep-read-files doesn't use it's buffer name.
But since by default there is no trailing slash in dired buffer names,
this is unsuitable.  So I agree with your approach to allow modes
to override using buffer names as candidates.

> diff --git a/lisp/dired.el b/lisp/dired.el
> index fc0b71238b..33abca550a 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -4162,6 +4162,17 @@ dired-restore-desktop-buffer
>  (add-to-list 'desktop-buffer-mode-handlers
>  	     '(dired-mode . dired-restore-desktop-buffer))
>  
> +(defun dired-grep-file-name-for-default-pattern ()
> +  "Use file at point as the file for grep's default file-name pattern suggestion.
> +If a directory or nothing is found at point, return nil."
> +  (let ((dired-file-name
> +	  (run-hook-with-args-until-success 'file-name-at-point-functions)))
> +    (if (and dired-file-name
> +	     (not (file-directory-p dired-file-name)))
> +	dired-file-name)))
> +(put 'dired-mode 'grep-file-name-for-default-pattern-function 'dired-grep-file-name-for-default-pattern)

This is an overwhelmingly long name, please use the same name as
the function that uses this feature, i.e. just 'grep-read-files'.
Then when someone reading dired.el will find a line

  (put 'dired-mode 'grep-read-files 'dired-grep-read-files)

it will be clear for them the purpose of this feature.

Please place this change in dired.el after the definition of
'dired-file-name-at-point' that you can use in your dired function
instead of (run-hook-with-args-until-success 'file-name-at-point-functions)

> diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
> index c0f47159c9..b0bb8e6924 100644
> --- a/lisp/progmodes/grep.el
> +++ b/lisp/progmodes/grep.el
> @@ -959,8 +959,12 @@ grep-read-files
>  The pattern can include shell wildcards.  As whitespace triggers
>  completion when entering a pattern, including it requires
>  quoting, e.g. `\\[quoted-insert]<space>'."
> -  (let* ((bn (or (buffer-file-name)
> -		 (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name))))
> +  (let* ((file-name-for-default-pattern-function
> +	   (get major-mode 'grep-file-name-for-default-pattern-function))
> +	 (bn (if file-name-for-default-pattern-function
> +		 (funcall file-name-for-default-pattern-function)
> +	       (or (buffer-file-name)
> +		 (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name)))))

If you want you could also add as a 4th option additionally
(run-hook-with-args-until-success 'file-name-at-point-functions)
to automatically support modes other than dired, i.e. other modes
that set file-name-at-point-functions.

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Christopher Thorne]

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

> This is an overwhelmingly long name, please use the same name as
> the function that uses this feature, i.e. just 'grep-read-files'.
> Then when someone reading dired.el will find a line
> 
>   (put 'dired-mode 'grep-read-files 'dired-grep-read-files)
> 
> it will be clear for them the purpose of this feature.

Good point, I've changed it to grep-read-files.

> Please place this change in dired.el after the definition of
> 'dired-file-name-at-point' that you can use in your dired function
> instead of (run-hook-with-args-until-success 
> 'file-name-at-point-functions)

Thanks, also done this.

> If you want you could also add as a 4th option additionally
> (run-hook-with-args-until-success 'file-name-at-point-functions)
> to automatically support modes other than dired, i.e. other modes
> that set file-name-at-point-functions.

Sounds like a sensible default, added this as well.

Patch attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rgrep-in-dired-using-directory-for-search-file-p.patch --]
[-- Type: text/x-diff; name=0001-Fix-rgrep-in-dired-using-directory-for-search-file-p.patch, Size: 2470 bytes --]

From bbf584e0c5f836c55579b9804e091c736bcf7a23 Mon Sep 17 00:00:00 2001
From: Christopher Thorne <c.thorne@reckondigital.com>
Date: Tue, 9 Apr 2019 13:12:39 +0100
Subject: [PATCH] Fix rgrep in dired using directory for search file pattern

* lisp/progmodes/grep.el (grep-read-files): Allow major modes to
define file name to use for default search pattern.
* lisp/progmodes/grep.el (grep-read-files): Add non-directory file at
point as default search pattern candidate.
* lisp/dired.el (dired-grep-read-files): Use non-directory file at
point for grep file name pattern.  (Bug#34621)
---
 lisp/dired.el          |  9 +++++++++
 lisp/progmodes/grep.el | 12 ++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/lisp/dired.el b/lisp/dired.el
index fc0b71238b..63d28835aa 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -774,6 +774,15 @@ dired-file-name-at-point
 	  (file-name-as-directory (abbreviate-file-name filename))
 	(abbreviate-file-name filename)))))
 
+(defun dired-grep-read-files ()
+  "Use file at point as the file for grep's default file-name pattern suggestion.
+If a directory or nothing is found at point, return nil."
+  (let ((file-name (dired-file-name-at-point)))
+    (if (and file-name
+	     (not (file-directory-p file-name)))
+	file-name)))
+(put 'dired-mode 'grep-read-files 'dired-grep-read-files)
+
 ;;;###autoload (define-key ctl-x-map "d" 'dired)
 ;;;###autoload
 (defun dired (dirname &optional switches)
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index c0f47159c9..8c7a58fd8b 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -959,8 +959,16 @@ grep-read-files
 The pattern can include shell wildcards.  As whitespace triggers
 completion when entering a pattern, including it requires
 quoting, e.g. `\\[quoted-insert]<space>'."
-  (let* ((bn (or (buffer-file-name)
-		 (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name))))
+  (let* ((grep-read-files-function (get major-mode 'grep-read-files))
+	 (file-name-at-point
+	   (run-hook-with-args-until-success 'file-name-at-point-functions))
+	 (bn (if grep-read-files-function
+		 (funcall grep-read-files-function)
+	       (or (if (and (stringp file-name-at-point)
+			    (not (file-directory-p file-name-at-point)))
+		       file-name-at-point)
+		   (buffer-file-name)
+		   (replace-regexp-in-string "<[0-9]+>\\'" "" (buffer-name)))))
 	 (fn (and bn
 		  (stringp bn)
 		  (file-name-nondirectory bn)))
-- 
2.11.0

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Juri Linkov]

> Patch attached.
>
> From bbf584e0c5f836c55579b9804e091c736bcf7a23 Mon Sep 17 00:00:00 2001
> From: Christopher Thorne <c.thorne@reckondigital.com>
> Date: Tue, 9 Apr 2019 13:12:39 +0100
> Subject: [PATCH] Fix rgrep in dired using directory for search file pattern
>
> * lisp/progmodes/grep.el (grep-read-files): Allow major modes to
> define file name to use for default search pattern.
> * lisp/progmodes/grep.el (grep-read-files): Add non-directory file at
> point as default search pattern candidate.
> * lisp/dired.el (dired-grep-read-files): Use non-directory file at
> point for grep file name pattern.  (Bug#34621)

Thanks, I tested your patch, and it works without problems.

bug#34621: [PATCH] Fix rgrep in dired taking default search file pattern from directory name (e.g. *.11 for django-1.11)

[Juri Linkov]

> Patch attached.

Your patch is pushed to master, thanks.