unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Fixes to let woman.el deal with MANPATH_MAP elements
@ 2007-03-23 10:29 David Kastrup
  2007-03-23 14:16 ` Eli Zaretskii
  2007-03-23 18:00 ` Richard Stallman
  0 siblings, 2 replies; 13+ messages in thread
From: David Kastrup @ 2007-03-23 10:29 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]


It has been annoying to me that M-x woman RET fails to obey
MANPATH_MAP elements in manpath.config.

It also has been annoying to me that woman.el happily picked up
manpath.config~ instead of manpath.config when available.

Both problems appear fixed with the following rather straightforward
patch.  While it clearly fixes faulty behavior, it could be considered
bordering on "new feature".

Should I still apply it?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 6284 bytes --]

*** woman.el	12 Mar 2007 20:24:09 +0100	1.55
--- woman.el	23 Mar 2007 09:45:53 +0100	
***************
*** 547,557 ****
  	(mapcar 'woman-Cyg-to-Win path)
        path))
    "*List of dirs to search and/or files to try for man config file.
! A trailing separator (`/' for UNIX etc.) on directories is optional,
! and the filename is used if a directory specified is the first to
! contain the strings \"man\" and \".conf\" (in that order).
! If MANPATH is not set but a config file is found then it is parsed
! instead to provide a default value for `woman-manpath'."
    :type '(repeat string)
    :group 'woman-interface)
  
--- 547,558 ----
  	(mapcar 'woman-Cyg-to-Win path)
        path))
    "*List of dirs to search and/or files to try for man config file.
! A trailing separator (`/' for UNIX etc.) on directories is
! optional, and the filename is used if a directory specified is
! the first to start with \"man\" and has an extension starting
! with \".conf\".  If MANPATH is not set but a config file is found
! then it is parsed instead to provide a default value for
! `woman-manpath'."
    :type '(repeat string)
    :group 'woman-interface)
  
***************
*** 564,570 ****
  or
    MANDATORY_MANPATH  /usr/man
  or
!   OPTIONAL_MANPATH  /usr/man"
    ;; Functionality suggested by Charles Curley.
    (let ((path woman-man.conf-path)
  	file manpath)
--- 565,573 ----
  or
    MANDATORY_MANPATH  /usr/man
  or
!   OPTIONAL_MANPATH  /usr/man
! or
!   MANPATH_MAP /opt/bin /opt/man"
    ;; Functionality suggested by Charles Curley.
    (let ((path woman-man.conf-path)
  	file manpath)
***************
*** 576,582 ****
  		  (or (not (file-directory-p file))
  		      (and
  		       (setq file
! 			     (directory-files file t "man.*\\.conf" t))
  		       (file-readable-p (setq file (car file)))))
  		  ;; Parse the file -- if no MANPATH data ignore it:
  		  (with-temp-buffer
--- 579,585 ----
  		  (or (not (file-directory-p file))
  		      (and
  		       (setq file
! 			     (directory-files file t "\\`man.*\\.conf[a-z]*\\'" t))
  		       (file-readable-p (setq file (car file)))))
  		  ;; Parse the file -- if no MANPATH data ignore it:
  		  (with-temp-buffer
***************
*** 584,591 ****
  		    (while (re-search-forward
  			    ;; `\(?: ... \)' is a "shy group"
  			    "\
! ^[ \t]*\\(?:MANDATORY_\\|OPTIONAL_\\)?MANPATH[ \t]+\\(\\S-+\\)" nil t)
! 		      (setq manpath (cons (match-string 1) manpath)))
  		    manpath))
  		 ))
        (setq path (cdr path)))
--- 587,599 ----
  		    (while (re-search-forward
  			    ;; `\(?: ... \)' is a "shy group"
  			    "\
! ^[ \t]*\\(?:\\(?:MANDATORY_\\|OPTIONAL_\\)?MANPATH[ \t]+\\(\\S-+\\)\\|\
! MANPATH_MAP[ \t]+\\(\\S-+\\)[ \t]+\\(\\S-+\\)\\)" nil t)
! 		      (add-to-list 'manpath
! 				   (if (match-beginning 1)
! 				       (match-string 1) 
! 				     (cons (match-string 2)
! 					   (match-string 3)))))
  		    manpath))
  		 ))
        (setq path (cdr path)))
