Re: conf-space-mode

[Wolfgang Jenkner <wjenkner@inode.at>]

Richard Stallman <rms@gnu.org> writes:

> I think I fixed the bug you reported and also got rid of the excess
> requests for confirmation.

The way you state this suggests that perhaps you forgot that I
actually sent a patch?

> Would you please test the changes I checked in?

You didn't address the problematic use of `current-prefix-arg'.

For ease of reference (and since you started a new thread) I include
here my original message `patch for conf-space-mode glitches' from
Mon, 04 Sep 2006 23:47:56 +0200:

Create a file (say, ./x) with the following content:

# -*-conf-*-

foo bar quux

# Local Variables:
# conf-space-keywords: "foo"
# End:

Visit it: C-x C-f x <return> y y

There is an inconvenience here, viz. that (by design) you are asked
twice about the same Local Variables.

Anyway, `foo' has the `font-lock-keyword-face' face text-property now
and is correctly highlighted.

Now type: C-u C-c SPC n f o o <return>

IIUC the result should be the same as above.  However `foo' has the
`font-lock-variable-name-face' face text-property instead (also, it
doesn't make sense to ask about the Local Variables here).

On the other hand,

C-u C-x C-v <return> n f o o <return> n

does give the expected result.  However, passing down like this the
prefix arg from a different command is quite problematic; for example,
`ffap-bindings' rebinds C-x C-f to `find-file-at-point', which also
plays tricks with `current-prefix-arg'.  In particular,

C-u C-x C-f / f t p . f o o . b a r : RET

surprises by asking for a "Regexp to match keywords: " (if you have a
valid ~/.netrc `ange-ftp-parse-netrc' inserts it in a buffer and calls
`normal-mode' etc.)

The following patch makes `conf-space-mode' behave like other modes,
while still not contradicting what is documented in the docstrings.
The disadvantage is that people who put some `conf-space-keywords'
dependent function on `conf-space-mode-hook' have to do some more work
now; see the comment at the end of the patch below.

Index: conf-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/textmodes/conf-mode.el,v
retrieving revision 1.14
diff -c -r1.14 conf-mode.el
*** conf-mode.el	6 Feb 2006 12:12:26 -0000	1.14
--- conf-mode.el	4 Sep 2006 19:37:58 -0000
***************
*** 190,195 ****
--- 190,199 ----
  	  '("^[ \t]*\\([^\000- ]+\\)" 1 'font-lock-variable-name-face)))
    "Keywords to hilight in Conf Space mode.")
  
