unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#23590: 25.0.94; Errors in default lgrep command
@ 2016-05-21  0:11 Alex
  2016-07-23  2:16 ` John Mastro
  0 siblings, 1 reply; 19+ messages in thread
From: Alex @ 2016-05-21  0:11 UTC (permalink / raw)
  To: 23590


The default lgrep command (`all' for the FILES argument) gives
off unexpected errors.

* When there are no hidden (dot) files in the given directory, then I
  get the following error:

zsh:1: no matches found: *.
Grep exited abnormally with code 1 at ...

By default lgrep uses my default shell (zsh). Should this be the case?
Setting `shell-file-name' to "bash" fixes this error, but it still
produces the next error.

* When there are directories in the given directory (this includes . and
  ..) then lgrep produces an error for each directory. For example:
  

grep: .: Is a directory
grep: ..: Is a directory
grep: .emacs.d: Is a directory

Grep exited abnormally with code 2 at ...


In GNU Emacs 25.0.94.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.18.9)
 of 2016-05-17 built on lylat
Windowing system distributor 'Fedora Project', version 11.0.11803000
Configured using:
 'configure --with-gif=no'

Configured features:
XPM JPEG TIFF PNG SOUND DBUS GSETTINGS NOTIFY FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_CTYPE: en_CA.utf8
  value of $LANG: en_CA.utf8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t


Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
cl-loaddefs pcase cl-lib mail-prsvr mail-utils thingatpt grep compile
comint ansi-color ring time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 95915 8690)
 (symbols 48 20220 0)
 (miscs 40 50 221)
 (strings 32 16338 4494)
 (string-bytes 1 485576)
 (vectors 16 12554)
 (vector-slots 8 436687 5729)
 (floats 8 172 70)
 (intervals 56 346 19)
 (buffers 976 12)
 (heap 1024 43598 1075))





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-05-21  0:11 bug#23590: 25.0.94; Errors in default lgrep command Alex
@ 2016-07-23  2:16 ` John Mastro
  2016-07-23  7:45   ` Eli Zaretskii
  2016-07-23 17:11   ` Alex
  0 siblings, 2 replies; 19+ messages in thread
From: John Mastro @ 2016-07-23  2:16 UTC (permalink / raw)
  To: 23590; +Cc: Alex