***************
*** 600,605 ****
--- 608,618 ----
  selected by the value of `woman-manpath-man-regexp'.  Non-directory
  and unreadable files are ignored.
  
+ Elements can also be a cons cell indicating a mapping from PATH
+ to manual trees: if such an element's car is equal to a path
+ element of the environment variable PATH, the cdr of the cons
+ cell is included in the directory tree search.
+ 
  If not set then the environment variable MANPATH is used.  If no such
  environment variable is found, the default list is determined by
  consulting the man configuration file if found, which is determined by
***************
*** 618,624 ****
  
  The MANPATH environment variable may be set using DOS semi-colon-
  separated or UN*X/Cygwin colon-separated syntax (but not mixed)."
!   :type '(repeat string)
    :group 'woman-interface)
  
  (defcustom woman-manpath-man-regexp "[Mm][Aa][Nn]"
--- 631,637 ----
  
  The MANPATH environment variable may be set using DOS semi-colon-
  separated or UN*X/Cygwin colon-separated syntax (but not mixed)."
!   :type '(repeat (choice string (cons string string)))
    :group 'woman-interface)
  
  (defcustom woman-manpath-man-regexp "[Mm][Aa][Nn]"
***************
*** 1159,1165 ****
  Called both to generate and to check the cache!"
    ;; Must use substituted paths because values of env vars may change!
    (list woman-cache-level
! 	(mapcar 'substitute-in-file-name woman-manpath)
  	(mapcar 'substitute-in-file-name woman-path)))
  
  (defun woman-read-directory-cache ()
--- 1172,1183 ----
  Called both to generate and to check the cache!"
    ;; Must use substituted paths because values of env vars may change!
    (list woman-cache-level
! 	(let (lst path)
! 	  (dolist (dir woman-manpath (nreverse lst))
! 	    (when (consp dir)
! 	      (unless path (setq path (split-string (getenv "PATH") ":" t)))
! 	      (setq dir (and (member (car dir) path) (cdr dir))))
! 	    (when dir (add-to-list 'lst (substitute-in-file-name dir)))))
  	(mapcar 'substitute-in-file-name woman-path)))
  
  (defun woman-read-directory-cache ()
***************
*** 1320,1329 ****
    ;; Allow each path to be a single string or a list of strings:
    (if (not (listp woman-manpath)) (setq woman-manpath (list woman-manpath)))
    (if (not (listp woman-path)) (setq woman-path (list woman-path)))
!   (let (dir head dirs)
      (while woman-manpath
        (setq dir (car woman-manpath)
  	    woman-manpath (cdr woman-manpath))
        (if (and dir (woman-file-readable-p dir))
  	  ;; NB: `parse-colon-path' creates null elements for
  	  ;; redundant (semi-)colons and trailing `/'s!
--- 1338,1352 ----
    ;; Allow each path to be a single string or a list of strings:
    (if (not (listp woman-manpath)) (setq woman-manpath (list woman-manpath)))
    (if (not (listp woman-path)) (setq woman-path (list woman-path)))
!   (let (dir head dirs path)
      (while woman-manpath
        (setq dir (car woman-manpath)
  	    woman-manpath (cdr woman-manpath))
+       (when (consp dir)
+ 	(unless path
+ 	  (setq path (split-string (getenv "PATH") ":" t)))
+ 	(setq dir (and (member (car dir) path)
+ 		       (cdr dir))))
        (if (and dir (woman-file-readable-p dir))
  	  ;; NB: `parse-colon-path' creates null elements for
  	  ;; redundant (semi-)colons and trailing `/'s!

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 10:29 Fixes to let woman.el deal with MANPATH_MAP elements David Kastrup
@ 2007-03-23 14:16 ` Eli Zaretskii
  2007-03-23 18:34   ` David Kastrup
  2007-03-23 18:00 ` Richard Stallman
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2007-03-23 14:16 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

> From: David Kastrup <dak@gnu.org>
> Date: Fri, 23 Mar 2007 11:29:25 +0100
> 
> Both problems appear fixed with the following rather straightforward
> patch.  While it clearly fixes faulty behavior, it could be considered
> bordering on "new feature".
> 
> Should I still apply it?

That's something for Richard to decide.

