bug#64795: 30.0.50; manual-entry [(1) fails

bug#64795: 30.0.50; manual-entry [(1) fails

[Michael Heerdegen]

Hello,

In Gnu/Linux there is a binary named "[" in /usr/bin/, and it has a man
page I can read in a terminal using "man '[(1)'".

Requesting the man page in Emacs using `man' is broken, however:

  M-x manual-entry RET [ TAB

echoes "[No match]" - this is the first problem (the failing
completion).

C-g, let's try differently:

  M-x manual-entry RET TAB [ TAB

(that's the same as before but additionally requesting all candidates
before writing in the minibuffer) suddenly completes to "[(1)" as
expected.  But when confirming with RET the man buffer only has these
contents:

| /bin/sh: 1: Syntax error: "(" unexpected
|
| process exited abnormally with code 2

That's the second problem.

Oh, and M-x woman [ RET works as expected.


TIA,

Michael.


In GNU Emacs 30.0.50 (build 33, x86_64-pc-linux-gnu, cairo version
 1.16.0) of 2023-07-22 built on drachen
Repository revision: 4b3ab8fae12dcb403e653edeafa1d4ce1dab0187
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

bug#64795: 30.0.50; manual-entry [(1) fails

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Sun, 23 Jul 2023 02:58:59 +0200
> 
> In Gnu/Linux there is a binary named "[" in /usr/bin/, and it has a man
> page I can read in a terminal using "man '[(1)'".
> 
> Requesting the man page in Emacs using `man' is broken, however:
> 
>   M-x manual-entry RET [ TAB
> 
> echoes "[No match]" - this is the first problem (the failing
> completion).
> 
> C-g, let's try differently:
> 
>   M-x manual-entry RET TAB [ TAB
> 
> (that's the same as before but additionally requesting all candidates
> before writing in the minibuffer) suddenly completes to "[(1)" as
> expected.  But when confirming with RET the man buffer only has these
> contents:
> 
> | /bin/sh: 1: Syntax error: "(" unexpected
> |
> | process exited abnormally with code 2
> 
> That's the second problem.

Does the patch below give good results?

diff --git a/lisp/man.el b/lisp/man.el
index 479bf9f9a3c..bbcbe5d1525 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -315,7 +315,7 @@ Man-cooked-hook
   :type 'hook
   :group 'man)
 
-(defvar Man-name-regexp "[-[:alnum:]_­+][-[:alnum:]_.:­+]*"
+(defvar Man-name-regexp "[-[:alnum:]_­+[][-[:alnum:]_.:­+]*"
   "Regular expression describing the name of a manpage (without section).")
 
 (defvar Man-section-regexp "[0-9][a-zA-Z0-9+]*\\|[LNln]"
@@ -937,7 +937,7 @@ Man-completion-table
                          "-k" (concat (when (or Man-man-k-use-anchor
                                                 (string-equal prefix ""))
                                         "^")
-                                      prefix))))
+                                      (shell-quote-argument prefix)))))
               (setq table (Man-parse-man-k)))))
 	;; Cache the table for later reuse.
         (when table

bug#64795: 30.0.50; manual-entry [(1) fails

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> Does the patch below give good results?

Thanks.  Yes, it fixes the man [(1) not completing and not displaying
problem.

However, man RET TAB displays [No match] now instead of showing all
completion candidates.


I searched for more problematic characters.  I found also "@" not
working, like in "journald@.conf (5)" or
"systemd-backlight@.service (8)".

I guess we need to add that character as well.  That's the only
one I found when looking through the output of "apropos '.'".


Thx,

Michael.

bug#64795: 30.0.50; manual-entry [(1) fails

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: 64795@debbugs.gnu.org
> Date: Mon, 24 Jul 2023 02:15:28 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Does the patch below give good results?
> 
> Thanks.  Yes, it fixes the man [(1) not completing and not displaying
> problem.
> 
> However, man RET TAB displays [No match] now instead of showing all
> completion candidates.

How about the patch below?

> I searched for more problematic characters.  I found also "@" not
> working, like in "journald@.conf (5)" or
> "systemd-backlight@.service (8)".
> 
> I guess we need to add that character as well.  That's the only
> one I found when looking through the output of "apropos '.'".

Thanks, I added that one as well.

diff --git a/lisp/man.el b/lisp/man.el
index 479bf9f..10ea1c8 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -315,7 +315,7 @@ Man-cooked-hook
   :type 'hook
   :group 'man)
 
-(defvar Man-name-regexp "[-[:alnum:]_­+][-[:alnum:]_.:­+]*"
+(defvar Man-name-regexp "[-[:alnum:]_­+[@][-[:alnum:]_.:­+]*"
   "Regular expression describing the name of a manpage (without section).")
 
 (defvar Man-section-regexp "[0-9][a-zA-Z0-9+]*\\|[LNln]"
@@ -937,7 +937,9 @@ Man-completion-table
                          "-k" (concat (when (or Man-man-k-use-anchor
                                                 (string-equal prefix ""))
                                         "^")
-                                      prefix))))
+                                      (if (string-equal prefix "")
+                                          prefix
+                                        (shell-quote-argument prefix))))))
               (setq table (Man-parse-man-k)))))
 	;; Cache the table for later reuse.
         (when table

bug#64795: 30.0.50; manual-entry [(1) fails

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> How about the patch below?

I think it works well now.

> -(defvar Man-name-regexp "[-[:alnum:]_­+][-[:alnum:]_.:­+]*"
> +(defvar Man-name-regexp "[-[:alnum:]_­+[@][-[:alnum:]_.:­+]*"
>    "Regular expression describing the name of a manpage (without section).")

And this also works well now.
  
>  (defvar Man-section-regexp "[0-9][a-zA-Z0-9+]*\\|[LNln]"
> @@ -937,7 +937,9 @@ Man-completion-table
>                           "-k" (concat (when (or Man-man-k-use-anchor
>                                                  (string-equal prefix ""))
>                                          "^")
> -                                      prefix))))
> +                                      (if (string-equal prefix "")
> +                                          prefix
> +                                        (shell-quote-argument prefix))))))

