bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Kévin Le Gouguec
Hello,

Unlike project-shell-command, project-compile first prompts for a
command, then binds default-directory and calls compile.  Binding
default-directory first makes completion work from the project root,
which is useful for completing on filenames relative to the root, on
targets from the toplevel Makefile, etc.

I see three ways to achieve this:

(1) Rewrite project-compile with call-interactively, the way
project-shell-command is written (see patch #1).

(2) Set COMMAND to nil in the interactive spec, then prompt for it after
binding default-directory (see patch #2).

(3) Let-binding default-directory once in the interactive spec, and
again before calling compile.


I'm assuming (1) is out of the question, given 2020-06-02
"* lisp/progmodes/project.el (project-vc-dir, project-shell): New
commands." (2c1e5b9e77).  I'm CC'ing Juri to get his opinion though;
project-compile is new in Emacs 28.1, so its argument list is not yet
set in stone.

I've taken a stab at (2), but my patch changes the semantics of COMMAND
for an edge case: for now calling (project-compile nil) from Lisp causes
(compile nil) to be called (which errors out); with my patch,
(project-compile nil) yields a prompt.

This can be solved using called-interactively-p (or using an optional
INTERACTIVE argument); I just don't know if it's worth the hassle?

I haven't given much thought to (3), so I haven't yet figured out how to
avoid prompting twice for the project (on the rare occasions where
prompting is needed).


Thanks for your time.





In GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-01-04 built on my-little-tumbleweed
Repository revision: 2c847902522ae74c9b25b2a3fa60565e0187fd0a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-xwidgets --with-cairo'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS JSON
PDUMPER LCMS2

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix

0001-Change-default-directory-before-prompting-in-project-1.patch (1K) Download Attachment
0001-Change-default-directory-before-prompting-in-project-2.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Juri Linkov-2
> Unlike project-shell-command, project-compile first prompts for a
> command, then binds default-directory and calls compile.  Binding
> default-directory first makes completion work from the project root,
> which is useful for completing on filenames relative to the root, on
> targets from the toplevel Makefile, etc.
>
> I see three ways to achieve this:
>
> (1) Rewrite project-compile with call-interactively, the way
> project-shell-command is written (see patch #1).

(1) has the obvious advantage of avoiding duplication of the
interactive spec of 'compile' in 'project-compile', in addition
to your use case of binding default-directory.

> I'm assuming (1) is out of the question, given 2020-06-02
> "* lisp/progmodes/project.el (project-vc-dir, project-shell): New
> commands." (2c1e5b9e77).  I'm CC'ing Juri to get his opinion though;
> project-compile is new in Emacs 28.1, so its argument list is not yet
> set in stone.

Actually, (1) is not out of the question.  Quite contrary, (1) is
the cleanest solution.  But before rewriting with call-interactively,
we need to find a way to use project-compile in the init files, i.e.
to find a replacement for such configurations:

  (define-key my-map "m" ;; mnemonics "make"
    (lambda ()
      (interactive)
      (project-compile
       ;; Use previous command from history
       ;; instead of the default from compile-command
       (compilation-read-command (car compile-history))
       ;; Don't use compilation-shell-minor-mode
       nil)))



Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Dmitry Gutov
On 11.01.2021 20:35, Juri Linkov wrote:

> But before rewriting with call-interactively,
> we need to find a way to use project-compile in the init files, i.e.
> to find a replacement for such configurations:
>
>    (define-key my-map "m" ;; mnemonics "make"
>      (lambda ()
>        (interactive)
>        (project-compile
>         ;; Use previous command from history
>         ;; instead of the default from compile-command
>         (compilation-read-command (car compile-history))
>         ;; Don't use compilation-shell-minor-mode
>         nil)))

Is it really possible to solve this without making project-compile a
macro of some sort?

For the above code sample not to exhibit the problem from this report,
it should use project-current and project-root directly.



Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Kévin Le Gouguec
Dmitry Gutov <[hidden email]> writes:

> On 11.01.2021 20:35, Juri Linkov wrote:
>> But before rewriting with call-interactively,
>> we need to find a way to use project-compile in the init files, i.e.
>> to find a replacement for such configurations:
>>    <snip>
>
> Is it really possible to solve this without making project-compile a
> macro of some sort?

Something like this maybe?



(Apologies if I misunderstood what Juri asked, or if my suggestion is
buggy)

We can also stick a "\(fn COMMAND &optional COMINT)" in the docstring if
we don't want to advertise the INTERACTIVE argument.

0001-Change-default-directory-before-prompting-in-project.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Dmitry Gutov
Hi Juri,

On 12.01.2021 20:46, Juri Linkov wrote:

>> Something like this maybe?
>>
>> +(defun project-compile (command &optional comint interactive)
>> +COMMAND and COMINT work as with `compile'.  When calling this
>> +function from Lisp, you can pretend that it was called
>> +interactively by passing a non-nil INTERACTIVE argument."
>> +  (interactive (list nil nil t))
>> +  (let ((default-directory (project-root (project-current t))))
>> +    (if interactive
>> +        (call-interactively #'compile)
>> +      (compile command comint))))
>>
>> (Apologies if I misunderstood what Juri asked, or if my suggestion is
>> buggy)
> Perfect, thank you for finding the middle ground for different needs.

So you're not worried about compilation-read-command in your code being
called in the wrong directory (exhibiting what the current bug report
aims to fix)?

I think you might as well inline the definition, it's almost as short:

(define-key my-map "m" ;; mnemonics "make"
     (lambda ()
       (interactive)
       (let ((default-directory (project-root (project-current t))))
         (compile
          ;; Use previous command from history
          ;; instead of the default from compile-command
          (compilation-read-command (car compile-history))
          ;; Don't use compilation-shell-minor-mode
          nil))))

But if you really like the new version of project-compile, OK.

I'm not a big fan of the 'interactive' argument. It could be replaced by
using (called-interactively-p 'interactive), though I'm not sure how
idiomatic that is.



Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Juri Linkov-2
> So you're not worried about compilation-read-command in your code being
> called in the wrong directory (exhibiting what the current bug report aims
> to fix)?
>
> I think you might as well inline the definition, it's almost as short:
>
> (define-key my-map "m" ;; mnemonics "make"
>     (lambda ()
>       (interactive)
>       (let ((default-directory (project-root (project-current t))))
>         (compile
>          ;; Use previous command from history
>          ;; instead of the default from compile-command
>          (compilation-read-command (car compile-history))
>          ;; Don't use compilation-shell-minor-mode
>          nil))))

Indeed, this means that let-binding default-directory to
(project-root (project-current t))) is unavoidable in the init file.
So it's easier to just replace 'project-compile' with 'compile'.

This means Kévin's first patch is the way to go.



Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Kévin Le Gouguec
In reply to this post by Dmitry Gutov
Dmitry Gutov <[hidden email]> writes:

> I'm not a big fan of the 'interactive' argument. It could be replaced
> by using (called-interactively-p 'interactive), though I'm not sure
> how idiomatic that is.

Not necessarily a fan either, but my takeaway from recent discussions on
emacs-devel is that the argument is preferred to called-interactively-p,
which should only be used when a function's arglist is set in stone.

<[hidden email]>
https://lists.gnu.org/archive/html/emacs-devel/2020-08/msg00463.html

<[hidden email]>
https://lists.gnu.org/archive/html/emacs-devel/2020-08/msg00472.html

I see a few dozen hits for "&optional \([\w-]+ \)*interactive)" under
lisp/, so it seems to be an established practice.

PS: I've just seen Juri's reponse in bug#45765#26; I'm glad there is a
consensus on the first patch, because after looking at more in-tree
examples of optional INTERACTIVE arguments, I found myself agonizing
over spelling the spec (list nil nil t), '(nil nil t), or "i\ni\np".



Reply | Threaded
Open this post in threaded view
|

bug#45765: [PATCH] 28.0.50; Change default-directory before prompting in project-compile

Dmitry Gutov
On 13.01.2021 21:46, Kévin Le Gouguec wrote:

> Dmitry Gutov <[hidden email]> writes:
>
>> I'm not a big fan of the 'interactive' argument. It could be replaced
>> by using (called-interactively-p 'interactive), though I'm not sure
>> how idiomatic that is.
>
> Not necessarily a fan either, but my takeaway from recent discussions on
> emacs-devel is that the argument is preferred to called-interactively-p,
> which should only be used when a function's arglist is set in stone.
>
> <[hidden email]>
> https://lists.gnu.org/archive/html/emacs-devel/2020-08/msg00463.html
>
> <[hidden email]>
> https://lists.gnu.org/archive/html/emacs-devel/2020-08/msg00472.html
>
> I see a few dozen hits for "&optional \([\w-]+ \)*interactive)" under
> lisp/, so it seems to be an established practice.

All right, that makes sense. It will help the next time such question
comes up.

> PS: I've just seen Juri's reponse in bug#45765#26; I'm glad there is a
> consensus on the first patch, because after looking at more in-tree
> examples of optional INTERACTIVE arguments, I found myself agonizing
> over spelling the spec (list nil nil t), '(nil nil t), or "i\ni\np".

I know the feeling ;-)

Patch installed, closing. Thanks again.