> ! 	      (unless path (setq path (split-string (getenv "PATH") ":" t)))

Please don't install such unportable code: only Posix platforms use
`:' to separate directories in PATH and other similar variables.
Please use `path-separator' instead.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 10:29 Fixes to let woman.el deal with MANPATH_MAP elements David Kastrup
  2007-03-23 14:16 ` Eli Zaretskii
@ 2007-03-23 18:00 ` Richard Stallman
  2007-03-24 15:09   ` David Kastrup
  2007-03-31 15:25   ` David Kastrup
  1 sibling, 2 replies; 13+ messages in thread
From: Richard Stallman @ 2007-03-23 18:00 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

I think this is more of a bug fix than a new feature.
If other people test it and verify it works, then you can
check it in.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 14:16 ` Eli Zaretskii
@ 2007-03-23 18:34   ` David Kastrup
  2007-03-23 20:03     ` David Kastrup
  0 siblings, 1 reply; 13+ messages in thread
From: David Kastrup @ 2007-03-23 18:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: David Kastrup <dak@gnu.org>
>> Date: Fri, 23 Mar 2007 11:29:25 +0100
>> 
>> Both problems appear fixed with the following rather straightforward
>> patch.  While it clearly fixes faulty behavior, it could be considered
>> bordering on "new feature".
>> 
>> Should I still apply it?
>
> That's something for Richard to decide.
>
>> ! 	      (unless path (setq path (split-string (getenv "PATH") ":" t)))
>
> Please don't install such unportable code: only Posix platforms use
> `:' to separate directories in PATH and other similar variables.
> Please use `path-separator' instead.

Thanks, that was important information.  It would not have broken
previously existing behavior, but would not have helped, either.  I'll
rework the patch.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 18:34   ` David Kastrup
@ 2007-03-23 20:03     ` David Kastrup
  2007-03-23 20:12       ` Drew Adams
  2007-03-23 21:09       ` David Kastrup
  0 siblings, 2 replies; 13+ messages in thread
From: David Kastrup @ 2007-03-23 20:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

David Kastrup <dak@gnu.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: David Kastrup <dak@gnu.org>
>>> Date: Fri, 23 Mar 2007 11:29:25 +0100
>>> 
>>> Both problems appear fixed with the following rather straightforward
>>> patch.  While it clearly fixes faulty behavior, it could be considered
>>> bordering on "new feature".
>>> 
>>> Should I still apply it?
>>
>> That's something for Richard to decide.
>>
>>> ! 	      (unless path (setq path (split-string (getenv "PATH") ":" t)))
>>
>> Please don't install such unportable code: only Posix platforms use
>> `:' to separate directories in PATH and other similar variables.
>> Please use `path-separator' instead.
>
> Thanks, that was important information.  It would not have broken
> previously existing behavior, but would not have helped, either.  I'll
> rework the patch.

Eli?

I am utterly befuddled by the code in woman.el.  Should I be using
"parse-colon-path" or "woman-parse-colon-path" here?

It is not clear to me whether "PATH" is covered by the same special
rules that either one of the above functions would use.

As the code appears to be cowritten by you, could you suggest what to
use instead of

(split-string (getenv "PATH") ":" t)

here?  Should I use one of the above-mentioned functions, or just
(split-string (getenv "PATH") path-separator t)?

Thanks,

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 20:03     ` David Kastrup
@ 2007-03-23 20:12       ` Drew Adams
  2007-03-23 20:25         ` David Kastrup
  2007-03-23 21:09       ` David Kastrup
  1 sibling, 1 reply; 13+ messages in thread
From: Drew Adams @ 2007-03-23 20:12 UTC (permalink / raw)
  To: emacs-devel

> "woman-parse-colon-path"

Gotta love it.  What will Emacs teach us next?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 20:12       ` Drew Adams
@ 2007-03-23 20:25         ` David Kastrup
  0 siblings, 0 replies; 13+ messages in thread
From: David Kastrup @ 2007-03-23 20:25 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel

"Drew Adams" <drew.adams@oracle.com> writes:

>> "woman-parse-colon-path"
>
> Gotta love it.  What will Emacs teach us next?

Actually, this function again calls woman-parse-man.conf, so
woman-parse-man.conf should likely not call it...

And I need to fix those calls, too.

What a mess.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 20:03     ` David Kastrup
  2007-03-23 20:12       ` Drew Adams
@ 2007-03-23 21:09       ` David Kastrup
  1 sibling, 0 replies; 13+ messages in thread