Alex <agrambot@gmail.com> wrote:
> The default lgrep command (`all' for the FILES argument) gives
> off unexpected errors.
>
> * When there are no hidden (dot) files in the given directory, then I
>   get the following error:
>
> zsh:1: no matches found: *.
> Grep exited abnormally with code 1 at ...
>
> By default lgrep uses my default shell (zsh). Should this be the case?
> Setting `shell-file-name' to "bash" fixes this error, but it still
> produces the next error.

Thanks for reporting this. I've seen it too.

The problem seems to be caused by a difference in how zsh handles globs
that don't match anything, compared to bash. I'm not sure what the right
way would be to accommodate it in Emacs. Hopefully someone will be along
shortly with ideas for that. However, there are a couple things you can
do to work around it in the meantime.

First, you could change the command used by `lgrep' to enable zsh's
NULL_GLOB option, of which the zsh documentation[1] says: "If a pattern
for filename generation has no matches, delete the pattern from the
argument list instead of reporting an error". To do that, you could use
something like:

(with-eval-after-load 'grep
  (grep-apply-setting
   'grep-template
   "setopt null_glob; grep <X> <C> -n -e <R> <F>"))

Second, you could use advice on `lgrep' so that it invokes grep via bash
rather than zsh. That would look like:

(defun grep-use-bash (original &rest args)
  (let ((shell-file-name (executable-find "bash")))
    (apply original args)))

(with-eval-after-load 'grep
  (advice-add 'lgrep :around #'grep-use-bash))

I tested both options only briefly; apologies if I missed any issues.

> * When there are directories in the given directory (this includes . and
>   ..) then lgrep produces an error for each directory. For example:
>
>
> grep: .: Is a directory
> grep: ..: Is a directory
> grep: .emacs.d: Is a directory
>
> Grep exited abnormally with code 2 at ...

GNU Grep has an option (-d ACTION or --directories=ACTION) that can be
used to skip over directories (with "skip" as the ACTION), but it's not
in POSIX so I doubt we can use it in Emacs. If you know it will be
available on your system(s), you could add it to your `grep-template'
using the same technique as above.

[1] http://www.cs.elte.hu/zsh-manual/zsh_16.html

        John





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23  2:16 ` John Mastro
@ 2016-07-23  7:45   ` Eli Zaretskii
  2016-07-23 16:57     ` John Mastro
  2016-07-23 17:28     ` Alex
  2016-07-23 17:11   ` Alex
  1 sibling, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2016-07-23  7:45 UTC (permalink / raw)
  To: John Mastro; +Cc: 23590, agrambot

> From: John Mastro <john.b.mastro@gmail.com>
> Date: Fri, 22 Jul 2016 19:16:27 -0700
> Cc: Alex <agrambot@gmail.com>
> 
> Alex <agrambot@gmail.com> wrote:
> > The default lgrep command (`all' for the FILES argument) gives
> > off unexpected errors.
> >
> > * When there are no hidden (dot) files in the given directory, then I
> >   get the following error:
> >
> > zsh:1: no matches found: *.
> > Grep exited abnormally with code 1 at ...
> >
> > By default lgrep uses my default shell (zsh). Should this be the case?
> > Setting `shell-file-name' to "bash" fixes this error, but it still
> > produces the next error.
> 
> Thanks for reporting this. I've seen it too.
> 
> The problem seems to be caused by a difference in how zsh handles globs
> that don't match anything, compared to bash. I'm not sure what the right
> way would be to accommodate it in Emacs. Hopefully someone will be along
> shortly with ideas for that.

Shell commands that Emacs emits support /bin/sh and compatible
shells.  Zsh's default treatment of unmatched wildcards isn't.

I don't know how it happened that lgrep invokes zsh on OP's system,
but if that is due to user customizations, they should be corrected.
If that is Emacs's fault (i.e. Emacs invokes zsh without any
customizations), it should be fixed.

> > * When there are directories in the given directory (this includes . and
> >   ..) then lgrep produces an error for each directory. For example:
> >
> >
> > grep: .: Is a directory
> > grep: ..: Is a directory
> > grep: .emacs.d: Is a directory
> >
> > Grep exited abnormally with code 2 at ...
> 
> GNU Grep has an option (-d ACTION or --directories=ACTION) that can be
> used to skip over directories (with "skip" as the ACTION), but it's not
> in POSIX so I doubt we can use it in Emacs. If you know it will be
> available on your system(s), you could add it to your `grep-template'
> using the same technique as above.

Yes, but I don't understand why the OP says these are errors.  They
aren't; they are just informative messages from Grep.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23  7:45   ` Eli Zaretskii
@ 2016-07-23 16:57     ` John Mastro
  2016-07-23 17:15       ` Eli Zaretskii
  2016-07-23 17:28     ` Alex
  1 sibling, 1 reply; 19+ messages in thread
From: John Mastro @ 2016-07-23 16:57 UTC (permalink / raw)
  To: 23590; +Cc: Alex

Eli Zaretskii <eliz@gnu.org> wrote:
> Shell commands that Emacs emits support /bin/sh and compatible
> shells.  Zsh's default treatment of unmatched wildcards isn't.
>
> I don't know how it happened that lgrep invokes zsh on OP's system,
> but if that is due to user customizations, they should be corrected.
> If that is Emacs's fault (i.e. Emacs invokes zsh without any
> customizations), it should be fixed.

Emacs invokes whichever shell is the value of the user's SHELL
environment variable.

My reading of the code is that this is the result of a call chain from
`lgrep', to `compilation-start', to `start-file-process-shell-command',
to `start-file-process'. The only way `shell-file-name' is disregarded
in favor of /bin/sh is if `default-directory' is remote:

(defun start-file-process-shell-command (name buffer &rest args)
  (start-file-process
   ...
   (if (file-remote-p default-directory) "/bin/sh" shell-file-name)
   ...))

And shell-file-name is initialized to the value of SHELL in
init_callproc():

char *sh;
...
sh = getenv ("SHELL");
Vshell_file_name = build_string (sh ? sh : "/bin/sh");

So the user may not have intended to customize Emacs per se, but setting
SHELL does so indirectly.

This is my first time looking at most of this code but, if the intent is
for `lgrep' to always use /bin/sh, the least ugly way I see of doing
that is to let-bind `shell-file-name' in `lgrep'.

Obviously it would remain the case that Emacs uses shell-file-name for
other commands, but since AFAIK that hasn't been a problem more
generally a minimal change may be best.

> Yes, but I don't understand why the OP says these are errors.  They
> aren't; they are just informative messages from Grep.

I'm guessing this was just a misunderstanding about how `lgrep' works.
If the reporter thought of it as "call grep on all files", and didn't
think of directories as files, then it might be surprising at first to
see those messages.

        John





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23  2:16 ` John Mastro
  2016-07-23  7:45   ` Eli Zaretskii
@ 2016-07-23 17:11   ` Alex
  1 sibling, 0 replies; 19+ messages in thread
From: Alex @ 2016-07-23 17:11 UTC (permalink / raw)
  To: John Mastro; +Cc: 23590

John Mastro <john.b.mastro@gmail.com> writes:

> (with-eval-after-load 'grep
>   (grep-apply-setting
>    'grep-template
>    "setopt null_glob; grep <X> <C> -n -e <R> <F>"))


> GNU Grep has an option (-d ACTION or --directories=ACTION) that can be
> used to skip over directories (with "skip" as the ACTION), but it's not
> in POSIX so I doubt we can use it in Emacs. If you know it will be
> available on your system(s), you could add it to your `grep-template'
> using the same technique as above.

Thanks, these workarounds work exactly as you said. It does seem that
outside of setting null_glob somewhere, there isn't a way to get zsh to
behave correctly.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 16:57     ` John Mastro
@ 2016-07-23 17:15       ` Eli Zaretskii
  2016-07-23 17:23         ` John Mastro
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2016-07-23 17:15 UTC (permalink / raw)
  To: John Mastro; +Cc: 23590, agrambot

> From: John Mastro <john.b.mastro@gmail.com>
> Date: Sat, 23 Jul 2016 09:57:02 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>, Alex <agrambot@gmail.com>
> 
> Eli Zaretskii <eliz@gnu.org> wrote:
> > Shell commands that Emacs emits support /bin/sh and compatible
> > shells.  Zsh's default treatment of unmatched wildcards isn't.
> >
> > I don't know how it happened that lgrep invokes zsh on OP's system,
> > but if that is due to user customizations, they should be corrected.
> > If that is Emacs's fault (i.e. Emacs invokes zsh without any
> > customizations), it should be fixed.
> 
> Emacs invokes whichever shell is the value of the user's SHELL
> environment variable.

Then maybe we should change that.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 17:15       ` Eli Zaretskii
@ 2016-07-23 17:23         ` John Mastro
  2016-07-23 17:58           ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: John Mastro @ 2016-07-23 17:23 UTC (permalink / raw)
  To: 23590; +Cc: Alex

Eli Zaretskii <eliz@gnu.org> wrote:
>> Emacs invokes whichever shell is the value of the user's SHELL
>> environment variable.
>
> Then maybe we should change that.

Do you mean for `lgrep' specifically or at a lower level? To be honest,
I don't have a good enough feel for the implications to have a strong
opinion.

This is admittedly an anecdote but, having used zsh as my SHELL (and
`shell-file-name') for several years, this scenario with `lgrep' and the
"all" alias is the only time I've run into a problem.

        John





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23  7:45   ` Eli Zaretskii
  2016-07-23 16:57     ` John Mastro
@ 2016-07-23 17:28     ` Alex
  2016-07-23 18:00       ` Eli Zaretskii
  1 sibling, 1 reply; 19+ messages in thread
From: Alex @ 2016-07-23 17:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: John Mastro, 23590

Eli Zaretskii <eliz@gnu.org> writes:

> Shell commands that Emacs emits support /bin/sh and compatible
> shells.  Zsh's default treatment of unmatched wildcards isn't.
>
> I don't know how it happened that lgrep invokes zsh on OP's system,
> but if that is due to user customizations, they should be corrected.
> If that is Emacs's fault (i.e. Emacs invokes zsh without any
> customizations), it should be fixed.

shell-file-name is set to zsh (as it's my default user shell) in emacs
-Q as well. This can be useful for commands like ansi-term. If an Emacs
program expects full sh compatibility, then perhaps there could be an
extra variable they can check before falling back to shell-file-name?


>> GNU Grep has an option (-d ACTION or --directories=ACTION) that can be
>> used to skip over directories (with "skip" as the ACTION), but it's not
>> in POSIX so I doubt we can use it in Emacs. If you know it will be
>> available on your system(s), you could add it to your `grep-template'
>> using the same technique as above.
>
> Yes, but I don't understand why the OP says these are errors.  They
> aren't; they are just informative messages from Grep.

When I use lgrep I already know that I'm not going to be searching
directories. I find such messages to be useless clutter.

Additionally, while zsh doesn't error on this part (with the workarounds
that John provided), bash does appear to error because it is including
directories:

Grep exited abnormally with code 2 at ...

PS:

I noticed that between 25.0.94 and 25.0.95 the default command changes
slightly. It now includes a wildcard ..?* that bash doesn't seem to like
if there are no matching files:

grep: ..?*: No such file or directory

Grep exited abnormally with code 2 at ...





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 17:23         ` John Mastro
@ 2016-07-23 17:58           ` Eli Zaretskii
  2016-07-23 19:52             ` Glenn Morris
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2016-07-23 17:58 UTC (permalink / raw)
  To: John Mastro; +Cc: 23590, agrambot

> From: John Mastro <john.b.mastro@gmail.com>
> Date: Sat, 23 Jul 2016 10:23:20 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>, Alex <agrambot@gmail.com>
> 
> Eli Zaretskii <eliz@gnu.org> wrote:
> >> Emacs invokes whichever shell is the value of the user's SHELL
> >> environment variable.
> >
> > Then maybe we should change that.
> 
> Do you mean for `lgrep' specifically or at a lower level?

No, I mean in general.

> This is admittedly an anecdote but, having used zsh as my SHELL (and
> `shell-file-name') for several years, this scenario with `lgrep' and the
> "all" alias is the only time I've run into a problem.

Why would you want to use zsh in commands issued by lgrep?





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 17:28     ` Alex
@ 2016-07-23 18:00       ` Eli Zaretskii
  2016-07-23 18:18         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2016-07-23 18:00 UTC (permalink / raw)
  To: Alex; +Cc: john.b.mastro, 23590

> From: Alex <agrambot@gmail.com>
> Cc: John Mastro <john.b.mastro@gmail.com>,  23590@debbugs.gnu.org
> Date: Sat, 23 Jul 2016 11:28:21 -0600
> 
> > Yes, but I don't understand why the OP says these are errors.  They
> > aren't; they are just informative messages from Grep.
> 
> When I use lgrep I already know that I'm not going to be searching
> directories. I find such messages to be useless clutter.

They cannot be avoided portably.  Sorry.

> Additionally, while zsh doesn't error on this part (with the workarounds
> that John provided), bash does appear to error because it is including
> directories:
> 
> Grep exited abnormally with code 2 at ...

Because of this:

> grep: ..?*: No such file or directory





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 18:00       ` Eli Zaretskii
@ 2016-07-23 18:18         ` Eli Zaretskii
  2016-07-23 22:12           ` Alex
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2016-07-23 18:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: john.b.mastro, 23590, agrambot

> Date: Sat, 23 Jul 2016 21:00:37 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: john.b.mastro@gmail.com, 23590@debbugs.gnu.org
> 
> > Grep exited abnormally with code 2 at ...
> 
> Because of this:
> 
> > grep: ..?*: No such file or directory

Actually, I see that "Is a directory" also caused exit code of 2.  So
I guess patches are welcome to add --directory=skip to the Grep
command when a preliminary test reveals that Grep supports it.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 17:58           ` Eli Zaretskii
@ 2016-07-23 19:52             ` Glenn Morris
  0 siblings, 0 replies; 19+ messages in thread
From: Glenn Morris @ 2016-07-23 19:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: John Mastro, 23590, agrambot

Eli Zaretskii wrote:

> Why would you want to use zsh in commands issued by lgrep?

Not in the "commands issued by lgrep", but in the glob pattern passed to lgrep.
One wants to use the same shell as one is used to using interactively,
so that one can use that shell's glob patterns, if it has specific ones,
as zsh does. (I'm not a zsh user.)

I don't think forcing it to use bash is the right solution.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 18:18         ` Eli Zaretskii
@ 2016-07-23 22:12           ` Alex
  2020-09-04 14:09             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Alex @ 2016-07-23 22:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: john.b.mastro, 23590

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 23 Jul 2016 21:00:37 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: john.b.mastro@gmail.com, 23590@debbugs.gnu.org
>> 
>> > Grep exited abnormally with code 2 at ...
>> 
>> Because of this:
>> 
>> > grep: ..?*: No such file or directory
>
> Actually, I see that "Is a directory" also caused exit code of 2.  So
> I guess patches are welcome to add --directory=skip to the Grep
> command when a preliminary test reveals that Grep supports it.

Is the following patch acceptable?

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index f04a722..c696f75 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -573,10 +573,14 @@ This function is called from `compilation-filter-hook'."
                 grep-template grep-find-template)
       (let ((grep-options
             (concat (if grep-use-null-device "-n" "-nH")
-                    (if (grep-probe grep-program
-                                    `(nil nil nil "-e" "foo" ,null-device)
-                                    nil 1)
-                        " -e"))))
+                     (when (grep-probe grep-program
+                                       `(nil nil nil "--directories=skip" "foo" ,null-device)
+                                       nil 1)
+                       " --directories=skip")
+                     (when (grep-probe grep-program
+                                       `(nil nil nil "-e" "foo" ,null-device)
+                                       nil 1)
+                       " -e"))))
        (unless grep-command
          (setq grep-command
                (format "%s %s %s " grep-program





^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2016-07-23 22:12           ` Alex
@ 2020-09-04 14:09             ` Lars Ingebrigtsen
  2020-10-07  3:41               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-04 14:09 UTC (permalink / raw)
  To: Alex; +Cc: john.b.mastro, 23590

Alex <agrambot@gmail.com> writes:

>> Actually, I see that "Is a directory" also caused exit code of 2.  So
>> I guess patches are welcome to add --directory=skip to the Grep
>> command when a preliminary test reveals that Grep supports it.
>
> Is the following patch acceptable?

There was discussion here before the patch was posted, but then no
follow-up.  I've respun the patch for Emacs 28 -- does anybody have any
comments?

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index c71a90344f..17f0422ac7 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -696,10 +696,15 @@ grep-compute-defaults
       (let ((grep-options
 	     (concat (if grep-use-null-device "-n" "-nH")
                      (if grep-use-null-filename-separator " --null")
-		     (if (grep-probe grep-program
-				     `(nil nil nil "-e" "foo" ,null-device)
-				     nil 1)
-			 " -e"))))
+                     (when (grep-probe grep-program
+                                       `(nil nil nil "--directories=skip" "foo"
+                                             ,null-device)
+                                       nil 1)
+                       " --directories=skip")
+                     (when (grep-probe grep-program
+                                       `(nil nil nil "-e" "foo" ,null-device)
+                                       nil 1)
+                       " -e"))))
 	(unless grep-command
 	  (setq grep-command
 		(format "%s %s %s " grep-program


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2020-09-04 14:09             ` Lars Ingebrigtsen
@ 2020-10-07  3:41               ` Lars Ingebrigtsen
  2020-10-07  8:17                 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-07  3:41 UTC (permalink / raw)
  To: Alex; +Cc: john.b.mastro, 23590

Lars Ingebrigtsen <larsi@gnus.org> writes:

> There was discussion here before the patch was posted, but then no
> follow-up.  I've respun the patch for Emacs 28 -- does anybody have any
> comments?

There were no comments in a month, and the patch makes sense to me, so
I've applied it to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2020-10-07  3:41               ` Lars Ingebrigtsen
@ 2020-10-07  8:17                 ` Eli Zaretskii
  2020-10-09  4:15                   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2020-10-07  8:17 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: john.b.mastro, 23590, agrambot

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: john.b.mastro@gmail.com,  Eli Zaretskii <eliz@gnu.org>,
>   23590@debbugs.gnu.org
> Date: Wed, 07 Oct 2020 05:41:56 +0200
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > There was discussion here before the patch was posted, but then no
> > follow-up.  I've respun the patch for Emacs 28 -- does anybody have any
> > comments?
> 
> There were no comments in a month, and the patch makes sense to me, so
> I've applied it to Emacs 28.

I'm sorry to not have chimed earlier, but I think this change goes too
far.  Now the default "M-x grep" command includes --directories=skip
if the user's Grep program supports that.  This is not a good idea for
the general-purpose Grep commands.  For example, if the user adds to
the default command "-R foo" somewhere before the --directories=skip
part, the command will say "no matches", which would be a surprise at
best, and at worst could mislead the user.

The original bug report was about "M-x lgrep".  Can we make this
change affect only that command?

In any case, this is a user-visible change, so it should be called out
in NEWS.

Thanks.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2020-10-07  8:17                 ` Eli Zaretskii
@ 2020-10-09  4:15                   ` Lars Ingebrigtsen
  2020-10-13 20:09                     ` Juri Linkov
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-09  4:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: john.b.mastro, 23590, agrambot

Eli Zaretskii <eliz@gnu.org> writes:

> The original bug report was about "M-x lgrep".  Can we make this
> change affect only that command?
>
> In any case, this is a user-visible change, so it should be called out
> in NEWS.

Yup, and yup.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2020-10-09  4:15                   ` Lars Ingebrigtsen
@ 2020-10-13 20:09                     ` Juri Linkov
  2020-10-14  4:10                       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Juri Linkov @ 2020-10-13 20:09 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: john.b.mastro, 23590, agrambot

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

>> The original bug report was about "M-x lgrep".  Can we make this
>> change affect only that command?
>>
>> In any case, this is a user-visible change, so it should be called out
>> in NEWS.
>
> Yup, and yup.

The latest fix has two problems:

1. it runs grep-probe every time lgrep is used;
2. it adds --directories=skip to the end of the command after regexp and file names

This patch improves both:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grep-use-directories-skip.patch --]
[-- Type: text/x-diff, Size: 2164 bytes --]

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index f028a4279d..9b1dc337e8 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -959,10 +959,10 @@ grep-expand-keywords
 these include `opts', `dir', `files', `null-device', `excl' and
 `regexp'.")
 
-(defun grep-expand-template (template &optional regexp files dir excl)
+(defun grep-expand-template (template &optional regexp files dir excl more-opts)
   "Expand grep COMMAND string replacing <C>, <D>, <F>, <R>, and <X>."
   (let* ((command template)
-         (env `((opts . ,(let (opts)
+         (env `((opts . ,(let ((opts more-opts))
                            (when (and case-fold-search
                                       (isearch-no-upper-case-p regexp t))
                              (push "-i" opts))
@@ -1058,6 +1058,8 @@ grep-read-files
 	 (or (cdr (assoc files grep-files-aliases))
 	     files))))
 
+(defvar grep-use-directories-skip 'auto-detect)
+
 ;;;###autoload
 (defun lgrep (regexp &optional files dir confirm)
   "Run grep, searching for REGEXP in FILES in directory DIR.
@@ -1103,6 +1105,12 @@ lgrep
 	  (if (string= command grep-command)
 	      (setq command nil))
 	(setq dir (file-name-as-directory (expand-file-name dir)))
+	(unless (or (not grep-use-directories-skip) (eq grep-use-directories-skip t))
+	  (setq grep-use-directories-skip
+		(grep-probe grep-program
+			  `(nil nil nil "--directories=skip" "foo"
+				,null-device)
+			  nil 1)))
 	(setq command (grep-expand-template
 		       grep-template
 		       regexp
@@ -1119,13 +1127,10 @@ lgrep
 						     (shell-quote-argument
 						      (cdr ignore))))))
 				     grep-find-ignored-files
-				     " --exclude=")))))
+				     " --exclude=")))
+		       (and grep-use-directories-skip
+			    '("--directories=skip"))))
 	(when command
-          (when (grep-probe grep-program
-                            `(nil nil nil "--directories=skip" "foo"
-                                  ,null-device)
-                            nil 1)
-            (setq command (concat command " --directories=skip")))
 	  (if confirm
 	      (setq command
 		    (read-from-minibuffer "Confirm: "

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#23590: 25.0.94; Errors in default lgrep command
  2020-10-13 20:09                     ` Juri Linkov
@ 2020-10-14  4:10                       ` Lars Ingebrigtsen
  0 siblings, 0 replies; 19+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-14  4:10 UTC (permalink / raw)
  To: Juri Linkov; +Cc: john.b.mastro, 23590, agrambot

Juri Linkov <juri@linkov.net> writes:

> 1. it runs grep-probe every time lgrep is used;
> 2. it adds --directories=skip to the end of the command after regexp
> and file names
>
> This patch improves both:

Looks good; go ahead and push.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-10-14  4:10 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-21  0:11 bug#23590: 25.0.94; Errors in default lgrep command Alex
2016-07-23  2:16 ` John Mastro
2016-07-23  7:45   ` Eli Zaretskii
2016-07-23 16:57     ` John Mastro
2016-07-23 17:15       ` Eli Zaretskii
2016-07-23 17:23         ` John Mastro
2016-07-23 17:58           ` Eli Zaretskii
2016-07-23 19:52             ` Glenn Morris
2016-07-23 17:28     ` Alex
2016-07-23 18:00       ` Eli Zaretskii
2016-07-23 18:18         ` Eli Zaretskii
2016-07-23 22:12           ` Alex
2020-09-04 14:09             ` Lars Ingebrigtsen
2020-10-07  3:41               ` Lars Ingebrigtsen
2020-10-07  8:17                 ` Eli Zaretskii
2020-10-09  4:15                   ` Lars Ingebrigtsen
2020-10-13 20:09                     ` Juri Linkov
2020-10-14  4:10                       ` Lars Ingebrigtsen
2016-07-23 17:11   ` Alex

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).