finder.el patch

finder.el patch

[Drew Adams]

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

This patch fixes two bugs in finder-exit:

- It treats buffer *Finder-package* also.
  This buffer is used by `finder-commentary'.

- Instead trying to avoid `delete-window' for
  `one-window-p', it wraps it in `condition-case'.

[-- Attachment #2: finder-2008-03-12a.patch --]
[-- Type: application/octet-stream, Size: 1078 bytes --]

diff -u -w finder-CVS-2008-03-12.el finder-patched-2008-03-12.el
--- finder-CVS-2008-03-12.el	2008-03-12 10:07:40.000000000 -0800
+++ finder-patched-2008-03-12.el	2008-03-12 10:08:58.000000000 -0800
@@ -359,16 +359,13 @@
 finder directory, \\[finder-exit] = quit, \\[finder-summary] = help")))
 
 (defun finder-exit ()
-  "Exit Finder mode and kill the buffer."
+  "Exit Finder mode.
+Delete the window and kill the buffer."
   (interactive)
-  (or (one-window-p t)
-      (delete-window))
-  ;; Can happen in either buffer -- kill each of the two that exists
-  (and (get-buffer "*Finder*")
-       (kill-buffer "*Finder*"))
-  (and (get-buffer "*Finder Category*")
-       (kill-buffer "*Finder Category*")))
-
+  (condition-case nil (delete-window window) (error nil))
+  (when (get-buffer "*Finder*") (kill-buffer "*Finder*"))
+  (when (get-buffer "*Finder-package*") (kill-buffer "*Finder-package*"))
+  (when (get-buffer "*Finder Category*") (kill-buffer "*Finder Category*")))
 \f
 (provide 'finder)
 

Diff finished.  Wed Mar 12 10:09:28 2008

RE: finder.el patch

[Drew Adams]

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

Here is an updated finder.el patch, which includes the bug fixes mentioned
previously. This patch adds two minor features:

* It uses a new syntax table, which inherits from
emacs-lisp-mode-syntax-table.  It does not consider `;' to be comment syntax
(so you don't get highlighting of half of a Commentary sentence that
contains `;').

* It font-lock highlights text between quotes: `...', as in Emacs-Lisp doc
strings.

Without these enhancements, finder uses Emacs-Lisp mode syntax, but with the
`;;' line prefix removed, which doesn't make much sense. With these changes,
a Commentary becomes quite readable.


> From: Drew Adams Sent: Wednesday, March 12, 2008 11:15 AM
> This patch fixes two bugs in finder-exit:
> 
> - It treats buffer *Finder-package* also.
>   This buffer is used by `finder-commentary'.
> 
> - Instead trying to avoid `delete-window' for
>   `one-window-p', it wraps it in `condition-case'.
> 

[-- Attachment #2: finder-2008-03-14a.patch --]
[-- Type: application/octet-stream, Size: 1919 bytes --]

diff -u -w "finder-CVS-2008-03-12.el" "finder-patched-2008-03-14a.el"
--- finder-CVS-2008-03-12.el	2008-03-12 10:07:40.000000000 -0700
+++ finder-patched-2008-03-14a.el	2008-03-14 14:29:10.000000000 -0700
@@ -110,6 +110,12 @@
     (define-key map "d"	'finder-list-keywords)
     map))
 
+(defvar finder-mode-syntax-table
+  (let ((st (copy-syntax-table emacs-lisp-mode-syntax-table)))
+    (modify-syntax-entry ?\; ".   " st)
+    st)
+  "Syntax table used while in `finder-mode'.")
+
 
 ;;; Code for regenerating the keyword list.
 
@@ -343,7 +349,11 @@
   (interactive)
   (kill-all-local-variables)
   (use-local-map finder-mode-map)
-  (set-syntax-table emacs-lisp-mode-syntax-table)
+  (set-syntax-table finder-mode-syntax-table)
+  (font-lock-add-keywords
+   'finder-mode
+   '(("`\\([^']+\\)'" 1 font-lock-constant-face prepend)))
+   ;; '(("`\\(\\sw\\sw+\\)'" 1 font-lock-constant-face prepend)))
   (setq mode-name "Finder")
   (setq major-mode 'finder-mode)
   (set (make-local-variable 'finder-headmark) nil)
@@ -359,15 +369,13 @@
 finder directory, \\[finder-exit] = quit, \\[finder-summary] = help")))
 
 (defun finder-exit ()
-  "Exit Finder mode and kill the buffer."
+  "Exit Finder mode.
+Delete the window and kill the buffer."
   (interactive)
-  (or (one-window-p t)
-      (delete-window))
-  ;; Can happen in either buffer -- kill each of the two that exists
-  (and (get-buffer "*Finder*")
-       (kill-buffer "*Finder*"))
-  (and (get-buffer "*Finder Category*")
-       (kill-buffer "*Finder Category*")))
+  (condition-case nil (delete-window window) (error nil))
+  (when (get-buffer "*Finder*") (kill-buffer "*Finder*"))
+  (when (get-buffer "*Finder-package*") (kill-buffer "*Finder-package*"))
+  (when (get-buffer "*Finder Category*") (kill-buffer "*Finder Category*")))
 
 \f
 (provide 'finder)

Diff finished at Fri Mar 14 14:30:19

Re: finder.el patch

[Stefan Monnier]

> Here is an updated finder.el patch, which includes the bug fixes mentioned
> previously. This patch adds two minor features:

Sorry for taking so long to reply.  These are good changes.

> * It uses a new syntax table, which inherits from
> emacs-lisp-mode-syntax-table.  It does not consider `;' to be comment syntax
> (so you don't get highlighting of half of a Commentary sentence that
> contains `;').

Actually, it does not inherit because it uses `copy-syntax-table'.
Please use `make-syntax-table' so it indeed inherits.

> +  (font-lock-add-keywords
> +   'finder-mode
> +   '(("`\\([^']+\\)'" 1 font-lock-constant-face prepend)))
> +   ;; '(("`\\(\\sw\\sw+\\)'" 1 font-lock-constant-face prepend)))

font-lock-add-keywords is best avoided in general.  Please set
font-lock-defaults properly instead.


        Stefan

RE: finder.el patch

[Drew Adams]

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

> > Here is an updated finder.el patch, which includes the bug 
> > fixes mentioned previously. This patch adds two minor features:
> 
> Sorry for taking so long to reply.  These are good changes.
> 
> Actually, it does not inherit because it uses `copy-syntax-table'.
> Please use `make-syntax-table' so it indeed inherits.

Done. I guess just substituting make-syntax-table for copy-syntax-table is
sufficient.

> font-lock-add-keywords is best avoided in general.  Please set
> font-lock-defaults properly instead.

New patch attached.


[-- Attachment #2: finder-2008-03-21a.patch --]
[-- Type: application/octet-stream, Size: 2114 bytes --]

diff -u -w "finder-CVS-2008-03-12.el" "finder-patched-2008-03-21a.el"
--- finder-CVS-2008-03-12.el	2008-03-12 10:07:40.000000000 -0700
+++ finder-patched-2008-03-21a.el	2008-03-21 13:52:48.000000000 -0700
@@ -110,6 +110,16 @@
     (define-key map "d"	'finder-list-keywords)
     map))
 
+(defvar finder-mode-syntax-table
+  (let ((st (make-syntax-table emacs-lisp-mode-syntax-table)))
+    (modify-syntax-entry ?\; ".   " st)
+    st)
+  "Syntax table used while in `finder-mode'.")
+
+(defvar finder-font-lock-keywords
+  '(("`\\([^']+\\)'" 1 font-lock-constant-face prepend))
+  "Font-lock keywords for Finder mode, in addition to Lisp mode keywords.")
+
 
 ;;; Code for regenerating the keyword list.
 