From: David Kastrup @ 2007-03-23 21:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

David Kastrup <dak@gnu.org> writes:

> I am utterly befuddled by the code in woman.el.  Should I be using
> "parse-colon-path" or "woman-parse-colon-path" here?
>
> It is not clear to me whether "PATH" is covered by the same special
> rules that either one of the above functions would use.
>
> As the code appears to be cowritten by you, could you suggest what to
> use instead of
>
> (split-string (getenv "PATH") ":" t)
>
> here?  Should I use one of the above-mentioned functions, or just
> (split-string (getenv "PATH") path-separator t)?

Ok, I have seen that parse-colon-path returns directory names rather
than directory file names, so that is unsuitable for MANPATH_MAP.
Also it would appear that I overlooked woman-Cyg-to-Win's behavior
(not sure whether Cygwin even has MANPATH_MAP, but better safe than
sorry).  I believe that now the behavior should be more or less
consistent in the Windows case (at least not less consistent than the
preexisting mess).

I don't understand why all this Cygwinniness should be in woman.el to
start with: this kind of stuff does not seem specific to woman.el, but
more like a general problem.

It also turns out that woman.texi exists, so it seems like I need to
add some words there, too.

Sigh.  Fixing things is more work than I'd like to.

Could some people using Windows and/or Cygwin systems perhaps check
the behavior?  Bonus points if their manpath.conf contains MANPATH_MAP
elements.  The current code should not cause a regression.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 8174 bytes --]

*** woman.el	12 Mar 2007 20:24:09 +0100	1.55
--- woman.el	23 Mar 2007 21:39:56 +0100	
***************
*** 486,509 ****
  
  (defun woman-Cyg-to-Win (file)
    "Convert an absolute filename FILE from Cygwin to Windows form."
!   ;; Code taken from w32-symlinks.el
!   (if (eq (aref file 0) ?/)
!       ;; Try to use Cygwin mount table via `cygpath.exe'.
!       (condition-case nil
! 	  (with-temp-buffer
! 	    ;; cygpath -m file
! 	    (call-process "cygpath" nil t nil "-m" file)
! 	    (buffer-substring 1 (buffer-size)))
! 	(error
! 	 ;; Assume no `cygpath' program available.
! 	 ;; Hack /cygdrive/x/ or /x/ or (obsolete) //x/ to x:/
! 	 (when (string-match "\\`\\(/cygdrive\\|/\\)?/./" file)
! 	   (if (match-string 1)		; /cygdrive/x/ or //x/ -> /x/
! 	       (setq file (substring file (match-end 1))))
! 	   (aset file 0 (aref file 1))	; /x/ -> xx/
! 	   (aset file 1 ?:))		; xx/ -> x:/
! 	 file))
!     file))
  
  \f
  ;;; User options:
--- 486,513 ----
  
  (defun woman-Cyg-to-Win (file)
    "Convert an absolute filename FILE from Cygwin to Windows form."
!   ;; MANPATH_MAP conses are not converted since they presumably map
!   ;; Cygwin to Cygwin form.
!   (if (consp file)
!       file
!     ;; Code taken from w32-symlinks.el
!     (if (eq (aref file 0) ?/)
! 	;; Try to use Cygwin mount table via `cygpath.exe'.
! 	(condition-case nil
! 	    (with-temp-buffer
! 	      ;; cygpath -m file
! 	      (call-process "cygpath" nil t nil "-m" file)
! 	      (buffer-substring 1 (buffer-size)))
! 	  (error
! 	   ;; Assume no `cygpath' program available.
! 	   ;; Hack /cygdrive/x/ or /x/ or (obsolete) //x/ to x:/
! 	   (when (string-match "\\`\\(/cygdrive\\|/\\)?/./" file)
! 	     (if (match-string 1)		; /cygdrive/x/ or //x/ -> /x/
! 		 (setq file (substring file (match-end 1))))
! 	     (aset file 0 (aref file 1))	; /x/ -> xx/
! 	     (aset file 1 ?:))		; xx/ -> x:/
! 	   file))
!       file)))
  
  \f
  ;;; User options:
