unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* patch for woman (woman-topic-at-point)
@ 2005-08-25 18:48 Emilio Lopes
  2005-08-25 19:51 ` David Kastrup
  2005-08-26 12:03 ` Richard M. Stallman
  0 siblings, 2 replies; 17+ messages in thread
From: Emilio Lopes @ 2005-08-25 18:48 UTC (permalink / raw)


[ Sorry for the long message.  I wanted to make the problem clear
  also for people not familiar with `woman'. ]

The variable `woman-topic-at-point' controls whether the command
`woman' should offer the word at point as a suggestion when asking the
user for the name of a manual page.

The default value is `confirm' and being neither t nor nil means to
use the word at point as a suggestion but give the user the option to
reject or confirm this suggestion.

The problem is that "to suggest" for `woman' means to actually
*insert* the suggested name in the minibuffer, a behavior described
as deprecated in the documentation of `completing-read'.

And because the word at point rarely (at least for me) is the name of a
man page actually installed in the system, this means that most of the
time the user has the burden of having to delete the useless suggestion
before entering the desired man page.  I find this annoying.

The following patch introduces a new possible value for
`woman-topic-at-point': if this variable is the symbol `exact',
`woman' will offer the word at point as a default ONLY IF it actually
matches the name of an existing man page in the system.  Even in this
case it will *not* insert the suggestion in the minibuffer.

This change is backwards compatible.

If I could only change the behavior of *any* woman with a patch :-)


2005-08-19  Emilio C. Lopes  <eclig@gmx.net>

	* woman.el (woman-topic-at-point-default, woman-topic-at-point):
	new possible value `exact'.
	(woman-file-name): support for this new value of
	`woman-topic-at-point'.


diff -rN -c old-emacs-darcs.eclig/lisp/woman.el new-emacs-darcs.eclig/lisp/woman.el
*** old-emacs-darcs.eclig/lisp/woman.el	Thu Aug 25 19:59:24 2005
--- new-emacs-darcs.eclig/lisp/woman.el	Fri Aug 19 18:07:36 2005
***************
*** 144,151 ****
  ;; topic must be confirmed or edited in the minibuffer.  This
  ;; suggestion can be turned off, or `woman' can use the suggested
  ;; topic without confirmation* if possible, by setting the user-option
! ;; `woman-topic-at-point' to nil or t respectively.  (Its default
! ;; value is neither nil nor t, meaning ask for confirmation.)
  
  ;; [* Thanks to Benjamin Riefenstahl for suggesting this
  ;; functionality.]
--- 144,153 ----
  ;; topic must be confirmed or edited in the minibuffer.  This
  ;; suggestion can be turned off, or `woman' can use the suggested
  ;; topic without confirmation* if possible, by setting the user-option
! ;; `woman-topic-at-point' to nil or t respectively.  If it is the
! ;; symbol `exact' then the word at point will be offered as a
! ;; suggestion only if it is an exact match for an existing topic.
! ;; (Its default value is none of these, meaning ask for confirmation.)
  
  ;; [* Thanks to Benjamin Riefenstahl for suggesting this
  ;; functionality.]
***************
*** 422,430 ****
  ;;   Geoff Voelker <voelker@cs.washington.edu>
  ;;   Eli Zaretskii <eliz@is.elta.co.il>
  
- ;;; History:
- ;;  For recent change log see end of file.
- 
  \f
  ;;; Code:
  
--- 424,429 ----
***************
*** 721,738 ****
    "*Default value for `woman-topic-at-point'."
    :type '(choice (const :tag "Yes" t)
  		 (const :tag "No" nil)
  		 (other :tag "Confirm" confirm))
    :group 'woman-interface)
  
  (defcustom woman-topic-at-point woman-topic-at-point-default
    "*Controls use by `woman' of `word at point' as a topic suggestion.
! If non-nil then the `woman' command uses the word at point as an
! initial topic suggestion when it reads a topic from the minibuffer; if
! t then the `woman' command uses the word at point WITHOUT
! INTERACTIVE CONFIRMATION if it exists as a topic.  The default value
! is `confirm', meaning suggest a topic and ask for confirmation."
    :type '(choice (const :tag "Yes" t)
  		 (const :tag "No" nil)
  		 (other :tag "Confirm" confirm))
    :group 'woman-interface)
  
--- 720,743 ----
    "*Default value for `woman-topic-at-point'."
    :type '(choice (const :tag "Yes" t)
  		 (const :tag "No" nil)
+                  (const :tag "Exact" exact)
  		 (other :tag "Confirm" confirm))
    :group 'woman-interface)
  
  (defcustom woman-topic-at-point woman-topic-at-point-default
    "*Controls use by `woman' of `word at point' as a topic suggestion.
! If t then the `woman' command uses the word at point WITHOUT
! INTERACTIVE CONFIRMATION if it exists as a topic.
! If it is the symbol `exact' then the word at point will be offered
! as a suggestion only if it is an exact match for an existing topic.
! If it has other non-nil value then the `woman' command uses the word
! at point as an initial topic suggestion when it reads a topic from
! the minibuffer.
! The default value is `confirm', meaning suggest a topic and ask
! for confirmation."
    :type '(choice (const :tag "Yes" t)
  		 (const :tag "No" nil)
+                  (const :tag "Exact" exact)
  		 (other :tag "Confirm" confirm))
    :group 'woman-interface)
  
