GNU bug report logs - #42411
Bug with M-x compile

Previous Next

Package: emacs;

Reported by: Gregory Heytings <ghe <at> sdf.org>

Date: Sat, 18 Jul 2020 09:03:01 UTC

Severity: normal

Tags: fixed, patch

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Sat, 18 Jul 2020 09:03:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Gregory Heytings <ghe <at> sdf.org>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sat, 18 Jul 2020 09:03:01 GMT) Full text and rfc822 format available.

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

From: Gregory Heytings <ghe <at> sdf.org>
To: bug-gnu-emacs <at> gnu.org
Subject: Bug with M-x compile
Date: Sat, 18 Jul 2020 09:01:48 +0000
Thanks for fixing (the second part of) bug#42383.

The first part remains unfixed, however:

There are too many completion candidates in the list of targets when 
completing M-x compile.  For example, for the Makefile 
"foo:\n\techo\x20bar:\n" three candidates are displayed: "foo", "echo" and 
"bar".  The regexp in pcmpl-gnu-make-targets is too large, and should be 
fixed as follows:

--- pcmpl-gnu.el.orig   2020-06-29 17:39:26.000000000 +0000
+++ pcmpl-gnu.el        2020-07-15 22:43:14.368938346 +0000
@@ -118,7 +118,7 @@
 Return the new list."
   (goto-char (point-min))
   (while (re-search-forward
-         "^\\s-*\\([^\n#%.$][^:=\n]*\\)\\s-*:[^=]" nil t)
+          "^\\([^\t\n#%.$][^:=\n]*\\)\\s-*:[^=]" nil t)
     (setq targets (nconc (split-string (match-string-no-properties 1))
                          targets)))
   targets)

I see no reason to allow one or more TABs or spaces at the beginning of 
targets, as does the "^\\s-*".  If one really wants to allow spaces (but 
not TABs) at the beginning of a target label, the following regexp could 
also be used: "^ *\\([^\t\n#%.$][^:=\n]*\\)\\s-*:[^=]".

The current regexp is an old one (since Emacs 21 at least), and is 
inconsistent with for example how bash computes completions (see 
_make_target_extract_script).

If changing the regexp is not an option, please make it a configuration 
option.

Gregory




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Sat, 25 Jul 2020 07:26:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gregory Heytings <ghe <at> sdf.org>
Cc: 42411 <at> debbugs.gnu.org
Subject: Re: bug#42411: Bug with M-x compile
Date: Sat, 25 Jul 2020 10:24:46 +0300
> Date: Sat, 18 Jul 2020 09:01:48 +0000
> From: Gregory Heytings via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> There are too many completion candidates in the list of targets when 
> completing M-x compile.  For example, for the Makefile 
> "foo:\n\techo\x20bar:\n" three candidates are displayed: "foo", "echo" and 
> "bar".  The regexp in pcmpl-gnu-make-targets is too large, and should be 
> fixed as follows:

I'm not sure your proposed change is TRT.  It could very well cause
the opposite problem: valid targets are not proposed as completion
candidates.

You seem to assume that the valid candidates are only those that
appear as explicit targets in a Makefile.  But that disregards
implicit targets, which Make intuits on its own.  In the example you
show, those implicit targets make no sense, but that's not the only
use case we might want to support.  In a more general case, there
could be targets collected that way which are non-trivial, and which
users may wish to have as completion candidates.

Another situation we need to consider is the X makefiles, where
target names were preceded by whitespace.

And there could be other situations as well.  I'm not an expert; if we
want to review all the possible use cases, perhaps we should ask Paul
Smith, the GNU Make maintainer, to join this discussion and help us
enumerate the possible cases.

So I think we could have the change you propose as an optional
feature, but certainly not as the only behavior.  We could later
discuss whether this would be an opt-in or opt-out, but IMO it must be
controlled by a user option.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Sat, 25 Jul 2020 23:30:02 GMT) Full text and rfc822 format available.

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

From: Gregory Heytings <ghe <at> sdf.org>
To: 42411 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#42411: Bug with M-x compile
Date: Sat, 25 Jul 2020 23:29:09 +0000
>
>> There are too many completion candidates in the list of targets when 
>> completing M-x compile.  For example, for the Makefile 
>> "foo:\n\techo\x20bar:\n" three candidates are displayed: "foo", "echo" 
>> and "bar".  The regexp in pcmpl-gnu-make-targets is too large, and 
>> should be fixed as follows:
>
> I'm not sure your proposed change is TRT.
>

I proposed three options: (1) the regexp "^\\([^\t\n#%.$][^:=\n]*\\)\\s-*:[^=]", (2) the regexp "^ *\\([^\t\n#%.$][^:=\n]*\\)\\s-*:[^=]", (3) make that regexp a configuration option.

>
> It could very well cause the opposite problem: valid targets are not 
> proposed as completion candidates.
>

I agree with you that in more complex cases, it is possible that valid 
targets would not be proposed as completion candidates anymore.  But OTOH 
I don't think it is possible to have a complete list of valid targets only 
by parsing the Makefile with a regexp.  To have such a list it is 
necessary to use make itself (for example by using the output of make 
--print-data-base for GNU Make).