***************
*** 547,557 ****
  	(mapcar 'woman-Cyg-to-Win path)
        path))
    "*List of dirs to search and/or files to try for man config file.
! A trailing separator (`/' for UNIX etc.) on directories is optional,
! and the filename is used if a directory specified is the first to
! contain the strings \"man\" and \".conf\" (in that order).
! If MANPATH is not set but a config file is found then it is parsed
! instead to provide a default value for `woman-manpath'."
    :type '(repeat string)
    :group 'woman-interface)
  
--- 551,562 ----
  	(mapcar 'woman-Cyg-to-Win path)
        path))
    "*List of dirs to search and/or files to try for man config file.
! A trailing separator (`/' for UNIX etc.) on directories is
! optional, and the filename is used if a directory specified is
! the first to start with \"man\" and has an extension starting
! with \".conf\".  If MANPATH is not set but a config file is found
! then it is parsed instead to provide a default value for
! `woman-manpath'."
    :type '(repeat string)
    :group 'woman-interface)
  
***************
*** 564,570 ****
  or
    MANDATORY_MANPATH  /usr/man
  or
!   OPTIONAL_MANPATH  /usr/man"
    ;; Functionality suggested by Charles Curley.
    (let ((path woman-man.conf-path)
  	file manpath)
--- 569,577 ----
  or
    MANDATORY_MANPATH  /usr/man
  or
!   OPTIONAL_MANPATH  /usr/man
! or
!   MANPATH_MAP /opt/bin /opt/man"
    ;; Functionality suggested by Charles Curley.
    (let ((path woman-man.conf-path)
  	file manpath)
***************
*** 576,582 ****
  		  (or (not (file-directory-p file))
  		      (and
  		       (setq file
! 			     (directory-files file t "man.*\\.conf" t))
  		       (file-readable-p (setq file (car file)))))
  		  ;; Parse the file -- if no MANPATH data ignore it:
  		  (with-temp-buffer
--- 583,589 ----
  		  (or (not (file-directory-p file))
  		      (and
  		       (setq file
! 			     (directory-files file t "\\`man.*\\.conf[a-z]*\\'" t))
  		       (file-readable-p (setq file (car file)))))
  		  ;; Parse the file -- if no MANPATH data ignore it:
  		  (with-temp-buffer
***************
*** 584,591 ****
  		    (while (re-search-forward
  			    ;; `\(?: ... \)' is a "shy group"
  			    "\
! ^[ \t]*\\(?:MANDATORY_\\|OPTIONAL_\\)?MANPATH[ \t]+\\(\\S-+\\)" nil t)
! 		      (setq manpath (cons (match-string 1) manpath)))
  		    manpath))
  		 ))
        (setq path (cdr path)))
--- 591,603 ----
  		    (while (re-search-forward
  			    ;; `\(?: ... \)' is a "shy group"
  			    "\
! ^[ \t]*\\(?:\\(?:MANDATORY_\\|OPTIONAL_\\)?MANPATH[ \t]+\\(\\S-+\\)\\|\
! MANPATH_MAP[ \t]+\\(\\S-+\\)[ \t]+\\(\\S-+\\)\\)" nil t)
! 		      (add-to-list 'manpath
! 				   (if (match-beginning 1)
! 				       (match-string 1) 
! 				     (cons (match-string 2)
! 					   (match-string 3)))))
  		    manpath))
  		 ))
        (setq path (cdr path)))
