bug#42383: 28.0.50; Two bugs with M-x compile

bug#42383: 28.0.50; Two bugs with M-x compile

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

** Bug 1 **

Steps to reproduce:

1. create a Makefile with a few targets
2. start emacs -Q
3. M-x compile
4. press TAB: the list of targets is displayed
5. type the first character of an existing target, and press TAB
6. the result is a "[No match]"

This has been working correctly for years (tested with Emacs 24, 25, 26), 
and does not work anymore with Emacs 28.

It seems that at step 5 above the list of completion candidates that is 
considered are subdirectories.

** Bug 2 **

There are too many completion candidates for the list of targets.  For 
example, for the Makefile "foo:\n\techo foo:\n" two candidates are 
displayed: "foo" and "echo".  It seems that 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-*".

Gregory

bug#42383: 28.0.50; Two bugs with M-x compile

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> ** Bug 1 **
>
> Steps to reproduce:
>
> 1. create a Makefile with a few targets
> 2. start emacs -Q
> 3. M-x compile
> 4. press TAB: the list of targets is displayed
> 5. type the first character of an existing target, and press TAB
> 6. the result is a "[No match]"
>
> This has been working correctly for years (tested with Emacs 24, 25, 
> 26), and does not work anymore with Emacs 28.
>
> It seems that at step 5 above the list of completion candidates that is 
> considered are subdirectories.
>

Upon further investigation, this bug has been introduced by the patch for 
bug#34330, which moved pcomplete-completions-at-point last in 
shell-dynamic-complete-functions in shell.el:

(defcustom shell-dynamic-complete-functions
    '(comint-c-a-p-replace-by-expanded-history
      shell-environment-variable-completion
      shell-command-completion
      shell-c-a-p-replace-by-expanded-directory
      shell-filename-completion
      comint-filename-completion
      ;; Put `pcomplete-completions-at-point' last so that other
      ;; functions can run before it does, see bug#34330.
      pcomplete-completions-at-point)

See https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e4d17d8cb479ffeeb7dfb7320a1432722ac8df75 .

The obvious fix is to revert the situation (that is, to move 
pcomplete-completions-at-point again before shell-filename-completion), 
but I'm not sure, because bug#34330 would again exist.  That being said, 
I'm not sure that handling the exceptional case of filenames with '&' 
characters correctly when completing in M-x shell (which is what bug#34330 
is about) justifies to break normal behavior in other situations.

Gregory

bug#42383: 28.0.50; Two bugs with M-x compile

[Eli Zaretskii]

> Date: Thu, 16 Jul 2020 09:48:56 +0000
> From: Gregory Heytings via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> > 1. create a Makefile with a few targets
> > 2. start emacs -Q
> > 3. M-x compile
> > 4. press TAB: the list of targets is displayed
> > 5. type the first character of an existing target, and press TAB
> > 6. the result is a "[No match]"
> >
> > This has been working correctly for years (tested with Emacs 24, 25, 
> > 26), and does not work anymore with Emacs 28.
> >
> > It seems that at step 5 above the list of completion candidates that is 
> > considered are subdirectories.
> >
> 
> Upon further investigation, this bug has been introduced by the patch for 
> bug#34330, which moved pcomplete-completions-at-point last in 
> shell-dynamic-complete-functions in shell.el:
> 
> (defcustom shell-dynamic-complete-functions
>     '(comint-c-a-p-replace-by-expanded-history
>       shell-environment-variable-completion
>       shell-command-completion
>       shell-c-a-p-replace-by-expanded-directory
>       shell-filename-completion
>       comint-filename-completion
>       ;; Put `pcomplete-completions-at-point' last so that other
>       ;; functions can run before it does, see bug#34330.
>       pcomplete-completions-at-point)
> 
> See https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e4d17d8cb479ffeeb7dfb7320a1432722ac8df75 .
> 
> The obvious fix is to revert the situation (that is, to move 
> pcomplete-completions-at-point again before shell-filename-completion), 
> but I'm not sure, because bug#34330 would again exist.  That being said, 
> I'm not sure that handling the exceptional case of filenames with '&' 
> characters correctly when completing in M-x shell (which is what bug#34330 
> is about) justifies to break normal behavior in other situations.

Indeed.

Stefan, Alex: any ideas for how to fix this?  This problem exists on
the emacs-27 branch, so we must find a safe solution for that branch,
and we must do that quickly.  If we don't have any better ideas, I
think we should revert that fix on emacs-27 at least.