I wonder however: why did you add that `shell-quote-argument' wrapper?
I think there is no shell involved.  It doesn't seem to have an effect
here, but I'm not sure if there are not more problems possible like the
one involving the empty string.

Or do you expect that some people might set `manual-program' to
something involving a shell?


Thx,

Michael.

bug#64795: 30.0.50; manual-entry [(1) fails

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: 64795@debbugs.gnu.org
> Date: Tue, 25 Jul 2023 03:05:34 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > @@ -937,7 +937,9 @@ Man-completion-table
> >                           "-k" (concat (when (or Man-man-k-use-anchor
> >                                                  (string-equal prefix ""))
> >                                          "^")
> > -                                      prefix))))
> > +                                      (if (string-equal prefix "")
> > +                                          prefix
> > +                                        (shell-quote-argument prefix))))))
> 
> I wonder however: why did you add that `shell-quote-argument' wrapper?
> I think there is no shell involved.  It doesn't seem to have an effect
> here, but I'm not sure if there are not more problems possible like the
> one involving the empty string.
> 
> Or do you expect that some people might set `manual-program' to
> something involving a shell?

I used shell-quote-argument because it did the job, and was simple.

What would be the alternative?  We could perhaps use regexp-quote, if
we believe the argument to "man -k" is interpreted as an Emacs regexp,
but the only reference I have just says "regular expression", and
"M-x man" says in the doc string it's an ERE, and I don't believe
regexp-quote can handle those reliably.  Developing a new function
that would quote an ERE was not my intent, but if you or someone else
wants to do that, please do.  (Btw, is there only one "man -k" variant
out there? if not, perhaps others don't accept regexps at all...)

Stefan, any suggestions?

bug#64795: 30.0.50; manual-entry [(1) fails

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > I wonder however: why did you add that `shell-quote-argument' wrapper?
> > I think there is no shell involved. [...]
>
> I used shell-quote-argument because it did the job, and was simple.
> What would be the alternative? [...]

Let's go with your patch.  I'm sorry - I was just confused and didn't
understand why the quoting was necessary.  I was missing that the string
will me interpreted as a regexp (although it's obvious...).


Michael.

bug#64795: 30.0.50; manual-entry [(1) fails

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: 64795@debbugs.gnu.org,  Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 26 Jul 2023 05:20:30 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > > I wonder however: why did you add that `shell-quote-argument' wrapper?
> > > I think there is no shell involved. [...]
> >
> > I used shell-quote-argument because it did the job, and was simple.
> > What would be the alternative? [...]
> 
> Let's go with your patch.  I'm sorry - I was just confused and didn't
> understand why the quoting was necessary.  I was missing that the string
> will me interpreted as a regexp (although it's obvious...).

Thanks, installed on master, and closing the bug.  I added a comment
there regarding the use of shell-quote-argument, for posterity.