***************
*** 600,605 ****
--- 612,622 ----
  selected by the value of `woman-manpath-man-regexp'.  Non-directory
  and unreadable files are ignored.
  
+ Elements can also be a cons cell indicating a mapping from PATH
+ to manual trees: if such an element's car is equal to a path
+ element of the environment variable PATH, the cdr of the cons
+ cell is included in the directory tree search.
+ 
  If not set then the environment variable MANPATH is used.  If no such
  environment variable is found, the default list is determined by
  consulting the man configuration file if found, which is determined by
***************
*** 618,624 ****
  
  The MANPATH environment variable may be set using DOS semi-colon-
  separated or UN*X/Cygwin colon-separated syntax (but not mixed)."
!   :type '(repeat string)
    :group 'woman-interface)
  
  (defcustom woman-manpath-man-regexp "[Mm][Aa][Nn]"
--- 635,641 ----
  
  The MANPATH environment variable may be set using DOS semi-colon-
  separated or UN*X/Cygwin colon-separated syntax (but not mixed)."
!   :type '(repeat (choice string (cons string string)))
    :group 'woman-interface)
  
  (defcustom woman-manpath-man-regexp "[Mm][Aa][Nn]"
***************
*** 1159,1165 ****
  Called both to generate and to check the cache!"
    ;; Must use substituted paths because values of env vars may change!
    (list woman-cache-level
! 	(mapcar 'substitute-in-file-name woman-manpath)
  	(mapcar 'substitute-in-file-name woman-path)))
  
  (defun woman-read-directory-cache ()
--- 1176,1189 ----
  Called both to generate and to check the cache!"
    ;; Must use substituted paths because values of env vars may change!
    (list woman-cache-level
! 	(let (lst path)
! 	  (dolist (dir woman-manpath (nreverse lst))
! 	    (when (consp dir)
! 	      (unless path
! 		(setq path
! 		      (split-string (getenv "PATH") path-separator t)))
! 	      (setq dir (and (member (car dir) path) (cdr dir))))
! 	    (when dir (add-to-list 'lst (substitute-in-file-name dir)))))
  	(mapcar 'substitute-in-file-name woman-path)))
  
  (defun woman-read-directory-cache ()
***************
*** 1320,1329 ****
    ;; Allow each path to be a single string or a list of strings:
    (if (not (listp woman-manpath)) (setq woman-manpath (list woman-manpath)))
    (if (not (listp woman-path)) (setq woman-path (list woman-path)))
!   (let (dir head dirs)
      (while woman-manpath
        (setq dir (car woman-manpath)
  	    woman-manpath (cdr woman-manpath))
        (if (and dir (woman-file-readable-p dir))
  	  ;; NB: `parse-colon-path' creates null elements for
  	  ;; redundant (semi-)colons and trailing `/'s!
--- 1344,1358 ----
    ;; Allow each path to be a single string or a list of strings:
    (if (not (listp woman-manpath)) (setq woman-manpath (list woman-manpath)))
    (if (not (listp woman-path)) (setq woman-path (list woman-path)))
!   (let (dir head dirs path)
      (while woman-manpath
        (setq dir (car woman-manpath)
  	    woman-manpath (cdr woman-manpath))
+       (when (consp dir)
+ 	(unless path
+ 	  (setq path (split-string (getenv "PATH") path-separator t)))
+ 	(setq dir (and (member (car dir) path)
+ 		       (cdr dir))))
        (if (and dir (woman-file-readable-p dir))
  	  ;; NB: `parse-colon-path' creates null elements for
  	  ;; redundant (semi-)colons and trailing `/'s!

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]



-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 18:00 ` Richard Stallman
@ 2007-03-24 15:09   ` David Kastrup
  2007-03-24 16:57     ` Eli Zaretskii
  2007-03-31 15:25   ` David Kastrup
  1 sibling, 1 reply; 13+ messages in thread
From: David Kastrup @ 2007-03-24 15:09 UTC (permalink / raw)
  To: emacs-devel

Richard Stallman <rms@gnu.org> writes:

> I think this is more of a bug fix than a new feature.

I certainly needed it in order to make woman behave properly, matching
more closely what man does.

> If other people test it and verify it works, then you can
> check it in.

Are there actually other users of "woman"?  While I tested the fixes
myself on GNU/Linux, woman.el does a _lot_ of Windows/Cygwin centric
stuff (however wise or not it may be to put such code into woman.el
instead of providing the general facilities elsewhere).  While I think
that I got the Cygwin angle covered properly by now, I have not
actually have the ability to check so myself.

If we don't have active users on this list, I don't see a good way to
get testing coverage apart from checking it in and having it appear in
the next pretest.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-24 15:09   ` David Kastrup
@ 2007-03-24 16:57     ` Eli Zaretskii
  2007-03-24 17:07       ` David Kastrup
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2007-03-24 16:57 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

> From: David Kastrup <dak@gnu.org>
> Date: Sat, 24 Mar 2007 16:09:41 +0100
> 
> Are there actually other users of "woman"?

Yes, there are.

