emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: master 861ac933dd8: Run `man' also on remote systems


From: Eshel Yaron
Subject: Re: master 861ac933dd8: Run `man' also on remote systems
Date: Wed, 01 Nov 2023 19:10:55 +0100
User-agent: Gnus/5.13 (Gnus v5.13)

Hello Michael,

Michael Albinus <Michael.Albinus@gmx.de> writes:

> branch: master
> commit 861ac933dd8aed1028edc4b9142400e3702874d5
> Author: Michael Albinus <michael.albinus@gmx.de>
> Commit: Michael Albinus <michael.albinus@gmx.de>
>
>     Run `man' also on remote systems

Thanks for implementing this, I just found myself wishing `M-x man`
would run on the remote host a few days ago.  This works well now!

I have a couple of suggestions around the new documentation:

> +*** New user option 'Man-support-remote-systems'.
> +If the user option 'Man-support-remote-systems' is non-nil, and
> +'default-directory' indicates a remote system, the man page is taken
> +from the remote system.  Calling the 'man' command with a prefix like
> +'C-u M-x man' reverts the value of 'Man-support-remote-systems' for
> +that call.

I think specifying the name of the option again is redundant, and
mentioning `default-directory` gets a bit too much into the
implementation details.  So I'd say instead:

    This option controls whether 'M-x man' executes the "man" command on
    a remote system when the current buffer is remote.  You can invoke
    the 'man' command with a prefix argument to reverse the value of
    this option only for the current invocation.
    
> +(defun Man-header-file-path ()
> +  "C Header file search path used in Man.
> +In the local case, it is the value of `Man-header-file-path'.
> +Otherwise, it will be checked on the remote system."

Here I'd suggest:

    Return the C header file search path that Man uses.
    Normally, this is the value of the user option
    `Man-header-file-path', but when executing "man" on a remote system
    this function tries to find the C header path on that system.
    
> +If `default-directory' is remote, and `Man-support-remote-systems'
> +is non-nil, the man page will be formatted on the corresponding
> +remote system.
> +
> +If `man' is called interactively with a prefix argument, the
> +value of `Man-support-remote-systems' is reverted."

Here it would be slightly clearer to use the active voice IMO, namely:

    If `default-directory' is remote, and `Man-support-remote-systems'
    is non-nil, this command formats the man page on the remote system.
    A prefix argument reverses the value of `Man-support-remote-systems'
    for the current call.

I'm would also consider changing the name of the new user option from
`Man-support-remote-systems' to `Man-execute-on-remote-systems', since
to me that better conveys the choice you make by setting this option.


Best,

Eshel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]