***************
*** 956,963 ****
      :group 'woman-faces)
  
    (defcustom woman-use-symbol-font nil
!     "*If non-nil then may use the symbol font.  It is off by default,
! mainly because it may change the line spacing (in NTEmacs 20.5)."
      :type 'boolean
      :group 'woman-faces)
  
--- 961,969 ----
      :group 'woman-faces)
  
    (defcustom woman-use-symbol-font nil
!     "*If non-nil then may use the symbol font.
! It is off by default,mainly because it may change the line spacing
! \(in NTEmacs 20.5)."
      :type 'boolean
      :group 'woman-faces)
  
***************
*** 1229,1245 ****
  		   ;; Was let-bound when file loaded, so ...
  		   (setq woman-topic-at-point woman-topic-at-point-default)))
  	     (setq topic
! 		   (or (current-word t) ""))	; only within or adjacent to word
  	     (assoc topic woman-topic-all-completions))
  	(setq topic
! 	      (completing-read
! 	       "Manual entry: "
! 	       woman-topic-all-completions nil 1
! 	       ;; Initial input suggestion (was nil), with
! 	       ;; cursor at left ready to kill suggestion!:
! 	       (and woman-topic-at-point
! 		    (cons (or (current-word) "") 0)) ; nearest word
! 	       'woman-topic-history)))
      ;; Note that completing-read always returns a string.
      (if (= (length topic) 0)
  	nil				; no topic, so no file!
--- 1235,1266 ----
  		   ;; Was let-bound when file loaded, so ...
  		   (setq woman-topic-at-point woman-topic-at-point-default)))
  	     (setq topic
! 		   (or (current-word t) "")) ; only within or adjacent to word
  	     (assoc topic woman-topic-all-completions))
  	(setq topic
!               (let (initial default)
!                 (cond
!                  ((eq woman-topic-at-point 'exact)
!                   (setq initial nil)
!                   (setq default (and (current-word)
!                                      (car (assoc (current-word)
!                                                  woman-topic-all-completions)))))
!                  (woman-topic-at-point  ; any other non-nil value
!                   ;; Initial input suggestion, with cursor at left
!                   ;; ready to kill suggestion!
!                   (setq initial (cons (or (current-word) "") 0))
!                   (setq default nil))
!                  (t
!                   (setq initial nil)
!                   (setq default nil)))
!                 (completing-read
!                  (if default
!                      (format "Manual entry [default: %s]: " default)
!                    "Manual entry: ")
!                  woman-topic-all-completions nil 1
!                  initial
!                  'woman-topic-history
!                  default))))
      ;; Note that completing-read always returns a string.
      (if (= (length topic) 0)
  	nil				; no topic, so no file!
***************
*** 1259,1268 ****
  	;; Unread the command event (TAB = ?\t = 9) that runs the command
  	;; `minibuffer-complete' in order to automatically complete the
  	;; minibuffer contents as far as possible.
! 	(setq unread-command-events '(9))	; and delete any type-ahead!
  	(completing-read "Manual file: " files nil 1
! 			 (try-completion "" files) 'woman-file-history)))
!       )))
  
  (defun woman-select (predicate list)
    "Select unique elements for which PREDICATE is true in LIST.
--- 1280,1288 ----
  	;; Unread the command event (TAB = ?\t = 9) that runs the command
  	;; `minibuffer-complete' in order to automatically complete the
  	;; minibuffer contents as far as possible.
! 	(setq unread-command-events '(9)) ; and delete any type-ahead!
  	(completing-read "Manual file: " files nil 1
! 			 (try-completion "" files) 'woman-file-history))))))
  
  (defun woman-select (predicate list)
    "Select unique elements for which PREDICATE is true in LIST.

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-25 18:48 patch for woman (woman-topic-at-point) Emilio Lopes
@ 2005-08-25 19:51 ` David Kastrup
  2005-08-26 12:03 ` Richard M. Stallman
  1 sibling, 0 replies; 17+ messages in thread
From: David Kastrup @ 2005-08-25 19:51 UTC (permalink / raw)


Emilio Lopes <eclig@gmx.net> writes:

> [ Sorry for the long message.  I wanted to make the problem clear
>   also for people not familiar with `woman'. ]

Most hackers, I take?

For a moment there I thought you had a patch that you could put on a
woman, and it would make her come right to the topic at point without
attempting any course of action that requires an advance course in
divination.

There'd be quite a sensational market for that, you know.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-25 18:48 patch for woman (woman-topic-at-point) Emilio Lopes
  2005-08-25 19:51 ` David Kastrup
@ 2005-08-26 12:03 ` Richard M. Stallman
  2005-08-26 15:21   ` Emilio Lopes
                     ` (2 more replies)
  1 sibling, 3 replies; 17+ messages in thread
From: Richard M. Stallman @ 2005-08-26 12:03 UTC (permalink / raw)
  Cc: Francis J. Wright, emacs-devel

    The problem is that "to suggest" for `woman' means to actually
    *insert* the suggested name in the minibuffer, a behavior described
    as deprecated in the documentation of `completing-read'.

That is an unnecessarily complex solution.  It would be better
just to change *how* that name is offered.  Instead of inserting
it in the minibuffer, make it the default, more or less as below.

This could be an unconditional change, so it would not require any
options.  However, it might also be desirable to check (perhaps as an
option) whether the name is valid before offering it as the default.

Your diff includes some other miscellanious fixes, such as breaking
a line in a doc string, which might be good to install independent
of this.

*** woman.el	07 Aug 2005 13:30:28 -0400	1.33
--- woman.el	26 Aug 2005 06:02:31 -0400	
***************
*** 1235,1245 ****
  	      (completing-read
  	       "Manual entry: "
  	       woman-topic-all-completions nil 1
  	       ;; Initial input suggestion (was nil), with
  	       ;; cursor at left ready to kill suggestion!:
  	       (and woman-topic-at-point
  		    (cons (or (current-word) "") 0)) ; nearest word
! 	       'woman-topic-history)))
      ;; Note that completing-read always returns a string.
      (if (= (length topic) 0)
  	nil				; no topic, so no file!
--- 1235,1247 ----
  	      (completing-read
  	       "Manual entry: "
  	       woman-topic-all-completions nil 1
+ 	       nil
+ 	       'woman-topic-history
  	       ;; Initial input suggestion (was nil), with
  	       ;; cursor at left ready to kill suggestion!:
  	       (and woman-topic-at-point
  		    (cons (or (current-word) "") 0)) ; nearest word
! )))
      ;; Note that completing-read always returns a string.
      (if (= (length topic) 0)
  	nil				; no topic, so no file!

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-26 12:03 ` Richard M. Stallman
@ 2005-08-26 15:21   ` Emilio Lopes
  2005-08-26 18:05     ` Stefan Monnier
  2005-08-28  2:45     ` Richard M. Stallman
  2005-08-26 15:23   ` Emilio Lopes
  2005-08-26 16:25   ` Dr Francis J Wright
  2 siblings, 2 replies; 17+ messages in thread
From: Emilio Lopes @ 2005-08-26 15:21 UTC (permalink / raw)


Richard M Stallman writes:

>     The problem is that "to suggest" for `woman' means to actually
>     *insert* the suggested name in the minibuffer, a behavior
>     described as deprecated in the documentation of
>     `completing-read'.

> That is an unnecessarily complex solution.  It would be better just
> to change *how* that name is offered.  Instead of inserting it in
> the minibuffer, make it the default, more or less as below.

I wanted to do it this way too initially.  It didn't work because:

   1- The code following `completing-read' expects the result from
      that function to be an existing man page.  I think this is a
      legitimate assumption.

   2- The argument REQUIRE-MATCH in `completing-read', currently used
      to satisfy item 1 above, has no effect on the argument DEF,
      the default value offered.  It affects only the argument
      INITIAL-INPUT.

Item 2 means that if the user just press <enter> at the prompt
`completing-read' will happily return DEF whether or not it matches
an item in TABLE.  This would break the assumption in item 1.

It follows from the above that if you want to offer some suggestion using
the argument DEF of `completing-read' the suggestion must be "valid".
Otherwise you'll break the code following the `completing-read'.  This is
accomplished by the patches I sent.

Yes, the patches make the code somewhat more complex, but that's
because I wanted it to be backwards compatible.  *I* would just remove
the old behavior, since it makes no sense for me to suggest man pages
which do not exist.

The following patch implements this simpler and more sensible behavior.
It is *not* backwards compatible.  To make that clear, I renamed the
variable `woman-topic-at-point' to `woman-use-topic-at-point'.


2005-08-26  Emilio C. Lopes  <eclig@gmx.net>

	* woman.el (woman-topic-at-point-default): renamed to
	woman-use-topic-at-point-default.
	(woman-topic-at-point): renamed to woman-use-topic-at-point.
	(woman-file-name): reflect renames above.  Automatically use the
	word at point as topic if woman-use-topic-at-point is non-nil.
	Otherwise offer it as default but don't insert it in the
	minibuffer.
  
diff -rN -c old-emacs-darcs.eclig/lisp/woman.el new-emacs-darcs.eclig/lisp/woman.el
*** old-emacs-darcs.eclig/lisp/woman.el	Fri Aug 26 17:11:11 2005
--- new-emacs-darcs.eclig/lisp/woman.el	Fri Aug 26 16:54:51 2005
***************
*** 136,162 ****
  ;;   man man_page_name
  
  
! ;; Using the `word at point' as a topic suggestion
! ;; ===============================================
  
! ;; By default, the `woman' command uses the word nearest to point in
! ;; the current buffer as a suggestion for the topic to look up.  The
! ;; topic must be confirmed or edited in the minibuffer.  This
! ;; suggestion can be turned off, or `woman' can use the suggested
! ;; topic without confirmation* if possible, by setting the user-option
! ;; `woman-topic-at-point' to nil or t respectively.  (Its default
! ;; value is neither nil nor t, meaning ask for confirmation.)
  
! ;; [* Thanks to Benjamin Riefenstahl for suggesting this
! ;; functionality.]
! 
! ;; The variable `woman-topic-at-point' can be rebound locally, which
! ;; may be useful to provide special private key bindings, e.g.
  
  ;;  (global-set-key "\C-cw"
  ;;  		  (lambda ()
  ;;  		    (interactive)
! ;;  		    (let ((woman-topic-at-point t))
  ;;  		      (woman)))))
  
  
--- 136,158 ----
  ;;   man man_page_name
  
  
! ;; Using the word at point as the default topic
! ;; ============================================
  
! ;; The `woman' command uses the word nearest to point in the current
! ;; buffer as the default topic to look up if it matches the name of a
! ;; manual page installed on the system.  The default topic can also be
! ;; used without confirmation by setting the user-option
! ;; `woman-use-topic-at-point' to t; thanks to Benjamin Riefenstahl for
! ;; suggesting this functionality.
  
! ;; The variable `woman-use-topic-at-point' can be rebound locally,
! ;; which may be useful to provide special private key bindings, e.g.
  
  ;;  (global-set-key "\C-cw"
  ;;  		  (lambda ()
  ;;  		    (interactive)
! ;;  		    (let ((woman-use-topic-at-point t))
  ;;  		      (woman)))))
  
  
***************
*** 711,736 ****
    :type 'string
    :group 'woman-interface)
  
! (defcustom woman-topic-at-point-default 'confirm
!   ;; `woman-topic-at-point' may be let-bound when woman is loaded, in
!   ;; which case its global value does not get defined.
    ;; `woman-file-name' sets it to this value if it is unbound.
!   "*Default value for `woman-topic-at-point'."
    :type '(choice (const :tag "Yes" t)
! 		 (const :tag "No" nil)
! 		 (other :tag "Confirm" confirm))
    :group 'woman-interface)
  
! (defcustom woman-topic-at-point woman-topic-at-point-default
!   "*Controls use by `woman' of `word at point' as a topic suggestion.
! If non-nil then the `woman' command uses the word at point as an
! initial topic suggestion when it reads a topic from the minibuffer; if
! t then the `woman' command uses the word at point WITHOUT
! INTERACTIVE CONFIRMATION if it exists as a topic.  The default value
! is `confirm', meaning suggest a topic and ask for confirmation."
    :type '(choice (const :tag "Yes" t)
! 		 (const :tag "No" nil)
! 		 (other :tag "Confirm" confirm))
    :group 'woman-interface)
  
  (defvar woman-file-regexp nil
--- 707,727 ----
    :type 'string
    :group 'woman-interface)
  
! (defcustom woman-use-topic-at-point-default nil
!   ;; `woman-use-topic-at-point' may be let-bound when woman is loaded,
!   ;; in which case its global value does not get defined.
    ;; `woman-file-name' sets it to this value if it is unbound.
!   "*Default value for `woman-use-topic-at-point'."
    :type '(choice (const :tag "Yes" t)
! 		 (const :tag "No" nil))
    :group 'woman-interface)
  
! (defcustom woman-use-topic-at-point woman-use-topic-at-point-default
!   "*Control use of the word at point as the default topic.
! If non-nil the `woman' command uses the word at point automatically,
! without interactive confirmation, if it exists as a topic."
    :type '(choice (const :tag "Yes" t)
! 		 (const :tag "No" nil))
    :group 'woman-interface)
  
  (defvar woman-file-regexp nil
***************
*** 1198,1207 ****
  
  (defun woman-file-name (topic &optional re-cache)
    "Get the name of the UN*X man-page file describing a chosen TOPIC.
! When `woman' is called interactively, the word at point may be used as
! the topic or initial topic suggestion, subject to the value of the
! user option `woman-topic-at-point'.  Return nil if no file can be found.
! Optional argument RE-CACHE, if non-nil, forces the cache to be re-read."
    ;; Handle the caching of the directory and topic lists:
    (if (and (not re-cache)
  	   (or
--- 1189,1199 ----
  
  (defun woman-file-name (topic &optional re-cache)
    "Get the name of the UN*X man-page file describing a chosen TOPIC.
! When `woman' is called interactively, the word at point may be
! automatically used as the topic, if the value of the user option
! `woman-use-topic-at-point' is non-nil.  Return nil if no file can
! be found.  Optional argument RE-CACHE, if non-nil, forces the
! cache to be re-read."
    ;; Handle the caching of the directory and topic lists:
    (if (and (not re-cache)
  	   (or
***************
*** 1221,1243 ****
    ;; completion if necessary.
    (let (files)
      (or (stringp topic)
! 	(and (eq t
! 		 (if (boundp 'woman-topic-at-point)
! 		     woman-topic-at-point
! 		   ;; Was let-bound when file loaded, so ...
! 		   (setq woman-topic-at-point woman-topic-at-point-default)))
! 	     (setq topic
! 		   (or (current-word t) ""))	; only within or adjacent to word
  	     (assoc topic woman-topic-all-completions))
  	(setq topic
! 	      (completing-read
! 	       "Manual entry: "
! 	       woman-topic-all-completions nil 1
! 	       ;; Initial input suggestion (was nil), with
! 	       ;; cursor at left ready to kill suggestion!:
! 	       (and woman-topic-at-point
! 		    (cons (or (current-word) "") 0)) ; nearest word
! 	       'woman-topic-history)))
      ;; Note that completing-read always returns a string.
      (if (= (length topic) 0)
  	nil				; no topic, so no file!
--- 1213,1236 ----
    ;; completion if necessary.
    (let (files)
      (or (stringp topic)
! 	(and (if (boundp 'woman-use-topic-at-point)
!                  woman-use-topic-at-point
!                ;; Was let-bound when file loaded, so ...
!                (setq woman-use-topic-at-point woman-use-topic-at-point-default))
! 	     (setq topic (or (current-word t) "")) ; only within or adjacent to word
  	     (assoc topic woman-topic-all-completions))
  	(setq topic
!               (let ((default (and (current-word)
!                                   (car (assoc (current-word)
!                                               woman-topic-all-completions)))))
!                 (completing-read
!                  (if default
!                      (format "Manual entry [default: %s]: " default)
!                    "Manual entry: ")
!                  woman-topic-all-completions nil 1
!                  nil
!                  'woman-topic-history
!                  default))))
      ;; Note that completing-read always returns a string.
      (if (= (length topic) 0)
  	nil				; no topic, so no file!

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-26 12:03 ` Richard M. Stallman
  2005-08-26 15:21   ` Emilio Lopes
@ 2005-08-26 15:23   ` Emilio Lopes
  2005-08-26 16:25   ` Dr Francis J Wright
  2 siblings, 0 replies; 17+ messages in thread
From: Emilio Lopes @ 2005-08-26 15:23 UTC (permalink / raw)


Richard M Stallman writes:

> Your diff includes some other miscellanious fixes, such as breaking
> a line in a doc string, which might be good to install independent
> of this.

I've just sent only those miscellaneous changes in another separate
message.

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

* RE: patch for woman (woman-topic-at-point)
  2005-08-26 12:03 ` Richard M. Stallman
  2005-08-26 15:21   ` Emilio Lopes
  2005-08-26 15:23   ` Emilio Lopes
@ 2005-08-26 16:25   ` Dr Francis J Wright
  2 siblings, 0 replies; 17+ messages in thread
From: Dr Francis J Wright @ 2005-08-26 16:25 UTC (permalink / raw)
  Cc: emacs-devel

I prefer Richard's proposal as below.

Breaking the line in the doc string for woman-use-symbol-font may well be a
good idea, but I would prefer to retain some white space after the comma.

Richard: Thanks for sending me the full context separately.

Best wishes, Francis

> -----Original Message-----
> From: Richard M. Stallman [mailto:rms@gnu.org] 
> Sent: Friday 26 August 2005 1:03 pm
> To: Emilio Lopes
> Cc: emacs-devel@gnu.org; Francis J. Wright
> Subject: Re: patch for woman (woman-topic-at-point)
> 
>     The problem is that "to suggest" for `woman' means to actually
>     *insert* the suggested name in the minibuffer, a behavior 
> described
>     as deprecated in the documentation of `completing-read'.
> 
> That is an unnecessarily complex solution.  It would be 
> better just to change *how* that name is offered.  Instead of 
> inserting it in the minibuffer, make it the default, more or 
> less as below.
> 
> This could be an unconditional change, so it would not 
> require any options.  However, it might also be desirable to 
> check (perhaps as an
> option) whether the name is valid before offering it as the default.
> 
> Your diff includes some other miscellanious fixes, such as 
> breaking a line in a doc string, which might be good to 
> install independent of this.
> 
> *** woman.el	07 Aug 2005 13:30:28 -0400	1.33
> --- woman.el	26 Aug 2005 06:02:31 -0400	
> ***************
> *** 1235,1245 ****
>   	      (completing-read
>   	       "Manual entry: "
>   	       woman-topic-all-completions nil 1
>   	       ;; Initial input suggestion (was nil), with
>   	       ;; cursor at left ready to kill suggestion!:
>   	       (and woman-topic-at-point
>   		    (cons (or (current-word) "") 0)) ; nearest word
> ! 	       'woman-topic-history)))
>       ;; Note that completing-read always returns a string.
>       (if (= (length topic) 0)
>   	nil				; no topic, so no file!
> --- 1235,1247 ----
>   	      (completing-read
>   	       "Manual entry: "
>   	       woman-topic-all-completions nil 1
> + 	       nil
> + 	       'woman-topic-history
>   	       ;; Initial input suggestion (was nil), with
>   	       ;; cursor at left ready to kill suggestion!:
>   	       (and woman-topic-at-point
>   		    (cons (or (current-word) "") 0)) ; nearest 
> word ! )))
>       ;; Note that completing-read always returns a string.
>       (if (= (length topic) 0)
>   	nil				; no topic, so no file!
> 

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-26 15:21   ` Emilio Lopes
@ 2005-08-26 18:05     ` Stefan Monnier
  2005-08-27 18:57       ` Emilio Lopes
  2005-08-28  2:45     ` Richard M. Stallman
  1 sibling, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2005-08-26 18:05 UTC (permalink / raw)
  Cc: emacs-devel

> !               (let ((default (and (current-word)
> !                                   (car (assoc (current-word)
> !                                               woman-topic-all-completions)))))

Instead of assoc, I'd recommend you use test-completion.  It's probably
equivalent given the current code, but it makes the intention much more
clear and will work if the completion table is ever changed to some
other format.


        Stefan

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-26 18:05     ` Stefan Monnier
@ 2005-08-27 18:57       ` Emilio Lopes
  2005-08-28 19:35         ` David Kastrup
  0 siblings, 1 reply; 17+ messages in thread
From: Emilio Lopes @ 2005-08-27 18:57 UTC (permalink / raw)


Stefan Monnier writes:

> Instead of assoc, I'd recommend you use test-completion.  It's
> probably equivalent given the current code, but it makes the
> intention much more clear and will work if the completion table is
> ever changed to some other format.

Thanks for the suggestion.  I changed my local copy and will send a
new version of the patch if there's interest on the changes.

BTW, is there a more elegant way of writing the following?  I have the
feeling I'm missing obvious something here...

   (and word-at-point
       (test-completion word-at-point woman-topic-all-completions)
       word-at-point)

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-26 15:21   ` Emilio Lopes
  2005-08-26 18:05     ` Stefan Monnier
@ 2005-08-28  2:45     ` Richard M. Stallman
  2005-09-02 11:11       ` Dr Francis J Wright
  1 sibling, 1 reply; 17+ messages in thread
From: Richard M. Stallman @ 2005-08-28  2:45 UTC (permalink / raw)
  Cc: Dr Francis J Wright, emacs-devel

    It follows from the above that if you want to offer some suggestion using
    the argument DEF of `completing-read' the suggestion must be "valid".

Ok, so test it for validity and provide it as a default only if it
is valid.

I see that's what your latest patch does.  I have not checked it line
by line, but it looks like the right overall.  Francis, do you agree?

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-27 18:57       ` Emilio Lopes
@ 2005-08-28 19:35         ` David Kastrup
  2005-08-29 16:08           ` Drew Adams
  2005-08-29 18:08           ` Emilio Lopes
  0 siblings, 2 replies; 17+ messages in thread
From: David Kastrup @ 2005-08-28 19:35 UTC (permalink / raw)
  Cc: emacs-devel

Emilio Lopes <eclig@gmx.net> writes:

> Stefan Monnier writes:
>
>> Instead of assoc, I'd recommend you use test-completion.  It's
>> probably equivalent given the current code, but it makes the
>> intention much more clear and will work if the completion table is
>> ever changed to some other format.
>
> Thanks for the suggestion.  I changed my local copy and will send a
> new version of the patch if there's interest on the changes.
>
> BTW, is there a more elegant way of writing the following?  I have the
> feeling I'm missing obvious something here...
>
>    (and word-at-point
>        (test-completion word-at-point woman-topic-all-completions)
>        word-at-point)

(when (and word-at-point
           (test-completion word-at-point woman-topic-all-completions))
  word-at-point)

It's more verbose, but brings across the purpose somewhat better.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

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

* RE: patch for woman (woman-topic-at-point)
  2005-08-28 19:35         ` David Kastrup
@ 2005-08-29 16:08           ` Drew Adams
  2005-08-30 16:13             ` Emilio Lopes
  2005-08-29 18:08           ` Emilio Lopes
  1 sibling, 1 reply; 17+ messages in thread
From: Drew Adams @ 2005-08-29 16:08 UTC (permalink / raw)


    > is there a more elegant way of writing the following?
    >    (and word-at-point
    >        (test-completion word-at-point woman-topic-all-completions)
    >        word-at-point)

    (when (and word-at-point
               (test-completion word-at-point woman-topic-all-completions))
      word-at-point)
    It's more verbose, but brings across the purpose somewhat better.

Why don't we let `test-completion' return the completion (string) argument,
whenever the return value is non-nil?

This would then be simply:
 (and word-at-point
      (test-completion word-at-point woman-topic-all-completions))

This is a common use case - most uses of `test-completion' will want to use
the string, but only if it is a completion. And a return value of `t' is not
particularly useful.

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-28 19:35         ` David Kastrup
  2005-08-29 16:08           ` Drew Adams
@ 2005-08-29 18:08           ` Emilio Lopes
  2005-08-29 20:56             ` Stefan Monnier
  1 sibling, 1 reply; 17+ messages in thread
From: Emilio Lopes @ 2005-08-29 18:08 UTC (permalink / raw)


David Kastrup writes:

> Emilio Lopes <eclig@gmx.net> writes:
>> BTW, is there a more elegant way of writing the following?  I have
>> the feeling I'm missing obvious something here...
>>
>> (and word-at-point (test-completion word-at-point
>> woman-topic-all-completions) word-at-point)

> (when (and word-at-point
>            (test-completion word-at-point
>            woman-topic-all-completions))
>   word-at-point)

> It's more verbose, but brings across the purpose somewhat better.

OTOH at a first glance *I* don't expect a control structures such as
`when' to return any useful value when the conditional clause fails.
But maybe I've been doing too much Scheme these days. (no, surely not
;-)

I'll use your suggestion, thanks.

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-29 18:08           ` Emilio Lopes
@ 2005-08-29 20:56             ` Stefan Monnier
  2005-08-29 22:15               ` Thien-Thi Nguyen
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2005-08-29 20:56 UTC (permalink / raw)
  Cc: emacs-devel

> OTOH at a first glance *I* don't expect a control structures such as
> `when' to return any useful value when the conditional clause fails.
> But maybe I've been doing too much Scheme these days. (no, surely not

You're just suffering from one of the many places where Scheme is
too imperative.  For once, Elisp is more functional in this case.


        Stefan

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-29 20:56             ` Stefan Monnier
@ 2005-08-29 22:15               ` Thien-Thi Nguyen
  2005-08-31  2:28                 ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Thien-Thi Nguyen @ 2005-08-29 22:15 UTC (permalink / raw)
  Cc: Emilio Lopes, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > OTOH at a first glance *I* don't expect a control structures such as
> > `when' to return any useful value when the conditional clause fails.
> > But maybe I've been doing too much Scheme these days. (no, surely not
> 
> You're just suffering from one of the many places where Scheme is
> too imperative.  For once, Elisp is more functional in this case.

one-armed `if' when the condition is false has unspecified value.
that's not "too imperative", just "underspecified" (for some tastes).
see info node: "(r5rs) Conditionals".

furthermore, scheme doesn't (usually) have `when', so schemers should
probably take that as a hint to read the elisp docs on `when' and
compare it to their implementation's docs on `when'.

anyway, i suppose it's fair to say schemers suffer for their cause, in
one way or another (but always unportably :-)...

thi

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-29 16:08           ` Drew Adams
@ 2005-08-30 16:13             ` Emilio Lopes
  0 siblings, 0 replies; 17+ messages in thread
From: Emilio Lopes @ 2005-08-30 16:13 UTC (permalink / raw)


Drew Adams writes:

> Why don't we let `test-completion' return the completion (string)
> argument, whenever the return value is non-nil?

I could imagine this is for consistency with `try-completion', which
returns a string with the longest string common to all matches or t if
the given string was already an exact match.

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

* Re: patch for woman (woman-topic-at-point)
  2005-08-29 22:15               ` Thien-Thi Nguyen
@ 2005-08-31  2:28                 ` Stefan Monnier
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Monnier @ 2005-08-31  2:28 UTC (permalink / raw)
  Cc: Emilio Lopes, emacs-devel

>> > OTOH at a first glance *I* don't expect a control structures such as
>> > `when' to return any useful value when the conditional clause fails.
>> > But maybe I've been doing too much Scheme these days. (no, surely not
>> 
>> You're just suffering from one of the many places where Scheme is
>> too imperative.  For once, Elisp is more functional in this case.

> one-armed `if' when the condition is false has unspecified value.
> that's not "too imperative", just "underspecified" (for some tastes).
> see info node: "(r5rs) Conditionals".

Since it's unspecified, you can't rely on its value, so you end up using
that form in an imperative style.  I.e. "too" imperative.


        Stefan

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

* RE: patch for woman (woman-topic-at-point)
  2005-08-28  2:45     ` Richard M. Stallman
@ 2005-09-02 11:11       ` Dr Francis J Wright
  0 siblings, 0 replies; 17+ messages in thread
From: Dr Francis J Wright @ 2005-09-02 11:11 UTC (permalink / raw)
  Cc: emacs-devel

> From: Richard M. Stallman [mailto:rms@gnu.org] 
> Sent: Sunday 28 August 2005 3:45 am
> To: Emilio Lopes
> Cc: Dr Francis J Wright; emacs-devel@gnu.org
> Subject: Re: patch for woman (woman-topic-at-point)
> 
>     It follows from the above that if you want to offer some 
> suggestion using
>     the argument DEF of `completing-read' the suggestion must 
> be "valid".
> 
> Ok, so test it for validity and provide it as a default only 
> if it is valid.
> 
> I see that's what your latest patch does.  I have not checked 
> it line by line, but it looks like the right overall.  
> Francis, do you agree?


I haven't checked the details either, but I'm happy with the principle of
this patch.

Francis

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

end of thread, other threads:[~2005-09-02 11:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-25 18:48 patch for woman (woman-topic-at-point) Emilio Lopes
2005-08-25 19:51 ` David Kastrup
2005-08-26 12:03 ` Richard M. Stallman
2005-08-26 15:21   ` Emilio Lopes
2005-08-26 18:05     ` Stefan Monnier
2005-08-27 18:57       ` Emilio Lopes
2005-08-28 19:35         ` David Kastrup
2005-08-29 16:08           ` Drew Adams
2005-08-30 16:13             ` Emilio Lopes
2005-08-29 18:08           ` Emilio Lopes
2005-08-29 20:56             ` Stefan Monnier
2005-08-29 22:15               ` Thien-Thi Nguyen
2005-08-31  2:28                 ` Stefan Monnier
2005-08-28  2:45     ` Richard M. Stallman
2005-09-02 11:11       ` Dr Francis J Wright
2005-08-26 15:23   ` Emilio Lopes
2005-08-26 16:25   ` Dr Francis J Wright

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).