>
> Another situation we need to consider is the X makefiles, where target 
> names were preceded by whitespace.
>

Yes, that was my option (2).

>
> And there could be other situations as well.  I'm not an expert; if we 
> want to review all the possible use cases, perhaps we should ask Paul 
> Smith, the GNU Make maintainer, to join this discussion and help us 
> enumerate the possible cases.
>

I'm not an expert either, so yes, please ask Paul Smith for advice on 
this.  I do think that the way to compute completion candidates should be 
improved.

There will always be exceptional cases (for example, for GNU Make, 
.RECIPEPREFIX with which it is possible to use another prefix character 
instead of TAB can apparently be used multiple times), but for something 
like 99.9% cases, a line starting with a TAB is a recipe element and not a 
target, a line starting with a '#' is a comment, and a line starting with 
a '.' sets a special variable.  The current regexp correctly excludes the 
last two, but includes the first one.

>
> So I think we could have the change you propose as an optional feature, 
> but certainly not as the only behavior.  We could later discuss whether 
> this would be an opt-in or opt-out, but IMO it must be controlled by a 
> user option.
>

Yes, that was my option (3).  I certainly don't want to impose a change on 
everyone, especially as it has been Emacs's behavior for a long time. 
Ideally I think that it should be controlled by a variable, with a 
docstring presenting a number of typical values.

Gregory




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Sun, 26 Jul 2020 13:56:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Smith <psmith <at> gnu.org>
Cc: 42411 <at> debbugs.gnu.org
Subject: Re: bug#42411: Bug with M-x compile
Date: Sun, 26 Jul 2020 16:55:29 +0300
> Date: Sat, 25 Jul 2020 23:29:09 +0000
> From: Gregory Heytings <ghe <at> sdf.org>
> cc: Eli Zaretskii <eliz <at> gnu.org>
> 
> > And there could be other situations as well.  I'm not an expert; if we 
> > want to review all the possible use cases, perhaps we should ask Paul 
> > Smith, the GNU Make maintainer, to join this discussion and help us 
> > enumerate the possible cases.
> >
> 
> I'm not an expert either, so yes, please ask Paul Smith for advice on 
> this.  I do think that the way to compute completion candidates should be 
> improved.
> 
> There will always be exceptional cases (for example, for GNU Make, 
> .RECIPEPREFIX with which it is possible to use another prefix character 
> instead of TAB can apparently be used multiple times), but for something 
> like 99.9% cases, a line starting with a TAB is a recipe element and not a 
> target, a line starting with a '#' is a comment, and a line starting with 
> a '.' sets a special variable.  The current regexp correctly excludes the 
> last two, but includes the first one.

Paul, could you please chime in and share your views on this?  If you
want to read the discussion from the beginning, you can find it at

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=42411

or, if you prefer to read all of it in your MUA, you can download all
the messages in the mbox format:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=42411;mbox=yes

TIA




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Fri, 31 Jul 2020 18:21:01 GMT) Full text and rfc822 format available.

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

From: Paul Smith <psmith <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 42411 <at> debbugs.gnu.org
Subject: Re: bug#42411: Bug with M-x compile
Date: Fri, 31 Jul 2020 14:20:38 -0400
On Sun, 2020-07-26 at 16:55 +0300, Eli Zaretskii wrote:
> > > And there could be other situations as well.  I'm not an expert; if we 
> > > want to review all the possible use cases, perhaps we should ask Paul 
> > > Smith, the GNU Make maintainer, to join this discussion and help us 
> > > enumerate the possible cases.
> > I'm not an expert either, so yes, please ask Paul Smith for advice on 
> > this.  I do think that the way to compute completion candidates should be 
> > improved.
> > There will always be exceptional cases (for example, for GNU Make, 
> > .RECIPEPREFIX with which it is possible to use another prefix character 
> > instead of TAB can apparently be used multiple times), but for something 
> > like 99.9% cases, a line starting with a TAB is a recipe element and not a 
> > target, a line starting with a '#' is a comment, and a line starting with 
> > a '.' sets a special variable.  The current regexp correctly excludes the 
> > last two, but includes the first one.
> 
> 
> Paul, could you please chime in and share your views on this?  If you
> want to read the discussion from the beginning, you can find it at
> 
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=42411
> 
> or, if you prefer to read all of it in your MUA, you can download all
> the messages in the mbox format:
> 
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=42411;mbox=yes

Sorry for the delay in response: it's been a week at $DAYJOB.

I guess I'm not exactly sure what the ask is here.

It's definitely true that technically, it is possible to have targets
that are indented by TABs.  A line indented by a TAB is only considered
part of a recipe if it appears in the "recipe context", which means
somewhere that a recipe is legal in the syntax.

If it's not legal for a recipe command to appear there then TABs are
treated like any other whitespace.

In practice, I think it's highly unlikely that anyone would
intentionally use TABs to indent targets because it's so fragile: any
reordering of the makefile or adding new lines could cause that
makefile to break.

So, as a simplifying assumption it makes sense to me to ignore any line
starting with TAB when trying to detect targets.

