bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

In lisp/man.el, completion of man pages and sections depends on parsing
the output of `man -k'.  As a bonus, this method provides short
descriptions in the form of tooltips in the *Completions* buffer.

However, the code only handles the `man -k' output format peculiar to
the man-db package, which, even on GNU/Linux, is not the only one used
for this purpose.

For example, Slackware seems to use man-1.6g, whose `man -k' output can
have an additional reference in square brackets between the page name
and the section, so that man topic completion does work in this case,
but neither sections nor descriptions are parsed.

The situation is even worse for FreeBSD, where multiple comma-separated
page names can be shown on the same line (which is arguably against the
spirit of POSIX man(1p), but that's not quite clear and doesn't help
much).

The last major discussion of man completion issues seems to be bug#3717,
through which I skimmed, but there doesn't seem to be anything related
to this kind of portability.  There is, however, some prior code in the
`man-completion-cache' function in

ftp://download.tuxfamily.org/user42/man-completion.el

to deal with this problem.  In particular, it handles the example

boot-scripts (7) [boot] - General description of boot sequence

where the present patch would miss the section.  However, I don't know
of a man program which actually produces this format (man-1.6g has the
reference in square brackets /preceding/ the section).  Otherwise, the
following patch, which I'd like to install, should handle more formats.

It mainly adds a single separate function for parsing various formats.
There's also an ert test with examples for what I described above and
other formats found on the web.

Wolfgang


=== modified file 'lisp/man.el'
--- lisp/man.el	2012-08-15 16:29:11 +0000
+++ lisp/man.el	2012-12-12 16:00:11 +0000
@@ -780,6 +780,57 @@
   ;; but apparently that's not the case in all cases, so let's add a cache.
   "Cache of completion table of the form (PREFIX . TABLE).")
 