Thanks.

bug#42383: 28.0.50; Two bugs with M-x compile

[Stefan Monnier]

>> Upon further investigation, this bug has been introduced by the patch for 
>> bug#34330, which moved pcomplete-completions-at-point last in 
>> shell-dynamic-complete-functions in shell.el:
> Stefan, Alex: any ideas for how to fix this?

I haven't had time to look into this, but I'll just note that the
commit's message says:

    commit e4d17d8cb479ffeeb7dfb7320a1432722ac8df75
    Author: Alex Branham <alex.branham@gmail.com>
    Date:   Thu Aug 15 11:02:38 2019 -0500
    
        Fix filename completion in shell mode buffers
        
        * lisp/shell.el (shell-dynamic-complete-functions): Move
        pcomplete-completions-at-point down the list so that filename
        completion has a chance to complete before pcompletion.
        
        Fixes bug#34330

yet, `pcomplete-completions-at-point` is supposed to perform filename
completion also (via `pcomplete-default-completion-function`), so
I think `pcomplete-completions-at-point` should come first and I suspect
that the problem in bug#34330 is internal to pcomplete and the above
commit just worked around it.


        Stefan

bug#42383: 28.0.50; Two bugs with M-x compile

[Alex Branham]

On Thu 16 Jul 2020 at 14:55, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>         * lisp/shell.el (shell-dynamic-complete-functions): Move
>         pcomplete-completions-at-point down the list so that filename
>         completion has a chance to complete before pcompletion.
>         
>         Fixes bug#34330
>
> yet, `pcomplete-completions-at-point` is supposed to perform filename
> completion also (via `pcomplete-default-completion-function`), so
> I think `pcomplete-completions-at-point` should come first and I suspect
> that the problem in bug#34330 is internal to pcomplete and the above
> commit just worked around it.

I find pcomplete really confusing (why is it that there's a whole second
kind of completion framework outside completion-at-point?). But yes,
sounds like this should be reverted. I didn't notice this regression
since I use ivy which obviates the need to re-complete after typing a
letter.

Someone will need to reopen bugs #10417 and #34330.

Alex

bug#42383: 28.0.50; Two bugs with M-x compile

[Stefan Monnier]

>>         * lisp/shell.el (shell-dynamic-complete-functions): Move
>>         pcomplete-completions-at-point down the list so that filename
>>         completion has a chance to complete before pcompletion.
>>         
>>         Fixes bug#34330
>>
>> yet, `pcomplete-completions-at-point` is supposed to perform filename
>> completion also (via `pcomplete-default-completion-function`), so
>> I think `pcomplete-completions-at-point` should come first and I suspect
>> that the problem in bug#34330 is internal to pcomplete and the above
>> commit just worked around it.
>
> I find pcomplete really confusing (why is it that there's a whole second
> kind of completion framework outside completion-at-point?).

They are not competing nor redundant: pcomplete.el is a system that lets
one explain how to split a line into fields and which completion table
applies to which field (i.e. explain that in a shell command the first
element should be an executable, while the rest will depend on which
executable was used in the first element, ...).

There is sadly some redundancy here in that originally pcomplete.el also
provided a completion UI (under the command `pcomplete`) and this one is
indeed competing with the `completion-at-point` command.
But this part is now obsolete.


        Stefan

bug#42383: 28.0.50; Two bugs with M-x compile

[Eli Zaretskii]

unarchive 10417
reopen 10417
unarchive 34330
reopen 34330
thanks

> From: Alex Branham <alex.branham@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Gregory Heytings <ghe@sdf.org>,
>   42383@debbugs.gnu.org
> Date: Thu, 16 Jul 2020 17:39:57 -0400
> 
> > yet, `pcomplete-completions-at-point` is supposed to perform filename
> > completion also (via `pcomplete-default-completion-function`), so
> > I think `pcomplete-completions-at-point` should come first and I suspect
> > that the problem in bug#34330 is internal to pcomplete and the above
> > commit just worked around it.
> 
> I find pcomplete really confusing (why is it that there's a whole second
> kind of completion framework outside completion-at-point?). But yes,
> sounds like this should be reverted. I didn't notice this regression
> since I use ivy which obviates the need to re-complete after typing a
> letter.
> 
> Someone will need to reopen bugs #10417 and #34330.

OK, thanks.  I've reverted the change and reopened those two bugs.