Of course, as Eli points out there are certainly a large number of
potential targets which cannot be determined using this type of simple
investigation.  The most obvious are targets that match patterns.

However I'll say two things about this:

First, I think it's unlikely that users would really want to see all
the potential matches of targets when doing completion.  It's most
likely that they are interested in the "top level" intended command
line goals rather than every possible object, source, etc. file that
make considers a target due to pattern or suffix rules.

Second, I don't think there's currently any good way to list those
targets anyway.  Just using --print-database by itself won't do it.
 Using the -n option will help, but many makefiles are not carefully
written to ensure that -n is really idempotent, and make -n could
delete files or similar operations.  And of course this still only
finds the targets that are available "by default"; providing a target
on the command line could cause more pattern rules to generate more
targets that the "default" goal target doesn't.

I hope that helps but if I completely missed the point please feel free
to redirect me!

Cheers, and stay safe;
Paul





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Fri, 31 Jul 2020 18:43:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: psmith <at> gnu.org
Cc: 42411 <at> debbugs.gnu.org
Subject: Re: bug#42411: Bug with M-x compile
Date: Fri, 31 Jul 2020 21:42:01 +0300
> From: Paul Smith <psmith <at> gnu.org>
> Cc: 42411 <at> debbugs.gnu.org
> Date: Fri, 31 Jul 2020 14:20:38 -0400
> 
> It's definitely true that technically, it is possible to have targets
> that are indented by TABs.  A line indented by a TAB is only considered
> part of a recipe if it appears in the "recipe context", which means
> somewhere that a recipe is legal in the syntax.
> 
> If it's not legal for a recipe command to appear there then TABs are
> treated like any other whitespace.
> 
> In practice, I think it's highly unlikely that anyone would
> intentionally use TABs to indent targets because it's so fragile: any
> reordering of the makefile or adding new lines could cause that
> makefile to break.
> 
> So, as a simplifying assumption it makes sense to me to ignore any line
> starting with TAB when trying to detect targets.
> 
> Of course, as Eli points out there are certainly a large number of
> potential targets which cannot be determined using this type of simple
> investigation.  The most obvious are targets that match patterns.
> 
> However I'll say two things about this:
> 
> First, I think it's unlikely that users would really want to see all
> the potential matches of targets when doing completion.  It's most
> likely that they are interested in the "top level" intended command
> line goals rather than every possible object, source, etc. file that
> make considers a target due to pattern or suffix rules.
> 
> Second, I don't think there's currently any good way to list those
> targets anyway.  Just using --print-database by itself won't do it.
>  Using the -n option will help, but many makefiles are not carefully
> written to ensure that -n is really idempotent, and make -n could
> delete files or similar operations.  And of course this still only
> finds the targets that are available "by default"; providing a target
> on the command line could cause more pattern rules to generate more
> targets that the "default" goal target doesn't.
> 
> I hope that helps but if I completely missed the point please feel free
> to redirect me!

Thanks for the feedback.

So you think the current regexp is trying to match too much, and the
proposed change is TRT and we should make it unconditionally?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Fri, 31 Jul 2020 18:59:02 GMT) Full text and rfc822 format available.

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

From: Paul Smith <psmith <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 42411 <at> debbugs.gnu.org
Subject: Re: bug#42411: Bug with M-x compile
Date: Fri, 31 Jul 2020 14:58:28 -0400
On Fri, 2020-07-31 at 21:42 +0300, Eli Zaretskii wrote:
> So you think the current regexp is trying to match too much, and the
> proposed change is TRT and we should make it unconditionally?

I think so yes.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#42411; Package emacs. (Fri, 21 Aug 2020 10:47:01 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Gregory Heytings <ghe <at> sdf.org>
Cc: 42411 <at> debbugs.gnu.org
Subject: Re: bug#42411: Bug with M-x compile
Date: Fri, 21 Aug 2020 12:45:52 +0200
Gregory Heytings <ghe <at> sdf.org> writes:

>    (while (re-search-forward
> -         "^\\s-*\\([^\n#%.$][^:=\n]*\\)\\s-*:[^=]" nil t)
> +          "^\\([^\t\n#%.$][^:=\n]*\\)\\s-*:[^=]" nil t)
>      (setq targets (nconc (split-string (match-string-no-properties 1))

Paul Smith <psmith <at> gnu.org> writes:

> On Fri, 2020-07-31 at 21:42 +0300, Eli Zaretskii wrote:
>> So you think the current regexp is trying to match too much, and the
>> proposed change is TRT and we should make it unconditionally?
>
> I think so yes.

OK; I've now applied Gregory's patch to Emacs 28 (after checking a bit).

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




Added tag(s) patch. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Fri, 21 Aug 2020 10:52:02 GMT) Full text and rfc822 format available.

Added tag(s) fixed. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Fri, 21 Aug 2020 10:52:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.1, send any further explanations to 42411 <at> debbugs.gnu.org and Gregory Heytings <ghe <at> sdf.org> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Fri, 21 Aug 2020 10:52:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 18 Sep 2020 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 191 days ago.

Previous Next


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