+(defvar Man-keyword-completion-twiddle
+  (cond
+   ;; man-db-* or man-*.
+   ((memq system-type '(gnu gnu/linux gnu/kfreebsd))
+    "^")
+   (t
+    ""))
+  "A string prepended to the keyword passed to `man -k' for completion.
+This should usually be an extended regular expression.
+
+Actually, unwanted page names are weeded out anyway, so with an
+empty string here we will always get correct results, but things
+may take a bit longer.")
+
+(defun Man-parse-apropos (&optional table)
+  "Parse `man -k' output, prepend page names to TABLE and return that.
+
+The current buffer should contain the output of a command of the
+form `man -k keyword', which is traditionally also available with
+apropos(1).
+
+While POSIX man(1p) is a bit vague about what to expect here,
+this function tries to parse some commonly used formats, which
+can be described in the following informal way, with square brackets
+indicating optional parts and whitespace being interpreted
+somewhat loosely.
+
+foo[, bar [, ...]] [other stuff] (sec) - description
+foo(sec)[, bar(sec) [, ...]] [other stuff] - description
+
+For more details and some regression tests, please see
+test/automated/man-tests.el in the emacs bzr repository."
+  (goto-char (point-min))
+  (while (search-forward-regexp "^\\([^ \t,\n]+\\)\\(.*?\\)\
+\\(?:[ \t]\\(([^ \t,\n]+?)\\)\\)?\\(?:[ \t]+- ?\\(.*\\)\\)?$" nil t)
+    (let ((section (match-string 3))
+	  (description (match-string 4))
+	  (bound (match-end 2))
+	  (continue (goto-char (match-end 1))))
+      (while continue
+	;; The first regexp grouping may already match the section
+	;; tacked on to the name, which is ok since for the formats we
+	;; claim to support the third (non-shy) grouping does not
+	;; match in this case, i.e., section is nil.
+	(push (propertize (concat (match-string 1) section)
+			  'help-echo description)
+	      table)
+	(setq continue (search-forward-regexp "\\=, *\\([^ \t,]+\\)"
+					      bound t)))))
+  table)
+
 (defun Man-completion-table (string pred action)
   (cond
    ;; This ends up returning t for pretty much any string, and hence leads to
@@ -813,12 +864,9 @@
             ;; error later.
             (ignore-errors
               (call-process manual-program nil '(t nil) nil
-                            "-k" (concat "^" prefix))))
-          (goto-char (point-min))
-          (while (re-search-forward "^\\([^ \t\n]+\\)\\(?: ?\\((.+?)\\)\\(?:[ \t]+- \\(.*\\)\\)?\\)?" nil t)
-            (push (propertize (concat (match-string 1) (match-string 2))
-                              'help-echo (match-string 3))
-                  table)))
+			    "-k" (concat Man-keyword-completion-twiddle
+					 prefix))))
+	  (setq table (Man-parse-apropos table)))
         ;; Cache the table for later reuse.
         (setq Man-completion-cache (cons prefix table)))
       ;; The table may contain false positives since the match is made

=== added file 'test/automated/man-tests.el'
--- test/automated/man-tests.el	1970-01-01 00:00:00 +0000
+++ test/automated/man-tests.el	2012-12-08 17:10:24 +0000
@@ -0,0 +1,118 @@
+;;; man-tests.el --- Test suite for man.
+
+;; Copyright (C) 2012  Free Software Foundation, Inc.
+
+;; Author: Wolfgang Jenkner <wjenkner@inode.at>
+;; Keywords: help, internal, unix
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'man)
+
+(defconst man-tests-parse-apropos-tests
+  '(;; GNU/Linux: man-db-2.6.1
+    ("\
+sin (3)              - sine function
+sinf (3)             - sine function
+sinl (3)             - sine function"
+     . (#("sinl(3)" 0 7 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sin(3)" 0 6 (help-echo "sine function"))))
+    ;; GNU/Linux: man-1.6g
+    ("\
+sin                  (3)  - sine function
+sinf [sin]           (3)  - sine function
+sinl [sin]           (3)  - sine function"
+     . (#("sinl(3)" 0 7 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sin(3)" 0 6 (help-echo "sine function"))))
+    ;; FreeBSD 9
+    ("\
+sin(3), sinf(3), sinl(3) - sine functions"
+     . (#("sinl(3)" 0 7 (help-echo "sine functions")) #("sinf(3)" 0 7 (help-echo "sine functions")) #("sin(3)" 0 6 (help-echo "sine functions"))))
+    ;; SunOS, Solaris
+    ;; http://docs.oracle.com/cd/E19455-01/805-6331/usradm-7/index.html
+    ;; SunOS 4
+    ("\
+tset, reset (1)    - establish or restore terminal characteristics"
+     . (#("reset(1)" 0 8 (help-echo "establish or restore terminal characteristics")) #("tset(1)" 0 7 (help-echo "establish or restore terminal characteristics"))))
+    ;; SunOS 5.7, Solaris
+    ("\
+reset  tset (1b)   - establish or restore terminal characteristics
+tset   tset (1b)   - establish or restore terminal characteristics"
+     . (#("tset(1b)" 0 7 (help-echo "establish or restore terminal characteristics")) #("reset(1b)" 0 8 (help-echo "establish or restore terminal characteristics"))))
+    ;; Minix 3
+    ;; http://www.minix3.org/manpages/html5/whatis.html
+    ("\
+cawf, nroff (1) - C version of the nroff-like, Amazingly Workable (text) Formatter
+whatis (5) - database of online manual pages"
+     . (#("whatis(5)" 0 9 (help-echo "database of online manual pages")) #("nroff(1)" 0 8 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter")) #("cawf(1)" 0 7 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter"))))
+    ;; HP-UX
+    ;; http://docstore.mik.ua/manuals/hp-ux/en/B2355-60130/man.1.html
+    ;; Assuming that the line break in the zgrep description was
+    ;; introduced by the man page formatting.
+    ("\
+grep, egrep, fgrep (1) - search a file for a pattern
+zgrep(1) - search possibly compressed files for a regular expression"
+     . (#("zgrep(1)" 0 8 (help-echo "search possibly compressed files for a regular expression")) #("fgrep(1)" 0 8 (help-echo "search a file for a pattern")) #("egrep(1)" 0 8 (help-echo "search a file for a pattern")) #("grep(1)" 0 7 (help-echo "search a file for a pattern"))))
+    ;; AIX
+    ;; http://pic.dhe.ibm.com/infocenter/aix/v7r1/topic/com.ibm.aix.cmds/doc/aixcmds6/whatis.htm
+    ("\
+ls(1)  -Displays the contents of a directory."
+    . (#("ls(1)" 0 5 (help-echo "Displays the contents of a directory."))))
+    ;; https://www.ibm.com/developerworks/mydeveloperworks/blogs/cgaix/entry/catman_0703_102_usr_lbin_mkwhatis_the_error_number_is_1?lang=en
+    ("\
+loopmount(1)    - Associate an image file to a loopback device."
+     . (#("loopmount(1)" 0 12 (help-echo "Associate an image file to a loopback device."))))
+    )
+  "List of tests for `Man-parse-apropos'.
+Each element is a cons cell whose car is a string containing
+man -k output.  That should result in the table which is stored
+in the cdr of the element.")
+
+(defun man-tests-name-equal-p (name description string)
+  (and (equal name string)
+       (not (next-single-property-change 0 'help-echo string))
+       (equal (get-text-property 0 'help-echo string) description)))
+
+(defun man-tests-parse-apropos-test-case (test)
+  (let ((temp-buffer (get-buffer-create " *test-man*"))
+	(apropos-output (car test)))
+    (unwind-protect
+	(save-window-excursion
+	  (with-current-buffer temp-buffer
+	    (erase-buffer)
+	    (insert apropos-output)
+	    (let ((result (Man-parse-apropos))
+		  (checklist (cdr test)))
+	      (while (and checklist result
+			  (man-tests-name-equal-p
+			   (car checklist)
+			   (get-text-property 0 'help-echo
+					      (car checklist))
+			   (pop result)))
+		(pop checklist))
+	      (and (null checklist) (null result)))))
+      (and (buffer-name temp-buffer)
+	   (kill-buffer temp-buffer)))))
+
+(ert-deftest man-tests ()
+  "Test man."
+  (dolist (test man-tests-parse-apropos-tests)
+    (should (man-tests-parse-apropos-test-case test))))
+
+(provide 'man-tests)
+
+;;; man-tests.el ends here

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Glenn Morris]

Wolfgang Jenkner wrote:

> In lisp/man.el, completion of man pages and sections depends on parsing
> the output of `man -k'.  As a bonus, this method provides short
> descriptions in the form of tooltips in the *Completions* buffer.

IMO it should just search MANPATH, like bash_completion does.
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10712#11

This is perhaps a separate issue, but would avoid all of this:

> However, the code only handles the `man -k' output format peculiar to
> the man-db package, which, even on GNU/Linux, is not the only one used
> for this purpose.
>
> For example, Slackware seems to use man-1.6g, whose `man -k' output can
> have an additional reference in square brackets between the page name
> and the section, so that man topic completion does work in this case,
> but neither sections nor descriptions are parsed.
>
> The situation is even worse for FreeBSD, where multiple comma-separated
> page names can be shown on the same line (which is arguably against the
> spirit of POSIX man(1p), but that's not quite clear and doesn't help
> much).

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Stefan Monnier]

> IMO it should just search MANPATH, like bash_completion does.
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10712#11

That just gives you a different set of problems (basically that
different `man' construct their MANPATH from different places).


        Stefan

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Kevin Ryde]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
> different `man' construct their MANPATH from different places).

Yes.  There's a bit in woman.el to attempt it and even
/etc/manpath.conf.  But that misses aliases when the page lists two
names in its NAME part.  In debian there's normally symlinks for such,
but it's possible they're only in the man index extracted by "lexgrog".

I reckoned it better to ask "man" what pages it knows, if it can give
its index in a reasonable way.  The "whatis" program might be an
alternative, or it might be the same.


Wolfgang Jenkner <wjenkner@inode.at> writes:
>
> boot-scripts (7) [boot] - General description of boot sequence

Some version of man-db printed that for me, but the aliases or the
output have changed now.

> +(defvar Man-keyword-completion-twiddle
> +  (cond
> +   ;; man-db-* or man-*.
> +   ((memq system-type '(gnu gnu/linux gnu/kfreebsd))
> +    "^")
> +   (t
> +    ""))

You'd much prefer to avoid that.  For a start `system-type' is probably
unreliable to distinguish the flavour of man.  The outputs look close
enough / different enough for a single parse/split/etc.

> +tset, reset (1)    - establish or restore terminal characteristics"

Thanks, I'll see if I can recognise some of those for my stuff too.

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Fri, Dec 14 2012, Kevin Ryde wrote:

>> +(defvar Man-keyword-completion-twiddle
>> +  (cond
>> +   ;; man-db-* or man-*.
>> +   ((memq system-type '(gnu gnu/linux gnu/kfreebsd))
>> +    "^")
>> +   (t
>> +    ""))
>
> You'd much prefer to avoid that.  For a start `system-type' is probably
> unreliable to distinguish the flavour of man.  The outputs look close
> enough / different enough for a single parse/split/etc.

It's not used for parsing, it's only prepended to the search string
passed to `man -k'.  And I'd happily ditch it since, as the doc string
indicates, it's not really necessary, but I'm not sure about performance
implications.

Thanks for the comments,

Wolfgang

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Kevin Ryde]

Wolfgang Jenkner <wjenkner@inode.at> writes:
>
> It's not used for parsing, it's only prepended to the search string
> passed to `man -k'.

Ah.  I even had a note in my bit about that.  Yes.  Daniel Pittman I
think it was reported that on macos man -k '' empty string gives no
output, hence "^".

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Sat, Dec 15 2012, Kevin Ryde wrote:

> Wolfgang Jenkner <wjenkner@inode.at> writes:
>>
>> It's not used for parsing, it's only prepended to the search string
>> passed to `man -k'.
>
> Ah.  I even had a note in my bit about that.  Yes.  Daniel Pittman I
> think it was reported that on macos man -k '' empty string gives no
> output, hence "^".

Right, the empty string deserves special attention, also because it has
the potential to mess up the cache.  Thanks for reminding me of this.

So, here's an updated patch (there's no change in the parsing code
proper).

Wolfgang

2012-12-16  Wolfgang Jenkner  <wjenkner@inode.at>

	Support man page completion for more man programs.
	* man.el (Man-parse-apropos): New function.
	(Man-keyword-completion-anchor): New variable.
	(Man-completion-table): Use them to replace man-db specific code
	for parsing `man -k' output.
	Filter the list returned by Man-parse-apropos before caching it in
	the cdr of Man-completion-cache.
	Also, don't add to the cache indefinitely, just replace it with
	the freshly parsed table.
	Handle completion of the empty string carefully.

2012-12-16  Wolfgang Jenkner  <wjenkner@inode.at>

	* automated/man-tests.el: New file.

=== modified file 'lisp/man.el'
--- lisp/man.el	2012-08-15 16:29:11 +0000
+++ lisp/man.el	2012-12-16 03:22:20 +0000
@@ -780,6 +780,59 @@
   ;; but apparently that's not the case in all cases, so let's add a cache.
   "Cache of completion table of the form (PREFIX . TABLE).")
 
+(defvar Man-keyword-completion-anchor
+  (cond
+   ;; man-db-* or man-*.
+   ((memq system-type '(gnu gnu/linux gnu/kfreebsd))
+    "^")
+   (t
+    ""))
+  "A string prepended to the keyword passed to `man -k' for completion.
+This should usually be an extended regular expression (but it
+need not be an anchor in the ERE sense).
+
+Actually, unwanted page names are weeded out anyway, so with an
+empty string here we will always get correct results, but things
+may take a bit longer.")
+
+(defun Man-parse-apropos ()
+  "Parse `man -k' output and return the list of page names.
+
+The current buffer should contain the output of a command of the
+form `man -k keyword', which is traditionally also available with
+apropos(1).
+
+While POSIX man(1p) is a bit vague about what to expect here,
+this function tries to parse some commonly used formats, which
+can be described in the following informal way, with square brackets
+indicating optional parts and whitespace being interpreted
+somewhat loosely.
+
+foo[, bar [, ...]] [other stuff] (sec) - description
+foo(sec)[, bar(sec) [, ...]] [other stuff] - description
+
+For more details and some regression tests, please see
+test/automated/man-tests.el in the emacs bzr repository."
+  (goto-char (point-min))
+  (let (table)
+    (while (search-forward-regexp "^\\([^ \t,\n]+\\)\\(.*?\\)\
+\\(?:[ \t]\\(([^ \t,\n]+?)\\)\\)?\\(?:[ \t]+- ?\\(.*\\)\\)?$" nil t)
+      (let ((section (match-string 3))
+	    (description (match-string 4))
+	    (bound (match-end 2))
+	    (continue (goto-char (match-end 1))))
+	(while continue
+	  ;; The first regexp grouping may already match the section
+	  ;; tacked on to the name, which is ok since for the formats we
+	  ;; claim to support the third (non-shy) grouping does not
+	  ;; match in this case, i.e., section is nil.
+	  (push (propertize (concat (match-string 1) section)
+			    'help-echo description)
+		table)
+	  (setq continue (search-forward-regexp "\\=, *\\([^ \t,]+\\)"
+						bound t)))))
+    (nreverse table)))
+
 (defun Man-completion-table (string pred action)
   (cond
    ;; This ends up returning t for pretty much any string, and hence leads to
@@ -811,16 +864,26 @@
             ;; run differently in Man-getpage-in-background, an error
             ;; here may not necessarily mean that we'll also get an
             ;; error later.
-            (ignore-errors
-              (call-process manual-program nil '(t nil) nil
-                            "-k" (concat "^" prefix))))
-          (goto-char (point-min))
-          (while (re-search-forward "^\\([^ \t\n]+\\)\\(?: ?\\((.+?)\\)\\(?:[ \t]+- \\(.*\\)\\)?\\)?" nil t)
-            (push (propertize (concat (match-string 1) (match-string 2))
-                              'help-echo (match-string 3))
-                  table)))
-        ;; Cache the table for later reuse.
-        (setq Man-completion-cache (cons prefix table)))
+	    (ignore-errors
+	      (call-process manual-program nil '(t nil) nil
+			    "-k" (if (string-equal prefix "")
+				     "^"
+				   (concat Man-keyword-completion-anchor
+					   prefix)))))
+	  (setq table (Man-parse-apropos)))
+	;; Weed out unwanted page names.
+	(dolist (comp (prog1 table (setq table nil)))
+	  (if (string-prefix-p prefix comp)
+	      (push comp table)))
+	;; Cache the table for later reuse.
+	(if (or (not (string-equal prefix ""))
+		;; Believe that we have valid completions of the empty
+		;; string, if there are at least three of them.
+		;; Otherwise, `table' is likely the result of parsing
+		;; an error message.
+		(cddr table))
+	    (setq Man-completion-cache (cons prefix table))
+	  (setq table nil)))
       ;; The table may contain false positives since the match is made
       ;; by "man -k" not just on the manpage's name.
       (if section

=== added file 'test/automated/man-tests.el'
--- test/automated/man-tests.el	1970-01-01 00:00:00 +0000
+++ test/automated/man-tests.el	2012-12-16 03:31:53 +0000
@@ -0,0 +1,118 @@
+;;; man-tests.el --- Test suite for man.
+
+;; Copyright (C) 2012  Free Software Foundation, Inc.
+
+;; Author: Wolfgang Jenkner <wjenkner@inode.at>
+;; Keywords: help, internal, unix
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'man)
+
+(defconst man-tests-parse-apropos-tests
+  '(;; GNU/Linux: man-db-2.6.1
+    ("\
+sin (3)              - sine function
+sinf (3)             - sine function
+sinl (3)             - sine function"
+     . (#("sin(3)" 0 6 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sinl(3)" 0 7 (help-echo "sine function"))))
+    ;; GNU/Linux: man-1.6g
+    ("\
+sin                  (3)  - sine function
+sinf [sin]           (3)  - sine function
+sinl [sin]           (3)  - sine function"
+     . (#("sin(3)" 0 6 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sinl(3)" 0 7 (help-echo "sine function"))))
+    ;; FreeBSD 9
+    ("\
+sin(3), sinf(3), sinl(3) - sine functions"
+     . (#("sin(3)" 0 6 (help-echo "sine functions")) #("sinf(3)" 0 7 (help-echo "sine functions")) #("sinl(3)" 0 7 (help-echo "sine functions"))))
+    ;; SunOS, Solaris
+    ;; http://docs.oracle.com/cd/E19455-01/805-6331/usradm-7/index.html
+    ;; SunOS 4
+    ("\
+tset, reset (1)    - establish or restore terminal characteristics"
+     . (#("tset(1)" 0 7 (help-echo "establish or restore terminal characteristics")) #("reset(1)" 0 8 (help-echo "establish or restore terminal characteristics"))))
+    ;; SunOS 5.7, Solaris
+    ("\
+reset  tset (1b)   - establish or restore terminal characteristics
+tset   tset (1b)   - establish or restore terminal characteristics"
+     . (#("reset(1b)" 0 8 (help-echo "establish or restore terminal characteristics")) #("tset(1b)" 0 7 (help-echo "establish or restore terminal characteristics"))))
+    ;; Minix 3
+    ;; http://www.minix3.org/manpages/html5/whatis.html
+    ("\
+cawf, nroff (1) - C version of the nroff-like, Amazingly Workable (text) Formatter
+whatis (5) - database of online manual pages"
+     . (#("cawf(1)" 0 7 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter")) #("nroff(1)" 0 8 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter")) #("whatis(5)" 0 9 (help-echo "database of online manual pages"))))
+    ;; HP-UX
+    ;; http://docstore.mik.ua/manuals/hp-ux/en/B2355-60130/man.1.html
+    ;; Assuming that the line break in the zgrep description was
+    ;; introduced by the man page formatting.
+    ("\
+grep, egrep, fgrep (1) - search a file for a pattern
+zgrep(1) - search possibly compressed files for a regular expression"
+     . (#("grep(1)" 0 7 (help-echo "search a file for a pattern")) #("egrep(1)" 0 8 (help-echo "search a file for a pattern")) #("fgrep(1)" 0 8 (help-echo "search a file for a pattern")) #("zgrep(1)" 0 8 (help-echo "search possibly compressed files for a regular expression"))))
+    ;; AIX
+    ;; http://pic.dhe.ibm.com/infocenter/aix/v7r1/topic/com.ibm.aix.cmds/doc/aixcmds6/whatis.htm
+    ("\
+ls(1)  -Displays the contents of a directory."
+    . (#("ls(1)" 0 5 (help-echo "Displays the contents of a directory."))))
+    ;; https://www.ibm.com/developerworks/mydeveloperworks/blogs/cgaix/entry/catman_0703_102_usr_lbin_mkwhatis_the_error_number_is_1?lang=en
+    ("\
+loopmount(1)    - Associate an image file to a loopback device."
+     . (#("loopmount(1)" 0 12 (help-echo "Associate an image file to a loopback device."))))
+    )
+  "List of tests for `Man-parse-apropos'.
+Each element is a cons cell whose car is a string containing
+man -k output.  That should result in the table which is stored
+in the cdr of the element.")
+
+(defun man-tests-name-equal-p (name description string)
+  (and (equal name string)
+       (not (next-single-property-change 0 'help-echo string))
+       (equal (get-text-property 0 'help-echo string) description)))
+
+(defun man-tests-parse-apropos-test-case (test)
+  (let ((temp-buffer (get-buffer-create " *test-man*"))
+	(apropos-output (car test)))
+    (unwind-protect
+	(save-window-excursion
+	  (with-current-buffer temp-buffer
+	    (erase-buffer)
+	    (insert apropos-output)
+	    (let ((result (Man-parse-apropos))
+		  (checklist (cdr test)))
+	      (while (and checklist result
+			  (man-tests-name-equal-p
+			   (car checklist)
+			   (get-text-property 0 'help-echo
+					      (car checklist))
+			   (pop result)))
+		(pop checklist))
+	      (and (null checklist) (null result)))))
+      (and (buffer-name temp-buffer)
+	   (kill-buffer temp-buffer)))))
+
+(ert-deftest man-tests ()
+  "Test man."
+  (dolist (test man-tests-parse-apropos-tests)
+    (should (man-tests-parse-apropos-test-case test))))
+
+(provide 'man-tests)
+
+;;; man-tests.el ends here

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Sun, Dec 16 2012, Wolfgang Jenkner wrote:

> On Sat, Dec 15 2012, Kevin Ryde wrote:
>
>> Wolfgang Jenkner <wjenkner@inode.at> writes:
>>>
>>> It's not used for parsing, it's only prepended to the search string
>>> passed to `man -k'.
>>
>> Ah.  I even had a note in my bit about that.  Yes.  Daniel Pittman I
>> think it was reported that on macos man -k '' empty string gives no
>> output, hence "^".
>
> Right, the empty string deserves special attention, also because it has
> the potential to mess up the cache.  Thanks for reminding me of this.

Actually, I think the cache is really the clue here: prepending
completions piecemeal to the cache makes only sense if the user
consciously avoids completing the empty string because of memory
constraints... and even then the cache would have to be a trie or
something to be useful.

So let's just cache all entries at the first completion attempt. This
simplifies things as it seems reasonable to assume that `man -k ^'
outputs all entries (I was unsure about this because of bug#10840, but
that was likely a red herring).

And since the present bug report has been requalified as being of
`wishlist' severity I'd like to commit the patch before christmas :-)

Wolfgang

2012-12-22  Wolfgang Jenkner  <wjenkner@inode.at>

	* man.el (Man-completion-table): Cache aggressively.
	Fill the cache all at once, at the first completion.
	Previously, when a prefix didn't start with the string contained
	in the car of Man-completion-cache the parsed output of `man -k'
	was prepended to the cache.
	Conveniently, the change entails that we don't have to guess the
	output format of `man -k' in order to pass it a useful regexp.

2012-12-22  Wolfgang Jenkner  <wjenkner@inode.at>

	Support man page completion for more man programs.  (Bug#13160)
	* man.el (Man-parse-apropos): New function.
	(Man-completion-table): Use it in parsing `man -k' output to
	replace code peculiar to man-db.
	Thanks to Kevin Ryde for commenting on a previous version of this.

2012-12-22  Wolfgang Jenkner  <wjenkner@inode.at>

	* automated/man-tests.el: New file.

=== modified file 'lisp/man.el'
--- lisp/man.el	2012-08-15 16:29:11 +0000
+++ lisp/man.el	2012-12-21 22:51:54 +0000
@@ -796,15 +796,11 @@
       (when (string-match "\\`\\([[:digit:]].*?\\) " string)
         (setq section (match-string 1 string))
         (setq prefix (substring string (match-end 0))))
-      (unless (and Man-completion-cache
-                   (string-prefix-p (car Man-completion-cache) prefix))
+      (unless Man-completion-cache
         (with-temp-buffer
           (setq default-directory "/") ;; in case inherited doesn't exist
-          ;; Actually for my `man' the arg is a regexp.
-          ;; POSIX says it must be ERE and "man-db" seems to agree,
-          ;; whereas under MacOSX it seems to be BRE-style and doesn't
-          ;; accept backslashes at all.  Let's not bother to
-          ;; quote anything.
+	  ;; Actually for `man -k' the arg is a regexp and POSIX says
+	  ;; it must be an ERE.  Anyway, only the ^ anchor is needed here.
           (let ((process-environment (copy-sequence process-environment)))
             (setenv "COLUMNS" "999") ;; don't truncate long names
             ;; manual-program might not even exist.  And since it's
@@ -812,15 +808,15 @@
             ;; here may not necessarily mean that we'll also get an
             ;; error later.
             (ignore-errors
-              (call-process manual-program nil '(t nil) nil
-                            "-k" (concat "^" prefix))))
+	      ;; Get all entries.
+              (call-process manual-program nil '(t nil) nil "-k" "^")))
           (goto-char (point-min))
           (while (re-search-forward "^\\([^ \t\n]+\\)\\(?: ?\\((.+?)\\)\\(?:[ \t]+- \\(.*\\)\\)?\\)?" nil t)
             (push (propertize (concat (match-string 1) (match-string 2))
                               'help-echo (match-string 3))
                   table)))
         ;; Cache the table for later reuse.
-        (setq Man-completion-cache (cons prefix table)))
+        (setq Man-completion-cache (cons "" (nreverse table))))
       ;; The table may contain false positives since the match is made
       ;; by "man -k" not just on the manpage's name.
       (if section


=== modified file 'lisp/man.el'
--- lisp/man.el	2012-12-21 22:51:54 +0000
+++ lisp/man.el	2012-12-21 22:55:19 +0000
@@ -780,6 +780,44 @@
   ;; but apparently that's not the case in all cases, so let's add a cache.
   "Cache of completion table of the form (PREFIX . TABLE).")
 
+(defun Man-parse-apropos ()
+  "Parse `man -k' output and return the list of page names.
+
+The current buffer should contain the output of a command of the
+form `man -k keyword', which is traditionally also available with
+apropos(1).
+
+While POSIX man(1p) is a bit vague about what to expect here,
+this function tries to parse some commonly used formats, which
+can be described in the following informal way, with square brackets
+indicating optional parts and whitespace being interpreted
+somewhat loosely.
+
+foo[, bar [, ...]] [other stuff] (sec) - description
+foo(sec)[, bar(sec) [, ...]] [other stuff] - description
+
+For more details and some regression tests, please see
+test/automated/man-tests.el in the emacs bzr repository."
+  (goto-char (point-min))
+  (let (table)
+    (while (search-forward-regexp "^\\([^ \t,\n]+\\)\\(.*?\\)\
+\\(?:[ \t]\\(([^ \t,\n]+?)\\)\\)?\\(?:[ \t]+- ?\\(.*\\)\\)?$" nil t)
+      (let ((section (match-string 3))
+	    (description (match-string 4))
+	    (bound (match-end 2))
+	    (continue (goto-char (match-end 1))))
+	(while continue
+	  ;; The first regexp grouping may already match the section
+	  ;; tacked on to the name, which is ok since for the formats we
+	  ;; claim to support the third (non-shy) grouping does not
+	  ;; match in this case, i.e., section is nil.
+	  (push (propertize (concat (match-string 1) section)
+			    'help-echo description)
+		table)
+	  (setq continue (search-forward-regexp "\\=, *\\([^ \t,]+\\)"
+						bound t)))))
+    (nreverse table)))
+
 (defun Man-completion-table (string pred action)
   (cond
    ;; This ends up returning t for pretty much any string, and hence leads to
@@ -810,13 +848,9 @@
             (ignore-errors
 	      ;; Get all entries.
               (call-process manual-program nil '(t nil) nil "-k" "^")))
-          (goto-char (point-min))
-          (while (re-search-forward "^\\([^ \t\n]+\\)\\(?: ?\\((.+?)\\)\\(?:[ \t]+- \\(.*\\)\\)?\\)?" nil t)
-            (push (propertize (concat (match-string 1) (match-string 2))
-                              'help-echo (match-string 3))
-                  table)))
-        ;; Cache the table for later reuse.
-        (setq Man-completion-cache (cons "" (nreverse table))))
+	  ;; Cache the table for later reuse.
+	  (setq Man-completion-cache (cons "" (setq table
+						    (Man-parse-apropos))))))
       ;; The table may contain false positives since the match is made
       ;; by "man -k" not just on the manpage's name.
       (if section

=== added file 'test/automated/man-tests.el'
--- test/automated/man-tests.el	1970-01-01 00:00:00 +0000
+++ test/automated/man-tests.el	2012-12-21 22:55:19 +0000
@@ -0,0 +1,118 @@
+;;; man-tests.el --- Test suite for man.
+
+;; Copyright (C) 2012  Free Software Foundation, Inc.
+
+;; Author: Wolfgang Jenkner <wjenkner@inode.at>
+;; Keywords: help, internal, unix
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'man)
+
+(defconst man-tests-parse-apropos-tests
+  '(;; GNU/Linux: man-db-2.6.1
+    ("\
+sin (3)              - sine function
+sinf (3)             - sine function
+sinl (3)             - sine function"
+     . (#("sin(3)" 0 6 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sinl(3)" 0 7 (help-echo "sine function"))))
+    ;; GNU/Linux: man-1.6g
+    ("\
+sin                  (3)  - sine function
+sinf [sin]           (3)  - sine function
+sinl [sin]           (3)  - sine function"
+     . (#("sin(3)" 0 6 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sinl(3)" 0 7 (help-echo "sine function"))))
+    ;; FreeBSD 9
+    ("\
+sin(3), sinf(3), sinl(3) - sine functions"
+     . (#("sin(3)" 0 6 (help-echo "sine functions")) #("sinf(3)" 0 7 (help-echo "sine functions")) #("sinl(3)" 0 7 (help-echo "sine functions"))))
+    ;; SunOS, Solaris
+    ;; http://docs.oracle.com/cd/E19455-01/805-6331/usradm-7/index.html
+    ;; SunOS 4
+    ("\
+tset, reset (1)    - establish or restore terminal characteristics"
+     . (#("tset(1)" 0 7 (help-echo "establish or restore terminal characteristics")) #("reset(1)" 0 8 (help-echo "establish or restore terminal characteristics"))))
+    ;; SunOS 5.7, Solaris
+    ("\
+reset  tset (1b)   - establish or restore terminal characteristics
+tset   tset (1b)   - establish or restore terminal characteristics"
+     . (#("reset(1b)" 0 8 (help-echo "establish or restore terminal characteristics")) #("tset(1b)" 0 7 (help-echo "establish or restore terminal characteristics"))))
+    ;; Minix 3
+    ;; http://www.minix3.org/manpages/html5/whatis.html
+    ("\
+cawf, nroff (1) - C version of the nroff-like, Amazingly Workable (text) Formatter
+whatis (5) - database of online manual pages"
+     . (#("cawf(1)" 0 7 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter")) #("nroff(1)" 0 8 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter")) #("whatis(5)" 0 9 (help-echo "database of online manual pages"))))
+    ;; HP-UX
+    ;; http://docstore.mik.ua/manuals/hp-ux/en/B2355-60130/man.1.html
+    ;; Assuming that the line break in the zgrep description was
+    ;; introduced by the man page formatting.
+    ("\
+grep, egrep, fgrep (1) - search a file for a pattern
+zgrep(1) - search possibly compressed files for a regular expression"
+     . (#("grep(1)" 0 7 (help-echo "search a file for a pattern")) #("egrep(1)" 0 8 (help-echo "search a file for a pattern")) #("fgrep(1)" 0 8 (help-echo "search a file for a pattern")) #("zgrep(1)" 0 8 (help-echo "search possibly compressed files for a regular expression"))))
+    ;; AIX
+    ;; http://pic.dhe.ibm.com/infocenter/aix/v7r1/topic/com.ibm.aix.cmds/doc/aixcmds6/whatis.htm
+    ("\
+ls(1)  -Displays the contents of a directory."
+    . (#("ls(1)" 0 5 (help-echo "Displays the contents of a directory."))))
+    ;; https://www.ibm.com/developerworks/mydeveloperworks/blogs/cgaix/entry/catman_0703_102_usr_lbin_mkwhatis_the_error_number_is_1?lang=en
+    ("\
+loopmount(1)    - Associate an image file to a loopback device."
+     . (#("loopmount(1)" 0 12 (help-echo "Associate an image file to a loopback device."))))
+    )
+  "List of tests for `Man-parse-apropos'.
+Each element is a cons cell whose car is a string containing
+man -k output.  That should result in the table which is stored
+in the cdr of the element.")
+
+(defun man-tests-name-equal-p (name description string)
+  (and (equal name string)
+       (not (next-single-property-change 0 'help-echo string))
+       (equal (get-text-property 0 'help-echo string) description)))
+
+(defun man-tests-parse-apropos-test-case (test)
+  (let ((temp-buffer (get-buffer-create " *test-man*"))
+	(apropos-output (car test)))
+    (unwind-protect
+	(save-window-excursion
+	  (with-current-buffer temp-buffer
+	    (erase-buffer)
+	    (insert apropos-output)
+	    (let ((result (Man-parse-apropos))
+		  (checklist (cdr test)))
+	      (while (and checklist result
+			  (man-tests-name-equal-p
+			   (car checklist)
+			   (get-text-property 0 'help-echo
+					      (car checklist))
+			   (pop result)))
+		(pop checklist))
+	      (and (null checklist) (null result)))))
+      (and (buffer-name temp-buffer)
+	   (kill-buffer temp-buffer)))))
+
+(ert-deftest man-tests ()
+  "Test man."
+  (dolist (test man-tests-parse-apropos-tests)
+    (should (man-tests-parse-apropos-test-case test))))
+
+(provide 'man-tests)
+
+;;; man-tests.el ends here

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Stefan Monnier]

>>>> It's not used for parsing, it's only prepended to the search string
>>>> passed to `man -k'.
>>> Ah.  I even had a note in my bit about that.  Yes.  Daniel Pittman I
>>> think it was reported that on macos man -k '' empty string gives no
>>> output, hence "^".
>> Right, the empty string deserves special attention, also because it has
>> the potential to mess up the cache.  Thanks for reminding me of this.
> Actually, I think the cache is really the clue here: prepending
> completions piecemeal to the cache makes only sense if the user
> consciously avoids completing the empty string because of memory
> constraints... and even then the cache would have to be a trie or
> something to be useful.

I don't consciously avoid it, but I just never hit TAB to complete
a manpage name without first typing a few chars.  I doubt I'm the
only one.

On  my system "make -k ^" returns about 9K commands of average length
14chars, so that's about 9K * (16B (chars) + 16B (string object) + 2*8B
(cons cells)) or more than 400KB (on a 32bit system) or 700KB (on
a 64bit system) for that cache.  I'm not sure it's such a good idea.

Does it fix an actual bug? [ Sorry if it does fix a real bug and this
was mentioned somewhere in this thread, but I haven't found the time to
read all threads as thoroughly as I'd like.  ]

> 	Support man page completion for more man programs.  (Bug#13160)
> 	* man.el (Man-parse-apropos): New function.
> 	(Man-completion-table): Use it in parsing `man -k' output to
> 	replace code peculiar to man-db.
> 	Thanks to Kevin Ryde for commenting on a previous version of this.

This part of the patch looks OK, feel free to install it.

>       * automated/man-tests.el: New file.

And that as well, of course.


        Stefan

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Sat, Dec 22 2012, Stefan Monnier wrote:

> I don't consciously avoid it, but I just never hit TAB to complete
> a manpage name without first typing a few chars.  I doubt I'm the
> only one.
>
> On  my system "make -k ^" returns about 9K commands of average length
> 14chars, so that's about 9K * (16B (chars) + 16B (string object) + 2*8B
> (cons cells)) or more than 400KB (on a 32bit system) or 700KB (on
> a 64bit system) for that cache.  I'm not sure it's such a good idea.

Worrying about this is the main reason why I posted this, but:

On an old system with not much memory you probably won't have much room
for man pages either (unless you mount a NFS share).  I actually tested
this with a slax image under qemu emulating a i486 system with 64M
(without hardware supported virtualization) and I saw no problem with
it.  Of course, qemu can't really emulate processor or memory-access
speed, I think, and there were only 1000 man entries or so.  I'd love to
test this on my old thinkpad 350 with 20M, but it's difficult to install
a recent emacs on a 120M hard disk ;-)

That said, my main system is reasonable 2009 vintage, has

man -k ^ | wc -l
    8702

(with the actual entries being roughly the double of that)
and the delay is not noticeably longer when caching all entries at the
first completion attempt instead of just part of them.

> Does it fix an actual bug? [ Sorry if it does fix a real bug and this
> was mentioned somewhere in this thread, but I haven't found the time to
> read all threads as thoroughly as I'd like.  ]

My proposed change log entry indicates that the cache handling is rather
bogus (complete for "foo" then for "bar" and then for "foo" again and
"man -k ^foo" will be run again and the result prepended to the cache).

The main reason, however, is that it eliminates any dependency on the
particular flavour of `man -k' output, so everything can be done in
elisp by the new Man-parse-apropos (I indicated that in the change log
entry, too).

Wolfgang

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Kevin Ryde]

Wolfgang Jenkner <wjenkner@inode.at> writes:
>
> So let's just cache all entries at the first completion attempt.

I do that for speed and because I have some pagename-at-point stuff
which tries some prefix/suffix combinations to make a good guess.
Holding everything uses up a fair amount of memory but I never came up
with anything better.  I only keep the description part of each name if
needed for completing-help.el or icicles.  I suppose turfing the cache
after a timer could reclaim memory.

For the cache I also watch /var/cache/man/index.db (configurable) for
mtime changes indicating newly installed programs.  If like me you
download and install to have a look at something then new pagenames
happen quite often and an automatic reload is good.

With man-db the index.db is a gdbm or whatever database of course.
I suppose direct access would be both much less ram and faster, but I
don't think man-db advertises it for external use.  I never worked out
why running man -k isn't already faster than it seems to be -- just the
quantity of data being churned by it and then by emacs I suppose.




-- 
Walk without rhythm and it won't attract the worm.

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Stefan Monnier]

>> Does it fix an actual bug? [ Sorry if it does fix a real bug and this
>> was mentioned somewhere in this thread, but I haven't found the time to
>> read all threads as thoroughly as I'd like.  ]
> My proposed change log entry indicates that the cache handling is rather
> bogus (complete for "foo" then for "bar" and then for "foo" again and
> "man -k ^foo" will be run again and the result prepended to the cache).

Oh, indeed.  Fixed with the trivial patch below, tho.

> The main reason, however, is that it eliminates any dependency on the
> particular flavour of `man -k' output, so everything can be done in
> elisp by the new Man-parse-apropos (I indicated that in the change log
> entry, too).

That indeed the part of the change log I did not understand.  How does
the "man -k" argument relate to the "output flavor"?


        Stefan


=== modified file 'lisp/man.el'
--- lisp/man.el	2012-08-16 01:03:53 +0000
+++ lisp/man.el	2012-12-23 13:45:03 +0000
@@ -815,6 +815,7 @@
               (call-process manual-program nil '(t nil) nil
                             "-k" (concat "^" prefix))))
           (goto-char (point-min))
+          (setq table nil)
           (while (re-search-forward "^\\([^ \t\n]+\\)\\(?: ?\\((.+?)\\)\\(?:[ \t]+- \\(.*\\)\\)?\\)?" nil t)
             (push (propertize (concat (match-string 1) (match-string 2))
                               'help-echo (match-string 3))

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Sun, Dec 23 2012, Stefan Monnier wrote:

>>> Does it fix an actual bug? [ Sorry if it does fix a real bug and this
>>> was mentioned somewhere in this thread, but I haven't found the time to
>>> read all threads as thoroughly as I'd like.  ]
>> My proposed change log entry indicates that the cache handling is rather
>> bogus (complete for "foo" then for "bar" and then for "foo" again and
>> "man -k ^foo" will be run again and the result prepended to the cache).
>
> Oh, indeed.  Fixed with the trivial patch below, tho.

Well yes, you nuke the old content, whis is what the previous versions
of my patches proposed here do as well, but this can hardly be called
a useful cache then.

>> The main reason, however, is that it eliminates any dependency on the
>> particular flavour of `man -k' output, so everything can be done in
>> elisp by the new Man-parse-apropos (I indicated that in the change log
>> entry, too).
>
> That indeed the part of the change log I did not understand.  How does
> the "man -k" argument relate to the "output flavor"?

Well, I wrote in the proposed change log:

	Conveniently, the change entails that we don't have to guess the
	output format of `man -k' in order to pass it a useful regexp.

The problem is that the page names are not necessarily anchored at the
beginning of a summary line (there are examples in the proposed
man-tests.el), so we can't always use "man -k ^foo" to find all summary
lines for man page names starting with "foo".  A previous version of the
patch posted here introduced a new variable holding a suitable regexp
`anchor', which, however, would depend on the output format of `man -k',
its flavour :-)

I think that parsing the full listing obtained with `man -k ^' is the
best solution, it's just that storing the result in a plain list is not
quite optimal, hence my babbling about tries.

I can wait with my changes until we've found a better data structure.

Wolfgang

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Sun, Dec 23 2012, Wolfgang Jenkner wrote:

> Well yes, you nuke the old content, whis is what the previous versions
> of my patches proposed here do as well,

The previous (second) version did, the very first version did not fix this.

Wolfgang

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Stefan Monnier]

>>>> Does it fix an actual bug? [ Sorry if it does fix a real bug and this
>>>> was mentioned somewhere in this thread, but I haven't found the time to
>>>> read all threads as thoroughly as I'd like.  ]
>>> My proposed change log entry indicates that the cache handling is rather
>>> bogus (complete for "foo" then for "bar" and then for "foo" again and
>>> "man -k ^foo" will be run again and the result prepended to the cache).
>> Oh, indeed.  Fixed with the trivial patch below, tho.
> Well yes, you nuke the old content, whis is what the previous versions
> of my patches proposed here do as well, but this can hardly be called
> a useful cache then.

It can still be very useful: when you do M-x man RET pe TAB r TAB TAB
it will only run `man' once instead of 3 or more times.
That's the main purpose of the cache.  The fact that the cache is
preserved between runs of M-x man is probably more a misfeature, since
it means that the cache is never refreshed (i.e. gets out of date after
"aptitude upgrade").


        Stefan

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Stefan Monnier]

[ Second part of my reply, sorry I had to leave in a hurry.  ]

>> That indeed the part of the change log I did not understand.  How does
>> the "man -k" argument relate to the "output flavor"?
> Well, I wrote in the proposed change log:
> 	Conveniently, the change entails that we don't have to guess the
> 	output format of `man -k' in order to pass it a useful regexp.
> The problem is that the page names are not necessarily anchored at the
> beginning of a summary line (there are examples in the proposed
> man-tests.el), so we can't always use "man -k ^foo" to find all summary
> lines for man page names starting with "foo".  A previous version of the
> patch posted here introduced a new variable holding a suitable regexp
> `anchor', which, however, would depend on the output format of `man -k',
> its flavour :-)

Oh, so you're saying that the regexp passed to "man -k" is matched not
against plain command names, but against lines of the same shape as the
output of "man -k"?  IOW, "man -k RE" is barely more than "man -k ^ |
grep RE"? Oh, yes, I see that now, indeed (I guess I did know that
a some point, but it doesn't even ring a remote bell).

> I think that parsing the full listing obtained with `man -k ^' is the
> best solution, it's just that storing the result in a plain list is not
> quite optimal, hence my babbling about tries.

The problem is not only the lookup time (which is usually small enough
not to be a problem), but the actual memory use (which slows down GC
among other things).  My earlier estimate was very optimistic since it
didn't take into account the `help-echo' text-property, which more than
doubles the size.

The easiest is probably to keep the current structure and just drop the
^ if the string is non-empty (we'll get lots more false-positives than
with the ^, but we still gain: even for some single letters, like
"man -k b", we get less than half as much data as "man -k ^").


        Stefan

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Mon, Dec 24 2012, Stefan Monnier wrote:

> The easiest is probably to keep the current structure and just drop the
> ^ if the string is non-empty (we'll get lots more false-positives than
> with the ^, but we still gain: even for some single letters, like
> "man -k b", we get less than half as much data as "man -k ^").

You mean, for all man programs?  I considered this as well, but I just
don't feel like making things worse for the two man programs which are
used on GNU-based system, and which do have a single page name per line,
and at its beginning (quite reasonably so, and in the spirit of POSIX
man(1p), I'd say).  So perhaps the second patch I proposed (without the
test for valid output of `man -k ^')

http://permalink.gmane.org/gmane.emacs.bugs/68605

would be better, after all, even though the
Man-keyword-completion-anchor variable is ugly.

Wolfgang

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Stefan Monnier]

> You mean, for all man programs?  I considered this as well, but I just
> don't feel like making things worse for the two man programs which are
> used on GNU-based system, and which do have a single page name per line,
> and at its beginning (quite reasonably so, and in the spirit of POSIX
> man(1p), I'd say).  So perhaps the second patch I proposed (without the
> test for valid output of `man -k ^')

> http://permalink.gmane.org/gmane.emacs.bugs/68605

Hmmm, yes that looks fine.

> would be better, after all, even though the
> Man-keyword-completion-anchor variable is ugly.

We could replace it with a boolean var like `Man--k-anchor-broken',
since I think it's a bug for "man -k ^foo" to not match commands that
start with a "foo" prefix.


        Stefan

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Kevin Ryde]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
> "man -k RE"

Daniel Pittman I think also reported that on macos the pattern is a
plain grep BRE whereas posix says it's supposed to be an egrep ERE.
Or some such.  If that makes a difference to escaping special chars ...




-- 
"How can you have any pudding if you don't eat your meat."

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Mon, Dec 24 2012, Stefan Monnier wrote:

> The fact that the cache is preserved between runs of M-x man is
> probably more a misfeature, since it means that the cache is never
> refreshed (i.e. gets out of date after "aptitude upgrade").

Given that and the fact that indefinitely keeping around large lists
(like Man-completion-cache holding the completion of an empty prefix) is
bad, why not preventing this (the first hunk is the fix you proposed in
http://permalink.gmane.org/gmane.emacs.bugs/68945).

Wolfgang

-- >8 --
Subject: [PATCH] * lisp/man.el (Man-completion-table): Reset cache to nil.
 (man): Bind Man-completion-cache to nil, thus clarifying that it is not meant
 to be persistent.  (Bug#13160)

---
 lisp/man.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/man.el b/lisp/man.el
index 847917d8..8210d63 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -815,6 +815,7 @@ POS defaults to `point'."
               (call-process manual-program nil '(t nil) nil
                             "-k" (concat "^" prefix))))
           (goto-char (point-min))
+          (setq table nil)
           (while (re-search-forward "^\\([^ \t\n]+\\)\\(?: ?\\((.+?)\\)\\(?:[ \t]+- \\(.*\\)\\)?\\)?" nil t)
             (push (propertize (concat (match-string 1) (match-string 2))
                               'help-echo (match-string 3))
@@ -891,6 +892,7 @@ names or descriptions.  The pattern argument is usually an
 		;; ("man -k" is case-insensitive similarly, so the
 		;; table has everything available to complete)
 		(completion-ignore-case t)
+		Man-completion-cache
 		(input (completing-read
 			(format "Manual entry%s"
 				(if (string= default-entry "")
-- 
1.8.0.2

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Wolfgang Jenkner]

On Mon, Dec 24 2012, Stefan Monnier wrote:

> We could replace it with a boolean var like `Man--k-anchor-broken',
> since I think it's a bug for "man -k ^foo" to not match commands that
> start with a "foo" prefix.

With a different name:

-- >8 --
Subject: [PATCH] Support man page completion for more man programs. *
 lisp/man.el (Man-parse-man-k): New function. (Man-man-k-use-anchor): New
 variable. (Man-completion-table): Use them to replace man-db specific code
 for parsing `man -k' output.

* test/automated/man-tests.el: New file.
---
 lisp/man.el                 |  75 +++++++++++++++++++++++-----
 test/automated/man-tests.el | 118 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+), 11 deletions(-)
 create mode 100644 test/automated/man-tests.el

diff --git a/lisp/man.el b/lisp/man.el
index 8210d63..ee3f507 100644
--- a/lisp/man.el
+++ b/lisp/man.el
@@ -780,6 +780,56 @@ POS defaults to `point'."
   ;; but apparently that's not the case in all cases, so let's add a cache.
   "Cache of completion table of the form (PREFIX . TABLE).")
 
+(defvar Man-man-k-use-anchor
+  ;; man-db or man-1.*
+  (memq system-type '(gnu gnu/linux gnu/kfreebsd))
+  "If non-nil prepend ^ to the prefix passed to \"man -k\" for completion.
+The value should be nil if \"man -k ^PREFIX\" may omit some man
+pages whose names start with PREFIX.
+
+Currently, the default value depends on `system-type' and is
+non-nil where the standard man programs are known to behave
+properly.  Setting the value to nil always gives correct results
+but computing the list of completions may take a bit longer.")
+
+(defun Man-parse-man-k ()
+  "Parse \"man -k\" output and return the list of page names.
+
+The current buffer should contain the output of a command of the
+form \"man -k keyword\", which is traditionally also available with
+apropos(1).
+
+While POSIX man(1p) is a bit vague about what to expect here,
+this function tries to parse some commonly used formats, which
+can be described in the following informal way, with square brackets
+indicating optional parts and whitespace being interpreted
+somewhat loosely.
+
+foo[, bar [, ...]] [other stuff] (sec) - description
+foo(sec)[, bar(sec) [, ...]] [other stuff] - description
+
+For more details and some regression tests, please see
+test/automated/man-tests.el in the emacs bzr repository."
+  (goto-char (point-min))
+  (let (table)
+    (while (search-forward-regexp "^\\([^ \t,\n]+\\)\\(.*?\\)\
+\\(?:[ \t]\\(([^ \t,\n]+?)\\)\\)?\\(?:[ \t]+- ?\\(.*\\)\\)?$" nil t)
+      (let ((section (match-string 3))
+	    (description (match-string 4))
+	    (bound (match-end 2))
+	    (continue (goto-char (match-end 1))))
+	(while continue
+	  ;; The first regexp grouping may already match the section
+	  ;; tacked on to the name, which is ok since for the formats we
+	  ;; claim to support the third (non-shy) grouping does not
+	  ;; match in this case, i.e., section is nil.
+	  (push (propertize (concat (match-string 1) section)
+			    'help-echo description)
+		table)
+	  (setq continue (search-forward-regexp "\\=, *\\([^ \t,]+\\)"
+						bound t)))))
+    (nreverse table)))
+
 (defun Man-completion-table (string pred action)
   (cond
    ;; This ends up returning t for pretty much any string, and hence leads to
@@ -811,17 +861,20 @@ POS defaults to `point'."
             ;; run differently in Man-getpage-in-background, an error
             ;; here may not necessarily mean that we'll also get an
             ;; error later.
-            (ignore-errors
-              (call-process manual-program nil '(t nil) nil
-                            "-k" (concat "^" prefix))))
-          (goto-char (point-min))
-          (setq table nil)
-          (while (re-search-forward "^\\([^ \t\n]+\\)\\(?: ?\\((.+?)\\)\\(?:[ \t]+- \\(.*\\)\\)?\\)?" nil t)
-            (push (propertize (concat (match-string 1) (match-string 2))
-                              'help-echo (match-string 3))
-                  table)))
-        ;; Cache the table for later reuse.
-        (setq Man-completion-cache (cons prefix table)))
+	    (ignore-errors
+	      (call-process manual-program nil '(t nil) nil
+			    "-k" (concat (when (or Man-man-k-use-anchor
+						   (string-equal prefix ""))
+					   "^")
+					 prefix))))
+	  (setq table (Man-parse-man-k)))
+	(unless Man-man-k-use-anchor
+	  ;; Weed out unwanted page names.
+	  (dolist (comp (prog1 table (setq table nil)))
+	    (if (string-prefix-p prefix comp)
+		(push comp table))))
+	;; Cache the table for later reuse.
+	(setq Man-completion-cache (cons prefix table)))
       ;; The table may contain false positives since the match is made
       ;; by "man -k" not just on the manpage's name.
       (if section
diff --git a/test/automated/man-tests.el b/test/automated/man-tests.el
new file mode 100644
index 0000000..8a2ec95
--- /dev/null
+++ b/test/automated/man-tests.el
@@ -0,0 +1,118 @@
+;;; man-tests.el --- Test suite for man.
+
+;; Copyright (C) 2013  Free Software Foundation, Inc.
+
+;; Author: Wolfgang Jenkner <wjenkner@inode.at>
+;; Keywords: help, internal, unix
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'man)
+
+(defconst man-tests-parse-man-k-tests
+  '(;; GNU/Linux: man-db-2.6.1
+    ("\
+sin (3)              - sine function
+sinf (3)             - sine function
+sinl (3)             - sine function"
+     . (#("sin(3)" 0 6 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sinl(3)" 0 7 (help-echo "sine function"))))
+    ;; GNU/Linux: man-1.6g
+    ("\
+sin                  (3)  - sine function
+sinf [sin]           (3)  - sine function
+sinl [sin]           (3)  - sine function"
+     . (#("sin(3)" 0 6 (help-echo "sine function")) #("sinf(3)" 0 7 (help-echo "sine function")) #("sinl(3)" 0 7 (help-echo "sine function"))))
+    ;; FreeBSD 9
+    ("\
+sin(3), sinf(3), sinl(3) - sine functions"
+     . (#("sin(3)" 0 6 (help-echo "sine functions")) #("sinf(3)" 0 7 (help-echo "sine functions")) #("sinl(3)" 0 7 (help-echo "sine functions"))))
+    ;; SunOS, Solaris
+    ;; http://docs.oracle.com/cd/E19455-01/805-6331/usradm-7/index.html
+    ;; SunOS 4
+    ("\
+tset, reset (1)    - establish or restore terminal characteristics"
+     . (#("tset(1)" 0 7 (help-echo "establish or restore terminal characteristics")) #("reset(1)" 0 8 (help-echo "establish or restore terminal characteristics"))))
+    ;; SunOS 5.7, Solaris
+    ("\
+reset  tset (1b)   - establish or restore terminal characteristics
+tset   tset (1b)   - establish or restore terminal characteristics"
+     . (#("reset(1b)" 0 8 (help-echo "establish or restore terminal characteristics")) #("tset(1b)" 0 7 (help-echo "establish or restore terminal characteristics"))))
+    ;; Minix 3
+    ;; http://www.minix3.org/manpages/html5/whatis.html
+    ("\
+cawf, nroff (1) - C version of the nroff-like, Amazingly Workable (text) Formatter
+whatis (5) - database of online manual pages"
+     . (#("cawf(1)" 0 7 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter")) #("nroff(1)" 0 8 (help-echo "C version of the nroff-like, Amazingly Workable (text) Formatter")) #("whatis(5)" 0 9 (help-echo "database of online manual pages"))))
+    ;; HP-UX
+    ;; http://docstore.mik.ua/manuals/hp-ux/en/B2355-60130/man.1.html
+    ;; Assuming that the line break in the zgrep description was
+    ;; introduced by the man page formatting.
+    ("\
+grep, egrep, fgrep (1) - search a file for a pattern
+zgrep(1) - search possibly compressed files for a regular expression"
+     . (#("grep(1)" 0 7 (help-echo "search a file for a pattern")) #("egrep(1)" 0 8 (help-echo "search a file for a pattern")) #("fgrep(1)" 0 8 (help-echo "search a file for a pattern")) #("zgrep(1)" 0 8 (help-echo "search possibly compressed files for a regular expression"))))
+    ;; AIX
+    ;; http://pic.dhe.ibm.com/infocenter/aix/v7r1/topic/com.ibm.aix.cmds/doc/aixcmds6/whatis.htm
+    ("\
+ls(1)  -Displays the contents of a directory."
+    . (#("ls(1)" 0 5 (help-echo "Displays the contents of a directory."))))
+    ;; https://www.ibm.com/developerworks/mydeveloperworks/blogs/cgaix/entry/catman_0703_102_usr_lbin_mkwhatis_the_error_number_is_1?lang=en
+    ("\
+loopmount(1)    - Associate an image file to a loopback device."
+     . (#("loopmount(1)" 0 12 (help-echo "Associate an image file to a loopback device."))))
+    )
+  "List of tests for `Man-parse-man-k'.
+Each element is a cons cell whose car is a string containing
+man -k output.  That should result in the table which is stored
+in the cdr of the element.")
+
+(defun man-tests-name-equal-p (name description string)
+  (and (equal name string)
+       (not (next-single-property-change 0 'help-echo string))
+       (equal (get-text-property 0 'help-echo string) description)))
+
+(defun man-tests-parse-man-k-test-case (test)
+  (let ((temp-buffer (get-buffer-create " *test-man*"))
+	(man-k-output (car test)))
+    (unwind-protect
+	(save-window-excursion
+	  (with-current-buffer temp-buffer
+	    (erase-buffer)
+	    (insert man-k-output)
+	    (let ((result (Man-parse-man-k))
+		  (checklist (cdr test)))
+	      (while (and checklist result
+			  (man-tests-name-equal-p
+			   (car checklist)
+			   (get-text-property 0 'help-echo
+					      (car checklist))
+			   (pop result)))
+		(pop checklist))
+	      (and (null checklist) (null result)))))
+      (and (buffer-name temp-buffer)
+	   (kill-buffer temp-buffer)))))
+
+(ert-deftest man-tests ()
+  "Test man."
+  (dolist (test man-tests-parse-man-k-tests)
+    (should (man-tests-parse-man-k-test-case test))))
+
+(provide 'man-tests)
+
+;;; man-tests.el ends here
-- 
1.8.0.2

bug#13160: 24.3.50; [PATCH] man page completion support beyond man-db

[Stefan Monnier]

> Subject: [PATCH] Support man page completion for more man programs. *
>  lisp/man.el (Man-parse-man-k): New function. (Man-man-k-use-anchor): New
>  variable. (Man-completion-table): Use them to replace man-db specific code
>  for parsing `man -k' output.

Thanks, installed (along with the previous cache-flush patch and with
a few minor changes, mostly not trimming the extraneous entries we get
if the anchor is not used).


        Stefan