> While I tested the fixes myself on GNU/Linux, woman.el does a _lot_
> of Windows/Cygwin centric stuff

That's because Windows systems don't come with a `man' command
installed.

> (however wise or not it may be to put such code into woman.el
> instead of providing the general facilities elsewhere).

There's no good general solution to mixing Cygwin programs with a
native Windows build of Emacs.  The author of woman.el wanted the
package to be kinder to such mixing that the rest of Emacs.

> If we don't have active users on this list, I don't see a good way to
> get testing coverage apart from checking it in and having it appear in
> the next pretest.

I'm not against committing this in time for the next pretest, but the
next pretest is still some time away, and you should see a heads-up
several days before that.  If by that time no one hollers, I think you
should go ahead and commit it.

Note that in addition to the Cygwin stuff, woman.el should also be
tested on Windows even without Cygwin.  Could people who don't have
man.exe installed on Windows please try the patched version and see
that the patched functions still work?  Thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-24 16:57     ` Eli Zaretskii
@ 2007-03-24 17:07       ` David Kastrup
  2007-03-24 17:39         ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: David Kastrup @ 2007-03-24 17:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: David Kastrup <dak@gnu.org>
>> Date: Sat, 24 Mar 2007 16:09:41 +0100
>> 
>> Are there actually other users of "woman"?
>
> Yes, there are.
>
>> While I tested the fixes myself on GNU/Linux, woman.el does a _lot_
>> of Windows/Cygwin centric stuff
>
> That's because Windows systems don't come with a `man' command
> installed.

I don't understand.  woman does not use any `man' command on other
operating systems either, so this is quite orthogonal to the operating
system support.

One interesting question is what kind of manpath.config one can expect
to be found on Cygwin systems when there is no actual `man' command
using it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-24 17:07       ` David Kastrup
@ 2007-03-24 17:39         ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2007-03-24 17:39 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

> Cc: emacs-devel@gnu.org
> From: David Kastrup <dak@gnu.org>
> Date: Sat, 24 Mar 2007 18:07:55 +0100
> 
> >> While I tested the fixes myself on GNU/Linux, woman.el does a _lot_
> >> of Windows/Cygwin centric stuff
> >
> > That's because Windows systems don't come with a `man' command
> > installed.
> 
> I don't understand.  woman does not use any `man' command on other
> operating systems either, so this is quite orthogonal to the operating
> system support.

Sorry for being unclear.  What I meant to say was that one of the main
motivations for having woman.el in the first place was the absence of
the `man' command from a typical Windows box.  So woman.el, at least
initially, was heavily biased towards MS-Windows users.

> One interesting question is what kind of manpath.config one can expect
> to be found on Cygwin systems when there is no actual `man' command
> using it.

Woman.el can be (and is) used even when `man' does exist.  Some of the
reasons are stated in the commentary section.

Of course, if `man' is absent, I wouldn't expect any manpath.config to
be present, either.  But I don't use Cygwin, so perhaps you should
wait for someone who does to chime in.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Fixes to let woman.el deal with MANPATH_MAP elements
  2007-03-23 18:00 ` Richard Stallman
  2007-03-24 15:09   ` David Kastrup
@ 2007-03-31 15:25   ` David Kastrup
  1 sibling, 0 replies; 13+ messages in thread
From: David Kastrup @ 2007-03-31 15:25 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

Richard Stallman <rms@gnu.org> writes:

> I think this is more of a bug fix than a new feature.
> If other people test it and verify it works, then you can
> check it in.

Since nobody complained so far, I checked this change in now.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2007-03-31 15:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-23 10:29 Fixes to let woman.el deal with MANPATH_MAP elements David Kastrup
2007-03-23 14:16 ` Eli Zaretskii
2007-03-23 18:34   ` David Kastrup
2007-03-23 20:03     ` David Kastrup
2007-03-23 20:12       ` Drew Adams
2007-03-23 20:25         ` David Kastrup
2007-03-23 21:09       ` David Kastrup
2007-03-23 18:00 ` Richard Stallman
2007-03-24 15:09   ` David Kastrup
2007-03-24 16:57     ` Eli Zaretskii
2007-03-24 17:07       ` David Kastrup
2007-03-24 17:39         ` Eli Zaretskii
2007-03-31 15:25   ` David Kastrup

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).