@@ -343,7 +353,11 @@
   (interactive)
   (kill-all-local-variables)
   (use-local-map finder-mode-map)
-  (set-syntax-table emacs-lisp-mode-syntax-table)
+  (set-syntax-table finder-mode-syntax-table)
+  (setq font-lock-defaults
+	'((finder-font-lock-keywords lisp-font-lock-keywords
+           lisp-font-lock-keywords-1 lisp-font-lock-keywords-2)
+	  nil nil (("+-*/.<>=!?$%_&~^:@" . "w")) nil))
   (setq mode-name "Finder")
   (setq major-mode 'finder-mode)
   (set (make-local-variable 'finder-headmark) nil)
@@ -359,15 +373,13 @@
 finder directory, \\[finder-exit] = quit, \\[finder-summary] = help")))
 
 (defun finder-exit ()
-  "Exit Finder mode and kill the buffer."
+  "Exit Finder mode.
+Delete the window and kill the buffer."
   (interactive)
-  (or (one-window-p t)
-      (delete-window))
-  ;; Can happen in either buffer -- kill each of the two that exists
-  (and (get-buffer "*Finder*")
-       (kill-buffer "*Finder*"))
-  (and (get-buffer "*Finder Category*")
-       (kill-buffer "*Finder Category*")))
+  (condition-case nil (delete-window) (error nil))
+  (when (get-buffer "*Finder*") (kill-buffer "*Finder*"))
+  (when (get-buffer "*Finder-package*") (kill-buffer "*Finder-package*"))
+  (when (get-buffer "*Finder Category*") (kill-buffer "*Finder Category*")))
 
 \f
 (provide 'finder)

Diff finished at Fri Mar 21 13:58:18

Re: finder.el patch

[Stefan Monnier]

> +  (setq font-lock-defaults
> +	'((finder-font-lock-keywords lisp-font-lock-keywords
> +           lisp-font-lock-keywords-1 lisp-font-lock-keywords-2)

This just sets to use either one of (finder-font-lock-keywords
lisp-font-lock-keywords lisp-font-lock-keywords-1
lisp-font-lock-keywords-2) depending on font-lock-maximum-decoration.
Based on the comment in finder-font-lock-keywords, it doesn't seem to be
the intention.

On a similar note, I'm not sure why we should use the font-lock rules
from lisp-mode, since the buffer's content won't be Lisp code.


        Stefan

RE: finder.el patch

[Drew Adams]

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

> I'm not sure why we should use the font-lock rules
> from lisp-mode, since the buffer's content won't be Lisp code.

The idea was that a Lisp library Commentary sometimes refers to Lisp keywords,
sexps, and so on. But I agree.

New patch attached.

[-- Attachment #2: finder-2008-03-22a.patch --]
[-- Type: application/octet-stream, Size: 2010 bytes --]

diff -u -w "finder-CVS-2008-03-12.el" "finder-patched-2008-03-22a.el"
--- finder-CVS-2008-03-12.el	2008-03-12 10:07:40.000000000 -0700
+++ finder-patched-2008-03-22a.el	2008-03-22 18:59:30.000000000 -0700
@@ -110,6 +110,16 @@
     (define-key map "d"	'finder-list-keywords)
     map))
 
+(defvar finder-mode-syntax-table
+  (let ((st (make-syntax-table emacs-lisp-mode-syntax-table)))
+    (modify-syntax-entry ?\; ".   " st)
+    st)
+  "Syntax table used while in `finder-mode'.")
+
+(defvar finder-font-lock-keywords
+  '(("`\\([^']+\\)'" 1 font-lock-constant-face prepend))
+  "Font-lock keywords for Finder mode.")
+
 
 ;;; Code for regenerating the keyword list.
 
@@ -343,7 +353,9 @@
   (interactive)
   (kill-all-local-variables)
   (use-local-map finder-mode-map)
-  (set-syntax-table emacs-lisp-mode-syntax-table)
+  (set-syntax-table finder-mode-syntax-table)
+  (setq font-lock-defaults '(finder-font-lock-keywords nil nil
+                             (("+-*/.<>=!?$%_&~^:@" . "w")) nil))
   (setq mode-name "Finder")
   (setq major-mode 'finder-mode)
   (set (make-local-variable 'finder-headmark) nil)
@@ -359,15 +371,13 @@
 finder directory, \\[finder-exit] = quit, \\[finder-summary] = help")))
 
 (defun finder-exit ()
-  "Exit Finder mode and kill the buffer."
+  "Exit Finder mode.
+Delete the window and kill the buffer."
   (interactive)
-  (or (one-window-p t)
-      (delete-window))
-  ;; Can happen in either buffer -- kill each of the two that exists
-  (and (get-buffer "*Finder*")
-       (kill-buffer "*Finder*"))
-  (and (get-buffer "*Finder Category*")
-       (kill-buffer "*Finder Category*")))
+  (condition-case nil (delete-window) (error nil))
+  (when (get-buffer "*Finder*") (kill-buffer "*Finder*"))
+  (when (get-buffer "*Finder-package*") (kill-buffer "*Finder-package*"))
+  (when (get-buffer "*Finder Category*") (kill-buffer "*Finder Category*")))
 
 \f
 (provide 'finder)

Diff finished at Sat Mar 22 18:59:50

Re: finder.el patch

[Stefan Monnier]

>> I'm not sure why we should use the font-lock rules
>> from lisp-mode, since the buffer's content won't be Lisp code.

> The idea was that a Lisp library Commentary sometimes refers to Lisp keywords,
> sexps, and so on. But I agree.

> New patch attached.

Looks good.  Can you provide a ChangeLog entry?


        Stefan

RE: finder.el patch

[Drew Adams]

> Looks good.  Can you provide a ChangeLog entry?

2008-03-22 Drew Adams  <drew.adams@oracle.com>
	* finder.el:
	(finder-mode-syntax-table, finder-font-lock-keywords):
	New variables.
	(finder-mode):
	Use finder-mode-syntax-table. Set font-lock-defaults.
	(finder-exit):
	Delete window unconditionally, without error.
	Kill *Finder-package* buffer also.