+ (defvar conf-space-change-flag nil
+   "Holds what the last call to `conf-space-hack-keywords' returned.
+ Might be useful if you put more stuff on `hack-local-variables-hook'.")
+ 
  (defvar conf-colon-font-lock-keywords
    `(;; [section] (do this first because it may look like a parameter)
      ("^[ \t]*\\[\\(.+\\)\\]" 1 'font-lock-type-face)
***************
*** 467,498 ****
    (conf-mode-initialize "#" 'conf-space-font-lock-keywords)
    (set (make-local-variable 'conf-assignment-sign)
         nil)
!   ;; This doesn't seem right, but the next two depend on conf-space-keywords
!   ;; being set, while after-change-major-mode-hook might set up imenu, needing
!   ;; the following result:
!   (hack-local-variables-prop-line)
!   (hack-local-variables)
!   (cond (current-prefix-arg
!          (set (make-local-variable 'conf-space-keywords)
!               (if (> (prefix-numeric-value current-prefix-arg) 0)
!                   (read-string "Regexp to match keywords: "))))
!         (conf-space-keywords)
!         (buffer-file-name
!          (set (make-local-variable 'conf-space-keywords)
!               (assoc-default buffer-file-name conf-space-keywords-alist
!                              'string-match))))
!   (set (make-local-variable 'conf-assignment-regexp)
!        (if conf-space-keywords
! 	   (concat "\\(?:" conf-space-keywords "\\)[ \t]+.+?\\([ \t]+\\|$\\)")
! 	 ".+?\\([ \t]+\\|$\\)"))
!   (setq imenu-generic-expression
! 	`(,@(cdr imenu-generic-expression)
! 	  ("Parameters"
! 	   ,(if conf-space-keywords
! 		(concat "^[ \t]*\\(?:" conf-space-keywords
! 			"\\)[ \t]+\\([^ \t\n]+\\)\\(?:[ \t]\\|$\\)")
! 	      "^[ \t]*\\([^ \t\n[]+\\)\\(?:[ \t]\\|$\\)")
! 	   1))))
  
  ;;;###autoload
  (define-derived-mode conf-colon-mode conf-unix-mode "Conf[Colon]"
--- 471,533 ----
    (conf-mode-initialize "#" 'conf-space-font-lock-keywords)
    (set (make-local-variable 'conf-assignment-sign)
         nil)
!   (let ((arg (and (eq this-command 'conf-space-mode) current-prefix-arg)))
!     (cond (arg
! 	   (set (make-local-variable 'conf-space-keywords)
! 		(if (> (prefix-numeric-value current-prefix-arg) 0)
! 		    (read-string "Regexp to match keywords: "))))
! 	  (conf-space-keywords)		;A global value.  Useful?
! 	  (buffer-file-name
! 	   (set (make-local-variable 'conf-space-keywords)
! 		(assoc-default buffer-file-name conf-space-keywords-alist
! 			       'string-match))))
!     (conf-space-hack-keywords t)
!     (unless arg
!       (add-hook 'hack-local-variables-hook 'conf-space-hack-keywords nil t))))
! 
! 
! (defun conf-space-hack-keywords (&optional mode-flag)
!   "Initialize or adjust variables depending on `conf-space-keywords'.
! Returns non-nil iff MODE-FLAG is nil and something actually changed."
!   (when (or mode-flag (eq major-mode 'conf-space-mode))
!     (let ((regexp (if conf-space-keywords
! 		      (concat "\\(?:" conf-space-keywords
! 			      "\\)[ \t]+.+?\\([ \t]+\\|$\\)")
! 		    ".+?\\([ \t]+\\|$\\)")))
!       (set (make-local-variable 'conf-space-change-flag)
! 	   (when (or mode-flag (not (string= conf-assignment-regexp regexp)))
! 	     (set (make-local-variable 'conf-assignment-regexp) regexp)
! 	     ;; Take care of `(eval ...)' in `conf-space-font-lock-keywords'.
! 	     (when (and font-lock-mode
! 			(boundp 'font-lock-keywords)) ;see `normal-mode'
! 	       (font-lock-add-keywords nil nil)
! 	       (font-lock-mode 1))
! 	     (setq imenu-generic-expression
! 		   `(,@(remq (assoc "Parameters" imenu-generic-expression)
! 			     imenu-generic-expression)
! 		     ("Parameters"
! 		      ,(if conf-space-keywords
! 			   (concat "^[ \t]*\\(?:" conf-space-keywords
! 				   "\\)[ \t]+\\([^ \t\n]+\\)\\(?:[ \t]\\|$\\)")
! 			 "^[ \t]*\\([^ \t\n[]+\\)\\(?:[ \t]\\|$\\)")
! 		      1)))
! 	     (not mode-flag))))))
! 
! ;; A comment in the original `conf-space-mode' states: [...]
! ;; `after-change-major-mode-hook' might set up imenu, needing [some
! ;; stuff depending on `conf-space-keywords'].
! 
! ;; The following snippet makes sure 'after-change-major-mode-hook' is
! ;; run at an appropriate time, though I think it would be cleaner to
! ;; run specific functions instead of the whole hook.
! 
! ;; (add-hook 'conf-space-mode-hook
! ;;	  (lambda ()
! ;;	    (add-hook 'hack-local-variables-hook
! ;;		      (lambda ()
! ;;			(when conf-space-change-flag
! ;;			  (run-hooks 'after-change-major-mode-hook))) ;Yuck!
! ;;		      t t)))
  
  ;;;###autoload
  (define-derived-mode conf-colon-mode conf-unix-mode "Conf[Colon]"