unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#5937: 23.1.95; why saving empty abbrev tables
@ 2010-04-12 15:23 Leo
  2010-04-12 18:32 ` Stefan Monnier
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2010-04-12 15:23 UTC (permalink / raw)
  To: 5937

Why empty abbrev tables are saved to file? It seems to make it more
difficult for editing (edit-abbrevs) because the buffer is full of empty
abbrev. I wonder if saving only non-empty tables is better and user
friendlier. For example, any harm of doing something like this:

(defun abbrev-table-empty-p (table)
  "Return nil if there are no abbrev symbols in abbrev table object TABLE."
  (unless (abbrev-table-p table)
    (error "Non abbrev table object"))
  (not (catch 'some
         (mapatoms (lambda (sym)
                     (when (abbrev-symbol (symbol-name sym) table)
                       (throw 'some t)))
                   table))))
(defadvice write-abbrev-file (around nonempty-abbrev-tables activate)
  "Ignore empty abbrev tables when writing to FILE."
  (let ((file (ad-get-arg 0))
        (coding-system-for-write 'emacs-mule)
        (tables (loop for table in abbrev-table-name-list
                      unless (abbrev-table-empty-p (symbol-value table))
                      collect table)))
    (or (and file (> (length file) 0)) (setq file abbrev-file-name))
    (with-temp-file file
      (insert ";;-*-coding: emacs-mule;-*-\n")
      (dolist (table (sort tables
                           (lambda (s1 s2)
                             (string< (symbol-name s1)
                                      (symbol-name s2)))))
        (insert-abbrev-table-description table nil)))))







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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-12 15:23 bug#5937: 23.1.95; why saving empty abbrev tables Leo
@ 2010-04-12 18:32 ` Stefan Monnier
  2010-04-15 10:26   ` Leo
  2010-04-16 10:36   ` Leo
  0 siblings, 2 replies; 29+ messages in thread
From: Stefan Monnier @ 2010-04-12 18:32 UTC (permalink / raw)
  To: Leo; +Cc: 5937

> Why empty abbrev tables are saved to file?

Don't know.

> It seems to make it more difficult for editing (edit-abbrevs) because
> the buffer is full of empty abbrev.  I wonder if saving only non-empty
> tables is better and user friendlier.

It does sound like a good idea.  Any objection?

> (defun abbrev-table-empty-p (table)
>   "Return nil if there are no abbrev symbols in abbrev table object TABLE."
>   (unless (abbrev-table-p table)
>     (error "Non abbrev table object"))
>   (not (catch 'some
>          (mapatoms (lambda (sym)
>                      (when (abbrev-symbol (symbol-name sym) table)
>                        (throw 'some t)))
>                    table))))
> (defadvice write-abbrev-file (around nonempty-abbrev-tables activate)
>   "Ignore empty abbrev tables when writing to FILE."
>   (let ((file (ad-get-arg 0))
>         (coding-system-for-write 'emacs-mule)
>         (tables (loop for table in abbrev-table-name-list
>                       unless (abbrev-table-empty-p (symbol-value table))
>                       collect table)))
>     (or (and file (> (length file) 0)) (setq file abbrev-file-name))
>     (with-temp-file file
>       (insert ";;-*-coding: emacs-mule;-*-\n")
>       (dolist (table (sort tables
>                            (lambda (s1 s2)
>                              (string< (symbol-name s1)
>                                       (symbol-name s2)))))
>         (insert-abbrev-table-description table nil)))))

Obviously, defadvice wouldn't be the right approach for us.
And rather than (abbrev-symbol (symbol-name sym) table), we can use
(symbol-value sym).
We should also eliminate abbrev tables that only contain "system
abbrevs".


        Stefan






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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-12 18:32 ` Stefan Monnier
@ 2010-04-15 10:26   ` Leo
  2010-04-15 12:44     ` Stefan Monnier
  2010-04-16 10:36   ` Leo
  1 sibling, 1 reply; 29+ messages in thread
From: Leo @ 2010-04-15 10:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

On 2010-04-12 19:32 +0100, Stefan Monnier wrote:
>> Why empty abbrev tables are saved to file?
>
> Don't know.
>
>> It seems to make it more difficult for editing (edit-abbrevs) because
>> the buffer is full of empty abbrev.  I wonder if saving only non-empty
>> tables is better and user friendlier.
>
> It does sound like a good idea.  Any objection?

I have been using this in my init file and I like it. It makes it easier
to see what's actually defined.

(defun abbrev-table-empty-p (table)
  "Return nil if there are no abbrev symbols in abbrev table object
  TABLE."
  (unless (abbrev-table-p table)
    (error "Non abbrev table object"))
  (not (catch 'some
         (mapatoms (lambda (sym)
                     (when (abbrev-symbol (symbol-name sym) table)
                       (throw 'some t)))
                   table))))
(defadvice prepare-abbrev-list-buffer (around nonempty-abbrev-tables
  activate)
  "Ignore empty abbrev tables."
  (let ((abbrev-table-name-list
         (loop for table in abbrev-table-name-list
               unless (abbrev-table-empty-p (symbol-value table))
               collect table)))
    ad-do-it))
(defadvice write-abbrev-file (around nonempty-abbrev-tables activate)
  "Ignore empty abbrev tables when writing to FILE."
  (let ((abbrev-table-name-list
         (loop for table in abbrev-table-name-list
               unless (abbrev-table-empty-p (symbol-value table))
               collect table)))
    ad-do-it))

[...]
> Obviously, defadvice wouldn't be the right approach for us.
> And rather than (abbrev-symbol (symbol-name sym) table), we can use
> (symbol-value sym).
> We should also eliminate abbrev tables that only contain "system
> abbrevs".

Regarding the :system property, I have experienced some oddity but I
by-passed that instead of looking into it for lack of time.

Here's what happened.

When I use define-abbrev to define some system abbrevs and then M-x
edit-abbrevs and move to the table where those abbrevs are defined then
paste some non-system abbrevs from, for example, another abbrev file.
Those pasted abbrevs are treated as system abbrevs and not saved.
Sometimes when the table already have non-system abbrevs, they are
turned into system ones and get lost.

I had a backup of the abbrev file so that didn't cause me any trouble.

>
>         Stefan

Leo






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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-15 10:26   ` Leo
@ 2010-04-15 12:44     ` Stefan Monnier
  0 siblings, 0 replies; 29+ messages in thread
From: Stefan Monnier @ 2010-04-15 12:44 UTC (permalink / raw)
  To: Leo; +Cc: 5937

> When I use define-abbrev to define some system abbrevs and then M-x
> edit-abbrevs and move to the table where those abbrevs are defined then
> paste some non-system abbrevs from, for example, another abbrev file.
> Those pasted abbrevs are treated as system abbrevs and not saved.
> Sometimes when the table already have non-system abbrevs, they are
> turned into system ones and get lost.

Please make a separate bug report about it (and try to give a more
precise recipe for it).


        Stefan






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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-12 18:32 ` Stefan Monnier
  2010-04-15 10:26   ` Leo
@ 2010-04-16 10:36   ` Leo
  2010-04-27  3:49     ` Stefan Monnier
  1 sibling, 1 reply; 29+ messages in thread
From: Leo @ 2010-04-16 10:36 UTC (permalink / raw)
  To: bug-gnu-emacs

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

On 2010-04-12 19:32 +0100, Stefan Monnier wrote:
>> It seems to make it more difficult for editing (edit-abbrevs) because
>> the buffer is full of empty abbrev.  I wonder if saving only non-empty
>> tables is better and user friendlier.
>
> It does sound like a good idea.  Any objection?

Would the following patch be acceptable?

* lisp/abbrev.el (write-abbrev-file prepare-abbrev-list-buffer):
ignore empty abbrev tables
  (abbrev-table-empty-p): new function.


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

diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index b72bdbb..21411a5 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -127,15 +127,20 @@ Otherwise display all abbrevs."
 (defun prepare-abbrev-list-buffer (&optional local)
   (with-current-buffer (get-buffer-create "*Abbrevs*")
     (erase-buffer)
-    (if local
-        (insert-abbrev-table-description
-         (abbrev-table-name local-abbrev-table) t)
-      (dolist (table abbrev-table-name-list)
-        (insert-abbrev-table-description table t)))
-    (goto-char (point-min))
-    (set-buffer-modified-p nil)
-    (edit-abbrevs-mode)
-    (current-buffer)))
+    (let ((abbrev-table-name-list
+           (delete nil (mapcar (lambda (tn)
+                                 (unless (abbrev-table-empty-p (symbol-value tn))
+                                   tn))
+                               abbrev-table-name-list))))
+      (if local
+          (insert-abbrev-table-description
+           (abbrev-table-name local-abbrev-table) t)
+        (dolist (table abbrev-table-name-list)
+          (insert-abbrev-table-description table t)))
+      (goto-char (point-min))
+      (set-buffer-modified-p nil)
+      (edit-abbrevs-mode)
+      (current-buffer))))
 
 (defun edit-abbrevs-mode ()
   "Major mode for editing the list of abbrev definitions.
@@ -232,7 +237,12 @@ specified in `abbrev-file-name' is used."
 		    abbrev-file-name)))
   (or (and file (> (length file) 0))
       (setq file abbrev-file-name))
-  (let ((coding-system-for-write 'emacs-mule))
+  (let ((coding-system-for-write 'emacs-mule)
+        (tables (delete nil (mapcar
+                             (lambda (tn)
+                               (unless (abbrev-table-empty-p (symbol-value tn))
+                                 tn))
+                             abbrev-table-name-list))))
     (with-temp-file file
       (insert ";;-*-coding: emacs-mule;-*-\n")
       (dolist (table
@@ -242,10 +252,9 @@ specified in `abbrev-file-name' is used."
                ;; user keeps their home directory in a revision
                ;; control system, and is therefore keeping multiple
                ;; slightly-differing copies loosely synchronized.
-               (sort (copy-sequence abbrev-table-name-list)
-                     (lambda (s1 s2)
-                       (string< (symbol-name s1)
-                                (symbol-name s2)))))
+               (sort tables (lambda (s1 s2)
+                              (string< (symbol-name s1)
+                                       (symbol-name s2)))))
 	(insert-abbrev-table-description table nil)))))
 \f
 (defun add-mode-abbrev (arg)
@@ -419,6 +428,16 @@ PROPS is a list of properties."
   (and (vectorp object)
        (numberp (abbrev-table-get object :abbrev-table-modiff))))
 
+(defun abbrev-table-empty-p (object)
+  "Return nil if there are no abbrev symbols in OBJECT."
+  (unless (abbrev-table-p object)
+    (error "Non abbrev table object"))
+  (not (catch 'some
+         (mapatoms (lambda (sym)
+                     (when (symbol-value sym)
+                       (throw 'some t)))
+                   object))))
+
 (defvar global-abbrev-table (make-abbrev-table)
   "The abbrev table whose abbrevs affect all buffers.
 Each buffer may also have a local abbrev table.

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


Thanks,
Leo

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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-16 10:36   ` Leo
@ 2010-04-27  3:49     ` Stefan Monnier
  2010-04-27  8:46       ` Leo
  2011-03-27  5:09       ` Leo
  0 siblings, 2 replies; 29+ messages in thread
From: Stefan Monnier @ 2010-04-27  3:49 UTC (permalink / raw)
  To: Leo; +Cc: bug-gnu-emacs

>>> It seems to make it more difficult for editing (edit-abbrevs) because
>>> the buffer is full of empty abbrev.  I wonder if saving only non-empty
>>> tables is better and user friendlier.
>> It does sound like a good idea.  Any objection?
> Would the following patch be acceptable?

Actually, having tried it now for a while I do have an objection: it
makes it harder to add entries to an abbrev-table if that table
is empty.

So I think a better option is to sort the tables such that empty tables
are pushed to the end.


        Stefan







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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-27  3:49     ` Stefan Monnier
@ 2010-04-27  8:46       ` Leo
  2010-04-27 10:12         ` Leo
  2010-04-27 10:32         ` Leo
  2011-03-27  5:09       ` Leo
  1 sibling, 2 replies; 29+ messages in thread
From: Leo @ 2010-04-27  8:46 UTC (permalink / raw)
  To: bug-gnu-emacs

On 2010-04-27 04:49 +0100, Stefan Monnier wrote:
>>>> It seems to make it more difficult for editing (edit-abbrevs) because
>>>> the buffer is full of empty abbrev.  I wonder if saving only non-empty
>>>> tables is better and user friendlier.
>>> It does sound like a good idea.  Any objection?
>> Would the following patch be acceptable?
>
> Actually, having tried it now for a while I do have an objection: it
> makes it harder to add entries to an abbrev-table if that table
> is empty.
>
> So I think a better option is to sort the tables such that empty tables
> are pushed to the end.

I thought of this question when creating the patch. I wasn't sure how
best to offer the option to add abbrevs to empty tables.

When I first tried out abbrev I was confused at edit-abbrevs by all
those empty tables. For example, the editing abbrevs buffer could easily
have 1000 lines with the empty tables show up. I thought I had done
something wrong with those C-x ail stuff. In the end I didn't use abbrev
for 2-3 years until last year when I started using snippet.

I think users use edit-abbrevs often to view and edit existing abbrevs
or adding new ones to a non-empty table. When an abbrev tale is empty it
is most likely the user hasn't used abbrevs in the major mode associated
with it. Personally, I prefer keeping the edit abbrevs buffer smaller.
It is cleaner and less confusing.

Do you think we can address this issue in another way for example by
offering a key biding to enter an empty table? For example, make M-RET
move point to the end of current abbrev table and asking for a table
name (with completion)? (this is similar to M-RET in org mode)

>         Stefan

Leo








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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-27  8:46       ` Leo
@ 2010-04-27 10:12         ` Leo
  2010-04-27 10:32         ` Leo
  1 sibling, 0 replies; 29+ messages in thread
From: Leo @ 2010-04-27 10:12 UTC (permalink / raw)
  To: bug-gnu-emacs

On 2010-04-27 09:46 +0100, Leo wrote:
> Do you think we can address this issue in another way for example by
> offering a key biding to enter an empty table? For example, make M-RET
> move point to the end of current abbrev table and asking for a table
> name (with completion)? (this is similar to M-RET in org mode)

The attached patch does this. Could you try and see if you like it?

diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index 21411a5..c27f29a 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -73,8 +73,9 @@ to enable or disable Abbrev mode in the current buffer."
 \f
 (defvar edit-abbrevs-map
   (let ((map (make-sparse-keymap)))
-    (define-key map "\C-x\C-s" 'edit-abbrevs-redefine)
-    (define-key map "\C-c\C-c" 'edit-abbrevs-redefine)
+    (define-key map "\C-x\C-s"      'edit-abbrevs-redefine)
+    (define-key map "\C-c\C-c"      'edit-abbrevs-redefine)
+    (define-key map [(meta return)] 'edit-abbrevs-new-table-entry)
     map)
   "Keymap used in `edit-abbrevs'.")
 
@@ -175,6 +176,24 @@ or may be omitted (it is usually omitted)."
     (define-abbrevs t)
     (set-buffer-modified-p nil)))
 
+(defun edit-abbrevs-new-table-entry (tablename)
+  "Enter a new table entry for TABLENAME into current buffer."
+  (interactive
+   (list (intern-soft
+          (completing-read "Table name: "
+                           (mapcar 'symbol-name abbrev-table-name-list)
+                           (lambda (tn)
+                             (abbrev-table-empty-p
+                              (symbol-value (intern-soft tn))))
+                           t))))
+  (or (looking-at "^(.*)$")
+      (and (re-search-forward "^(.*)$" nil t)
+           (goto-char (match-beginning 0)))
+      (goto-char (point-max)))
+  (unless (bolp) (insert "\n"))
+  (insert-abbrev-table-description tablename t)
+  (forward-line -2))
+
 (defun define-abbrevs (&optional arg)
   "Define abbrevs according to current visible buffer contents.
 See documentation of `edit-abbrevs' for info on the format of the








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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-27  8:46       ` Leo
  2010-04-27 10:12         ` Leo
@ 2010-04-27 10:32         ` Leo
  2011-03-27 20:40           ` Stefan Monnier
  1 sibling, 1 reply; 29+ messages in thread
From: Leo @ 2010-04-27 10:32 UTC (permalink / raw)
  To: bug-gnu-emacs

On 2010-04-27 09:46 +0100, Leo wrote:
> When I first tried out abbrev I was confused at edit-abbrevs by all
> those empty tables. For example, the editing abbrevs buffer could easily
> have 1000 lines with the empty tables show up. I thought I had done
> something wrong with those C-x ail stuff. In the end I didn't use abbrev
> for 2-3 years until last year when I started using snippet.

While recollecting some of the surprises, I think there's another one.
The meaning of C-x C-s is different in edit-abbrevs.

C-x C-s usually means things have been saved and are safe unless
something happen to the harddisk.

In abbrevs it is not. And sometimes one doesn't get the chance to
actually save it because emacs crashes halfway.

I would propose separating C-x C-s and C-c C-c. C-x C-s should stay
close to the global key binding.

Leo








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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-27  3:49     ` Stefan Monnier
  2010-04-27  8:46       ` Leo
@ 2011-03-27  5:09       ` Leo
  2011-03-27 17:34         ` Andreas Röhler
  2011-03-27 20:35         ` Stefan Monnier
  1 sibling, 2 replies; 29+ messages in thread
From: Leo @ 2011-03-27  5:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

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

On 2010-04-27 11:49 +0800, Stefan Monnier wrote:
> Actually, having tried it now for a while I do have an objection: it
> makes it harder to add entries to an abbrev-table if that table
> is empty.
>
> So I think a better option is to sort the tables such that empty tables
> are pushed to the end.
>
>
>         Stefan

Hello Stefan,

I propose we allow C-u M-x edit-abbrevs to include all abbrev tables but
default to include only nonempty tables.

Here is an updated patch.


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

=== modified file 'lisp/abbrev.el'
--- lisp/abbrev.el	2011-03-22 15:38:40 +0000
+++ lisp/abbrev.el	2011-03-27 04:56:22 +0000
@@ -104,7 +104,7 @@
 prefix arg, display only local, i.e. mode-specific, abbrevs.
 Otherwise display all abbrevs."
   (interactive "P")
-  (display-buffer (prepare-abbrev-list-buffer local)))
+  (display-buffer (prepare-abbrev-list-buffer (and local 'local))))
 
 (defun abbrev-table-name (table)
   "Value is the name of abbrev table TABLE."
@@ -116,14 +116,28 @@
       (setq tables (cdr tables)))
     found))
 
-(defun prepare-abbrev-list-buffer (&optional local)
-  (let ((local-table local-abbrev-table))
+(defun abbrev-nonempty-table-names (&optional ignore-system)
+  "Return a list of nonempty abbrev table names.
+If IGNORE-SYSTEM is non-nil, system definitions are ignored."
+  (delete nil
+          (mapcar (lambda (tn)
+                    (unless (abbrev-table-empty-p
+                             (symbol-value tn) ignore-system)
+                      tn))
+                  abbrev-table-name-list)))
+
+;; HOW takes value: nil, local or all.
+(defun prepare-abbrev-list-buffer (&optional how)
+  (let ((local-table local-abbrev-table)
+        (tables (if (eq how 'all)
+                    abbrev-table-name-list
+                  (abbrev-nonempty-table-names))))
     (with-current-buffer (get-buffer-create "*Abbrevs*")
       (erase-buffer)
-      (if local
+      (if (eq how 'local)
           (insert-abbrev-table-description
            (abbrev-table-name local-table) t)
-        (dolist (table abbrev-table-name-list)
+        (dolist (table tables)
           (insert-abbrev-table-description table t)))
       (goto-char (point-min))
       (set-buffer-modified-p nil)
@@ -140,10 +154,11 @@
   (use-local-map edit-abbrevs-map)
   (run-mode-hooks 'edit-abbrevs-mode-hook))
 
-(defun edit-abbrevs ()
+(defun edit-abbrevs (&optional all)
   "Alter abbrev definitions by editing a list of them.
 Selects a buffer containing a list of abbrev definitions.
-You can edit them and type \\<edit-abbrevs-map>\\[edit-abbrevs-redefine] to redefine abbrevs
+Use \\[universal-argument] to include empty abbrev tables.  You can edit
+them and type \\<edit-abbrevs-map>\\[edit-abbrevs-redefine] to redefine abbrevs
 according to your editing.
 Buffer contains a header line for each abbrev table,
  which is the abbrev table name in parentheses.
@@ -152,8 +167,8 @@
 where NAME and EXPANSION are strings with quotes,
 USECOUNT is an integer, and HOOK is any valid function
 or may be omitted (it is usually omitted)."
-  (interactive)
-  (switch-to-buffer (prepare-abbrev-list-buffer)))
+  (interactive "P")
+  (switch-to-buffer (prepare-abbrev-list-buffer (and all 'all))))
 
 (defun edit-abbrevs-redefine ()
   "Redefine abbrevs according to current buffer contents."
@@ -225,7 +240,8 @@
 		    abbrev-file-name)))
   (or (and file (> (length file) 0))
       (setq file abbrev-file-name))
-  (let ((coding-system-for-write 'utf-8))
+  (let ((coding-system-for-write 'utf-8)
+        (tables (abbrev-nonempty-table-names t)))
     (with-temp-buffer
       (dolist (table
 	       ;; We sort the table in order to ease the automatic
@@ -234,10 +250,9 @@
 	       ;; user keeps their home directory in a revision
 	       ;; control system, and is therefore keeping multiple
 	       ;; slightly-differing copies loosely synchronized.
-	       (sort (copy-sequence abbrev-table-name-list)
-		     (lambda (s1 s2)
-		       (string< (symbol-name s1)
-				(symbol-name s2)))))
+	       (sort tables (lambda (s1 s2)
+			      (string< (symbol-name s1)
+				       (symbol-name s2)))))
 	(insert-abbrev-table-description table nil))
       (when (unencodable-char-position (point-min) (point-max) 'utf-8)
 	(setq coding-system-for-write
@@ -420,6 +435,18 @@
   (and (vectorp object)
        (numberp (abbrev-table-get object :abbrev-table-modiff))))
 
+(defun abbrev-table-empty-p (object &optional ignore-system)
+  "Return nil if there are no abbrev symbols in OBJECT.
+If IGNORE-SYSTEM is non-nil, system definitions are ignored."
+  (unless (abbrev-table-p object)
+    (error "Non abbrev table object"))
+  (not (catch 'some
+         (mapatoms (lambda (abbrev)
+                     (unless (or (zerop (length (symbol-name abbrev)))
+                                 (and ignore-system (abbrev-get abbrev :system)))
+                       (throw 'some t)))
+                   object))))
+
 (defvar global-abbrev-table (make-abbrev-table)
   "The abbrev table whose abbrevs affect all buffers.
 Each buffer may also have a local abbrev table.


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


Leo

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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-27  5:09       ` Leo
@ 2011-03-27 17:34         ` Andreas Röhler
  2011-03-28  3:38           ` Leo
  2011-03-27 20:35         ` Stefan Monnier
  1 sibling, 1 reply; 29+ messages in thread
From: Andreas Röhler @ 2011-03-27 17:34 UTC (permalink / raw)
  To: bug-gnu-emacs

Am 27.03.2011 07:09, schrieb Leo:
> On 2010-04-27 11:49 +0800, Stefan Monnier wrote:
>> Actually, having tried it now for a while I do have an objection: it
>> makes it harder to add entries to an abbrev-table if that table
>> is empty.
>>
>> So I think a better option is to sort the tables such that empty tables
>> are pushed to the end.
>>
>>
>>          Stefan
>
> Hello Stefan,
>
> I propose we allow C-u M-x edit-abbrevs to include all abbrev tables but
> default to include only nonempty tables.
>
> Here is an updated patch.
>
>
>
>
> === modified file 'lisp/abbrev.el'
> --- lisp/abbrev.el	2011-03-22 15:38:40 +0000
> +++ lisp/abbrev.el	2011-03-27 04:56:22 +0000
> @@ -104,7 +104,7 @@
>   prefix arg, display only local, i.e. mode-specific, abbrevs.
>   Otherwise display all abbrevs."
>     (interactive "P")
> -  (display-buffer (prepare-abbrev-list-buffer local)))
> +  (display-buffer (prepare-abbrev-list-buffer (and local 'local))))
>
>   (defun abbrev-table-name (table)
>     "Value is the name of abbrev table TABLE."
> @@ -116,14 +116,28 @@
>         (setq tables (cdr tables)))
>       found))
>
> -(defun prepare-abbrev-list-buffer (&optional local)
> -  (let ((local-table local-abbrev-table))
> +(defun abbrev-nonempty-table-names (&optional ignore-system)
> +  "Return a list of nonempty abbrev table names.
> +If IGNORE-SYSTEM is non-nil, system definitions are ignored."
> +  (delete nil
> +          (mapcar (lambda (tn)
> +                    (unless (abbrev-table-empty-p
> +                             (symbol-value tn) ignore-system)
> +                      tn))
> +                  abbrev-table-name-list)))
> +
> +;; HOW takes value: nil, local or all.
> +(defun prepare-abbrev-list-buffer (&optional how)
> +  (let ((local-table local-abbrev-table)
> +        (tables (if (eq how 'all)
> +                    abbrev-table-name-list
> +                  (abbrev-nonempty-table-names))))
>       (with-current-buffer (get-buffer-create "*Abbrevs*")
>         (erase-buffer)
> -      (if local
> +      (if (eq how 'local)
>             (insert-abbrev-table-description
>              (abbrev-table-name local-table) t)
> -        (dolist (table abbrev-table-name-list)
> +        (dolist (table tables)
>             (insert-abbrev-table-description table t)))
>         (goto-char (point-min))
>         (set-buffer-modified-p nil)
> @@ -140,10 +154,11 @@
>     (use-local-map edit-abbrevs-map)
>     (run-mode-hooks 'edit-abbrevs-mode-hook))
>
> -(defun edit-abbrevs ()
> +(defun edit-abbrevs (&optional all)
>     "Alter abbrev definitions by editing a list of them.
>   Selects a buffer containing a list of abbrev definitions.
> -You can edit them and type \\<edit-abbrevs-map>\\[edit-abbrevs-redefine] to redefine abbrevs
> +Use \\[universal-argument] to include empty abbrev tables.  You can edit
> +them and type \\<edit-abbrevs-map>\\[edit-abbrevs-redefine] to redefine abbrevs
>   according to your editing.
>   Buffer contains a header line for each abbrev table,
>    which is the abbrev table name in parentheses.
> @@ -152,8 +167,8 @@
>   where NAME and EXPANSION are strings with quotes,
>   USECOUNT is an integer, and HOOK is any valid function
>   or may be omitted (it is usually omitted)."
> -  (interactive)
> -  (switch-to-buffer (prepare-abbrev-list-buffer)))
> +  (interactive "P")
> +  (switch-to-buffer (prepare-abbrev-list-buffer (and all 'all))))
>
>   (defun edit-abbrevs-redefine ()
>     "Redefine abbrevs according to current buffer contents."
> @@ -225,7 +240,8 @@
>   		    abbrev-file-name)))
>     (or (and file (>  (length file) 0))
>         (setq file abbrev-file-name))
> -  (let ((coding-system-for-write 'utf-8))
> +  (let ((coding-system-for-write 'utf-8)
> +        (tables (abbrev-nonempty-table-names t)))
>       (with-temp-buffer
>         (dolist (table
>   	       ;; We sort the table in order to ease the automatic
> @@ -234,10 +250,9 @@
>   	       ;; user keeps their home directory in a revision
>   	       ;; control system, and is therefore keeping multiple
>   	       ;; slightly-differing copies loosely synchronized.
> -	       (sort (copy-sequence abbrev-table-name-list)
> -		     (lambda (s1 s2)
> -		       (string<  (symbol-name s1)
> -				(symbol-name s2)))))
> +	       (sort tables (lambda (s1 s2)
> +			      (string<  (symbol-name s1)
> +				       (symbol-name s2)))))
>   	(insert-abbrev-table-description table nil))
>         (when (unencodable-char-position (point-min) (point-max) 'utf-8)
>   	(setq coding-system-for-write
> @@ -420,6 +435,18 @@
>     (and (vectorp object)
>          (numberp (abbrev-table-get object :abbrev-table-modiff))))
>
> +(defun abbrev-table-empty-p (object&optional ignore-system)
> +  "Return nil if there are no abbrev symbols in OBJECT.
> +If IGNORE-SYSTEM is non-nil, system definitions are ignored."
> +  (unless (abbrev-table-p object)
> +    (error "Non abbrev table object"))
> +  (not (catch 'some
> +         (mapatoms (lambda (abbrev)
> +                     (unless (or (zerop (length (symbol-name abbrev)))
> +                                 (and ignore-system (abbrev-get abbrev :system)))
> +                       (throw 'some t)))
> +                   object))))
> +
>   (defvar global-abbrev-table (make-abbrev-table)
>     "The abbrev table whose abbrevs affect all buffers.
>   Each buffer may also have a local abbrev table.
>
>
>
>
>
> Leo

Hi Leo,

just an abstract reasoning: don't change things you are not compelled 
to. Or to say: unless there is a reason for it.

An empty table doesn't take considerable space nor time.

OTOH when changing a thing, always remains a certain risk to break 
something.

As said, just abtract reasoning. Don't see a real danger here.

Cheers

Andreas








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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-27  5:09       ` Leo
  2011-03-27 17:34         ` Andreas Röhler
@ 2011-03-27 20:35         ` Stefan Monnier
  2011-03-28  4:03           ` Leo
  1 sibling, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-27 20:35 UTC (permalink / raw)
  To: Leo; +Cc: 5937

>> Actually, having tried it now for a while I do have an objection: it
>> makes it harder to add entries to an abbrev-table if that table
>> is empty.
>> 
>> So I think a better option is to sort the tables such that empty tables
>> are pushed to the end.

> I propose we allow C-u M-x edit-abbrevs to include all abbrev tables but
> default to include only nonempty tables.

I don't think we need such a thing.  We just need to make it clear what
those empty tables mean and keep them out of the way.  I.e. move them to
the end of the buffer and add a comment before them explaining that
these are currently empty.


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2010-04-27 10:32         ` Leo
@ 2011-03-27 20:40           ` Stefan Monnier
  2011-03-28  4:45             ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-27 20:40 UTC (permalink / raw)
  To: Leo; +Cc: bug-gnu-emacs

> C-x C-s usually means things have been saved and are safe unless
> something happen to the harddisk.

> In abbrevs it is not. And sometimes one doesn't get the chance to
> actually save it because emacs crashes halfway.

> I would propose separating C-x C-s and C-c C-c. C-x C-s should stay
> close to the global key binding.

You mean keep C-c C-c bound to edit-abbrevs-redefine, but change C-x C-s
to call write-abbrev-file?  Sounds good (tho it'll have to call
edit-abbrevs-redefine before write-abbrev-file beforehand, of course).
Oh and by the way: the abbrev.el file should be (slowly) fixed to use
the "abbrev-" prefix for all functions and commands, so whenever you
introduce a new function/command/variable please use this prefix rather
than the previous style of <verb>-abbrev-<thing>.


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-27 17:34         ` Andreas Röhler
@ 2011-03-28  3:38           ` Leo
  0 siblings, 0 replies; 29+ messages in thread
From: Leo @ 2011-03-28  3:38 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: bug-gnu-emacs

On 2011-03-28 01:34 +0800, Andreas Röhler wrote:
> just an abstract reasoning: don't change things you are not compelled
> to. Or to say: unless there is a reason for it.

In practice and in my currently running emacs, there are 68 abbrev
tables breaking down to: 57 empty + 4 with pure system abbrevs + 7 with
user defined abbrevs.

Obviously the 57 empty tables which, in my case, never get used at all
get in the way pretty badly.

Leo





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-27 20:35         ` Stefan Monnier
@ 2011-03-28  4:03           ` Leo
  2011-03-28 14:02             ` Stefan Monnier
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2011-03-28  4:03 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

On 2011-03-28 04:35 +0800, Stefan Monnier wrote:
> I don't think we need such a thing.  We just need to make it clear what
> those empty tables mean and keep them out of the way.  I.e. move them to
> the end of the buffer and add a comment before them explaining that
> these are currently empty.

OK let's do this. I didn't insert a line explaining empty tables because
they are obvious (See this screenshot: http://imgur.com/zCegB.png).

(BTW, after looking at that screenshot I realised Makefile mode
introduces 6 abbrev tables; amazing.)

Let me know if you are OK with this patch.

Thanks,
Leo

=== modified file 'lisp/abbrev.el'
--- lisp/abbrev.el	2011-03-22 15:38:40 +0000
+++ lisp/abbrev.el	2011-03-28 03:52:49 +0000
@@ -123,8 +123,13 @@
       (if local
           (insert-abbrev-table-description
            (abbrev-table-name local-table) t)
-        (dolist (table abbrev-table-name-list)
-          (insert-abbrev-table-description table t)))
+        (let (empty-tables)
+	  (dolist (table abbrev-table-name-list)
+	    (if (abbrev-table-empty-p (symbol-value table))
+		(push table empty-tables)
+	      (insert-abbrev-table-description table t)))
+	  (dolist (table (nreverse empty-tables))
+	    (insert-abbrev-table-description table t))))
       (goto-char (point-min))
       (set-buffer-modified-p nil)
       (edit-abbrevs-mode)
@@ -420,6 +425,18 @@
   (and (vectorp object)
        (numberp (abbrev-table-get object :abbrev-table-modiff))))
 
+(defun abbrev-table-empty-p (object &optional ignore-system)
+  "Return nil if there are no abbrev symbols in OBJECT.
+If IGNORE-SYSTEM is non-nil, system definitions are ignored."
+  (unless (abbrev-table-p object)
+    (error "Non abbrev table object"))
+  (not (catch 'some
+	 (mapatoms (lambda (abbrev)
+		     (unless (or (zerop (length (symbol-name abbrev)))
+				 (and ignore-system (abbrev-get abbrev :system)))
+		       (throw 'some t)))
+		   object))))
+
 (defvar global-abbrev-table (make-abbrev-table)
   "The abbrev table whose abbrevs affect all buffers.
 Each buffer may also have a local abbrev table.







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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-27 20:40           ` Stefan Monnier
@ 2011-03-28  4:45             ` Leo
  2011-03-28 13:58               ` Stefan Monnier
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2011-03-28  4:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

On 2011-03-28 04:40 +0800, Stefan Monnier wrote:
> You mean keep C-c C-c bound to edit-abbrevs-redefine, but change C-x C-s
> to call write-abbrev-file?  Sounds good (tho it'll have to call
> edit-abbrevs-redefine before write-abbrev-file beforehand, of course).
> Oh and by the way: the abbrev.el file should be (slowly) fixed to use
> the "abbrev-" prefix for all functions and commands, so whenever you
> introduce a new function/command/variable please use this prefix rather
> than the previous style of <verb>-abbrev-<thing>.

How about this patch?

=== modified file 'lisp/abbrev.el'
--- lisp/abbrev.el	2011-03-22 15:38:40 +0000
+++ lisp/abbrev.el	2011-03-28 04:42:26 +0000
@@ -65,7 +65,8 @@
 \f
 (defvar edit-abbrevs-map
   (let ((map (make-sparse-keymap)))
-    (define-key map "\C-x\C-s" 'edit-abbrevs-redefine)
+    (define-key map "\C-x\C-s" 'abbrev-save-buffer)
+    (define-key map "\C-x\C-w" 'write-abbrev-file)
     (define-key map "\C-c\C-c" 'edit-abbrevs-redefine)
     map)
   "Keymap used in `edit-abbrevs'.")
@@ -211,13 +212,14 @@
 					;(interactive "fRead abbrev file: ")
   (read-abbrev-file file t))
 
-(defun write-abbrev-file (&optional file)
+(defun write-abbrev-file (&optional file verbose)
   "Write all user-level abbrev definitions to a file of Lisp code.
 This does not include system abbrevs; it includes only the abbrev tables
 listed in listed in `abbrev-table-name-list'.
 The file written can be loaded in another session to define the same abbrevs.
 The argument FILE is the file name to write.  If omitted or nil, the file
-specified in `abbrev-file-name' is used."
+specified in `abbrev-file-name' is used.
+If VERBOSE is non-nil, display a message where abbrevs have been saved."
   (interactive
    (list
     (read-file-name "Write abbrev file: "
@@ -225,6 +227,7 @@
 		    abbrev-file-name)))
   (or (and file (> (length file) 0))
       (setq file abbrev-file-name))
+  (edit-abbrevs-redefine)
   (let ((coding-system-for-write 'utf-8))
     (with-temp-buffer
       (dolist (table
@@ -247,7 +250,14 @@
 		'emacs-mule)))
       (goto-char (point-min))
       (insert (format ";;-*-coding: %s;-*-\n" coding-system-for-write))
-      (write-region nil nil file nil 0))))
+      (write-region nil nil file nil (and (not verbose) 0)))))
+
+(defun abbrev-save-buffer ()
+  "Save all user-level abbrev definitions in current buffer.
+The file name to use is `abbrev-file-name'."
+  (interactive)
+  (write-abbrev-file abbrev-file-name t))
+
 \f
 (defun add-mode-abbrev (arg)
   "Define mode-specific abbrev for last word(s) before point.






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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-28  4:45             ` Leo
@ 2011-03-28 13:58               ` Stefan Monnier
  2011-03-28 14:26                 ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-28 13:58 UTC (permalink / raw)
  To: Leo; +Cc: 5937

>> You mean keep C-c C-c bound to edit-abbrevs-redefine, but change C-x C-s
>> to call write-abbrev-file?  Sounds good (tho it'll have to call
>> edit-abbrevs-redefine before write-abbrev-file beforehand, of course).
>> Oh and by the way: the abbrev.el file should be (slowly) fixed to use
>> the "abbrev-" prefix for all functions and commands, so whenever you
>> introduce a new function/command/variable please use this prefix rather
>> than the previous style of <verb>-abbrev-<thing>.
> How about this patch?

> @@ -225,6 +227,7 @@
>  		    abbrev-file-name)))
>    (or (and file (> (length file) 0))
>        (setq file abbrev-file-name))
> +  (edit-abbrevs-redefine)
>    (let ((coding-system-for-write 'utf-8))
>      (with-temp-buffer
>        (dolist (table
> @@ -247,7 +250,14 @@
>  		'emacs-mule)))
>        (goto-char (point-min))
>        (insert (format ";;-*-coding: %s;-*-\n" coding-system-for-write))
> -      (write-region nil nil file nil 0))))
> +      (write-region nil nil file nil (and (not verbose) 0)))))
> +
> +(defun abbrev-save-buffer ()
> +  "Save all user-level abbrev definitions in current buffer.
> +The file name to use is `abbrev-file-name'."
> +  (interactive)
> +  (write-abbrev-file abbrev-file-name t))

write-abbrev-file may be called from a non-"edit-abbrevs" buffer
(e.g. from save-some-buffer), so I think that calling
edit-abbrevs-redefine from it is problematic.  Is there a particular
reason why you put the call to edit-abbrevs-redefine in
write-abbrev-file rather than in abbrev-save-buffer.


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-28  4:03           ` Leo
@ 2011-03-28 14:02             ` Stefan Monnier
  2011-03-28 14:40               ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-28 14:02 UTC (permalink / raw)
  To: Leo; +Cc: 5937

>> I don't think we need such a thing.  We just need to make it clear what
>> those empty tables mean and keep them out of the way.  I.e. move them to
>> the end of the buffer and add a comment before them explaining that
>> these are currently empty.
> OK let's do this. I didn't insert a line explaining empty tables because
> they are obvious (See this screenshot: http://imgur.com/zCegB.png).

> (BTW, after looking at that screenshot I realised Makefile mode
> introduces 6 abbrev tables; amazing.)

> Let me know if you are OK with this patch.

The patch looks good, thank you, please install it.  I'd still like to
see a comment added, but there are many other things that would be good
to add to edit-abbrevs (e.g. highlighting, or an actually useful C-h m
output explaining the format expected by C-c C-c, ...).


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-28 13:58               ` Stefan Monnier
@ 2011-03-28 14:26                 ` Leo
  2011-03-28 15:09                   ` Stefan Monnier
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2011-03-28 14:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

On 2011-03-28 21:58 +0800, Stefan Monnier wrote:
> write-abbrev-file may be called from a non-"edit-abbrevs" buffer
> (e.g. from save-some-buffer), so I think that calling
> edit-abbrevs-redefine from it is problematic.  Is there a particular
> reason why you put the call to edit-abbrevs-redefine in
> write-abbrev-file rather than in abbrev-save-buffer.

Good point. I missed it. I want C-x C-w to also redefine the abbrevs if
it is called in that editing buffer. Do you mind if I do something like
the following in write-abbrev-file:

 (and (derived-mode-p 'edit-abbrevs-mode)
      (edit-abbrevs-redefine))

Leo

-- 
Oracle is the new evil





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-28 14:02             ` Stefan Monnier
@ 2011-03-28 14:40               ` Leo
  0 siblings, 0 replies; 29+ messages in thread
From: Leo @ 2011-03-28 14:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

On 2011-03-28 22:02 +0800, Stefan Monnier wrote:
> The patch looks good, thank you, please install it.

Installed in trunk.

> I'd still like to see a comment added, but there are many other things
> that would be good to add to edit-abbrevs (e.g. highlighting, or an
> actually useful C-h m output explaining the format expected by C-c
> C-c, ...).

`highlighting' would be good. I think the comment is best inserted when
it has its own face. At the moment it looks out of place.

Leo





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-28 14:26                 ` Leo
@ 2011-03-28 15:09                   ` Stefan Monnier
  2011-03-29  0:35                     ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-28 15:09 UTC (permalink / raw)
  To: Leo; +Cc: 5937

>> write-abbrev-file may be called from a non-"edit-abbrevs" buffer
>> (e.g. from save-some-buffer), so I think that calling
>> edit-abbrevs-redefine from it is problematic.  Is there a particular
>> reason why you put the call to edit-abbrevs-redefine in
>> write-abbrev-file rather than in abbrev-save-buffer.
> Good point. I missed it. I want C-x C-w to also redefine the abbrevs if
> it is called in that editing buffer. Do you mind if I do something like
> the following in write-abbrev-file:

>  (and (derived-mode-p 'edit-abbrevs-mode)
>       (edit-abbrevs-redefine))

I'd rather call edit-abbrevs-redefine from the new command you'll bind
to C-x C-w.


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-28 15:09                   ` Stefan Monnier
@ 2011-03-29  0:35                     ` Leo
  2011-03-29  3:31                       ` Stefan Monnier
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2011-03-29  0:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

On 2011-03-28 23:09 +0800, Stefan Monnier wrote:
> I'd rather call edit-abbrevs-redefine from the new command you'll bind
> to C-x C-w.

Do you mean 'C-x C-s'? C-x C-w is bound to write-abbrev-file in the
patch? If so, I guess it makes sense not to bind C-x C-w.

Leo





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-29  0:35                     ` Leo
@ 2011-03-29  3:31                       ` Stefan Monnier
  2011-03-29  4:41                         ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-29  3:31 UTC (permalink / raw)
  To: Leo; +Cc: 5937

>> I'd rather call edit-abbrevs-redefine from the new command you'll bind
>> to C-x C-w.
> Do you mean 'C-x C-s'? C-x C-w is bound to write-abbrev-file in the
> patch? If so, I guess it makes sense not to bind C-x C-w.

Oh, I didn't notice this part of the patch.  So I guess what I'm
suggesting is to create a new command for use in C-x C-w which calls
edit-abbrevs-redefine and then calls interactively write-abbrev-file.
The issue with write-abbrev-file is that it's called from
save-some-buffers so we can't/shouldn't bind it to edit-abbrevs.

Maybe another solution is to change edit-abbrevs-mode so that
save-buffer and write-file work in it by calling internally
write-abbrev-file.  Note sure if that can be made to work without major
surgery, tho (write-region-annotate-functions could do some of it, but
I think the lack of buffer-file-name setting will get in the way, so
we'd have to change edit-abbrevs so that it sets buffer-file-name and
is really turned into a major mode to edit the .abbrev-defs file, which
seems like a fairly different beast).


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-29  3:31                       ` Stefan Monnier
@ 2011-03-29  4:41                         ` Leo
  2011-03-29  5:16                           ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2011-03-29  4:41 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 5937

On 2011-03-29 11:31 +0800, Stefan Monnier wrote:
> Oh, I didn't notice this part of the patch.  So I guess what I'm
> suggesting is to create a new command for use in C-x C-w which calls
> edit-abbrevs-redefine and then calls interactively write-abbrev-file.
> The issue with write-abbrev-file is that it's called from
> save-some-buffers so we can't/shouldn't bind it to edit-abbrevs.

Thanks for that. I forgot write-abbrev-file is also called elsewhere.
How about this patch?


diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index 0a3e1fd1..b1d9e024 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -73,7 +73,8 @@ (put 'abbrev-mode 'safe-local-variable 'booleanp)
 \f
 (defvar edit-abbrevs-map
   (let ((map (make-sparse-keymap)))
-    (define-key map "\C-x\C-s" 'edit-abbrevs-redefine)
+    (define-key map "\C-x\C-s" 'abbrev-save-buffer)
+    (define-key map "\C-x\C-w" 'abbrev-save-to-file)
     (define-key map "\C-c\C-c" 'edit-abbrevs-redefine)
     map)
   "Keymap used in `edit-abbrevs'.")
@@ -224,13 +225,15 @@ (defun quietly-read-abbrev-file (&optional file)
 					;(interactive "fRead abbrev file: ")
   (read-abbrev-file file t))
 
-(defun write-abbrev-file (&optional file)
+(defun write-abbrev-file (&optional file verbose)
   "Write all user-level abbrev definitions to a file of Lisp code.
 This does not include system abbrevs; it includes only the abbrev tables
 listed in listed in `abbrev-table-name-list'.
 The file written can be loaded in another session to define the same abbrevs.
 The argument FILE is the file name to write.  If omitted or nil, the file
-specified in `abbrev-file-name' is used."
+specified in `abbrev-file-name' is used.
+If VERBOSE is non-nil, display a message indicating where abbrevs
+have been saved."
   (interactive
    (list
     (read-file-name "Write abbrev file: "
@@ -260,7 +263,28 @@ (defun write-abbrev-file (&optional file)
 		'emacs-mule)))
       (goto-char (point-min))
       (insert (format ";;-*-coding: %s;-*-\n" coding-system-for-write))
-      (write-region nil nil file nil 0))))
+      (write-region nil nil file nil (and (not verbose) 0)))))
+
+(defun abbrev-save-buffer ()
+  "Save all user-level abbrev definitions in current buffer.
+The saved abbrevs are written to the file specified by
+`abbrev-file-name'."
+  (interactive)
+  (and (derived-mode-p 'edit-abbrevs-mode)
+       (edit-abbrevs-redefine))
+  (write-abbrev-file abbrev-file-name t))
+
+(defun abbrev-save-to-file (file)
+  "Save all user-level abbrev definitions in current buffer to FILE."
+  (interactive
+   (list (read-file-name "Save abbrevs to file: "
+			 (file-name-directory
+			  (expand-file-name abbrev-file-name))
+			 abbrev-file-name)))
+  (and (derived-mode-p 'edit-abbrevs-mode)
+       (edit-abbrevs-redefine))
+  (write-abbrev-file file t))
+
 \f
 (defun add-mode-abbrev (arg)
   "Define mode-specific abbrev for last word(s) before point.

> Maybe another solution is to change edit-abbrevs-mode so that
> save-buffer and write-file work in it by calling internally
> write-abbrev-file.  Note sure if that can be made to work without major
> surgery, tho (write-region-annotate-functions could do some of it, but
> I think the lack of buffer-file-name setting will get in the way, so
> we'd have to change edit-abbrevs so that it sets buffer-file-name and
> is really turned into a major mode to edit the .abbrev-defs file, which
> seems like a fairly different beast).
>
>
>         Stefan

This isn't cleaner at the moment. So probably leave it for future
improvement.

Leo





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-29  4:41                         ` Leo
@ 2011-03-29  5:16                           ` Leo
  2011-03-29 13:49                             ` Stefan Monnier
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2011-03-29  5:16 UTC (permalink / raw)
  To: bug-gnu-emacs

On 2011-03-29 12:41 +0800, Leo wrote:
> +(defun abbrev-save-buffer ()
> +  "Save all user-level abbrev definitions in current buffer.
> +The saved abbrevs are written to the file specified by
> +`abbrev-file-name'."
> +  (interactive)
> +  (and (derived-mode-p 'edit-abbrevs-mode)
> +       (edit-abbrevs-redefine))
> +  (write-abbrev-file abbrev-file-name t))
> +
> +(defun abbrev-save-to-file (file)
> +  "Save all user-level abbrev definitions in current buffer to FILE."
> +  (interactive
> +   (list (read-file-name "Save abbrevs to file: "
> +			 (file-name-directory
> +			  (expand-file-name abbrev-file-name))
> +			 abbrev-file-name)))
> +  (and (derived-mode-p 'edit-abbrevs-mode)
> +       (edit-abbrevs-redefine))
> +  (write-abbrev-file file t))

Should define abbrev-save-buffer as:

(defun abbrev-save-buffer ()
  "Save all user-level abbrev definitions in current buffer.
The saved abbrevs are written to the file specified by
`abbrev-file-name'."
  (interactive)
  (abbrev-save-to-file abbrev-file-name))






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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-29  5:16                           ` Leo
@ 2011-03-29 13:49                             ` Stefan Monnier
  2011-03-29 15:42                               ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-29 13:49 UTC (permalink / raw)
  To: Leo; +Cc: bug-gnu-emacs

>> +(defun abbrev-save-buffer ()
>> +  "Save all user-level abbrev definitions in current buffer.
>> +The saved abbrevs are written to the file specified by
>> +`abbrev-file-name'."
>> +  (interactive)
>> +  (and (derived-mode-p 'edit-abbrevs-mode)
>> +       (edit-abbrevs-redefine))

No need for the derived-mode-p check.  But this reminds me: the name
should include "edit" (e.g. abbrev-edit-save-buffer) to make it clear
that it's about edit-abbrevs.

>> +(defun abbrev-save-to-file (file)

Same here.

>> +  "Save all user-level abbrev definitions in current buffer to FILE."
>> +  (interactive
>> +   (list (read-file-name "Save abbrevs to file: "
>> +			 (file-name-directory
>> +			  (expand-file-name abbrev-file-name))
>> +			 abbrev-file-name)))
>> +  (and (derived-mode-p 'edit-abbrevs-mode)
>> +       (edit-abbrevs-redefine))
>> +  (write-abbrev-file file t))

> Should define abbrev-save-buffer as:

> (defun abbrev-save-buffer ()
>   "Save all user-level abbrev definitions in current buffer.
> The saved abbrevs are written to the file specified by
> `abbrev-file-name'."
>   (interactive)
>   (abbrev-save-to-file abbrev-file-name))

Yes, that sounds like a good idea.


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-29 13:49                             ` Stefan Monnier
@ 2011-03-29 15:42                               ` Leo
  2011-03-29 20:54                                 ` Stefan Monnier
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2011-03-29 15:42 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: bug-gnu-emacs

Hopefully this patch is ready to commit.

diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index 0a3e1fd1..fbc4bdbd 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -73,7 +73,8 @@ (put 'abbrev-mode 'safe-local-variable 'booleanp)
 \f
 (defvar edit-abbrevs-map
   (let ((map (make-sparse-keymap)))
-    (define-key map "\C-x\C-s" 'edit-abbrevs-redefine)
+    (define-key map "\C-x\C-s" 'abbrev-edit-save-buffer)
+    (define-key map "\C-x\C-w" 'abbrev-edit-save-to-file)
     (define-key map "\C-c\C-c" 'edit-abbrevs-redefine)
     map)
   "Keymap used in `edit-abbrevs'.")
@@ -224,13 +225,15 @@ (defun quietly-read-abbrev-file (&optional file)
 					;(interactive "fRead abbrev file: ")
   (read-abbrev-file file t))
 
-(defun write-abbrev-file (&optional file)
+(defun write-abbrev-file (&optional file verbose)
   "Write all user-level abbrev definitions to a file of Lisp code.
 This does not include system abbrevs; it includes only the abbrev tables
 listed in listed in `abbrev-table-name-list'.
 The file written can be loaded in another session to define the same abbrevs.
 The argument FILE is the file name to write.  If omitted or nil, the file
-specified in `abbrev-file-name' is used."
+specified in `abbrev-file-name' is used.
+If VERBOSE is non-nil, display a message indicating where abbrevs
+have been saved."
   (interactive
    (list
     (read-file-name "Write abbrev file: "
@@ -260,7 +263,25 @@ (defun write-abbrev-file (&optional file)
 		'emacs-mule)))
       (goto-char (point-min))
       (insert (format ";;-*-coding: %s;-*-\n" coding-system-for-write))
-      (write-region nil nil file nil 0))))
+      (write-region nil nil file nil (and (not verbose) 0)))))
+
+(defun abbrev-edit-save-to-file (file)
+  "Save all user-level abbrev definitions in current buffer to FILE."
+  (interactive
+   (list (read-file-name "Save abbrevs to file: "
+			 (file-name-directory
+			  (expand-file-name abbrev-file-name))
+			 abbrev-file-name)))
+  (edit-abbrevs-redefine)
+  (write-abbrev-file file t))
+
+(defun abbrev-edit-save-buffer ()
+  "Save all user-level abbrev definitions in current buffer.
+The saved abbrevs are written to the file specified by
+`abbrev-file-name'."
+  (interactive)
+  (abbrev-edit-save-to-file abbrev-file-name))
+
 \f
 (defun add-mode-abbrev (arg)
   "Define mode-specific abbrev for last word(s) before point.






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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-29 15:42                               ` Leo
@ 2011-03-29 20:54                                 ` Stefan Monnier
  2011-03-30  1:08                                   ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2011-03-29 20:54 UTC (permalink / raw)
  To: Leo; +Cc: bug-gnu-emacs

> Hopefully this patch is ready to commit.

Looks good, thanks,


        Stefan





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

* bug#5937: 23.1.95; why saving empty abbrev tables
  2011-03-29 20:54                                 ` Stefan Monnier
@ 2011-03-30  1:08                                   ` Leo
  0 siblings, 0 replies; 29+ messages in thread
From: Leo @ 2011-03-30  1:08 UTC (permalink / raw)
  To: 5937-done

Version: 24.1





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

end of thread, other threads:[~2011-03-30  1:08 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-12 15:23 bug#5937: 23.1.95; why saving empty abbrev tables Leo
2010-04-12 18:32 ` Stefan Monnier
2010-04-15 10:26   ` Leo
2010-04-15 12:44     ` Stefan Monnier
2010-04-16 10:36   ` Leo
2010-04-27  3:49     ` Stefan Monnier
2010-04-27  8:46       ` Leo
2010-04-27 10:12         ` Leo
2010-04-27 10:32         ` Leo
2011-03-27 20:40           ` Stefan Monnier
2011-03-28  4:45             ` Leo
2011-03-28 13:58               ` Stefan Monnier
2011-03-28 14:26                 ` Leo
2011-03-28 15:09                   ` Stefan Monnier
2011-03-29  0:35                     ` Leo
2011-03-29  3:31                       ` Stefan Monnier
2011-03-29  4:41                         ` Leo
2011-03-29  5:16                           ` Leo
2011-03-29 13:49                             ` Stefan Monnier
2011-03-29 15:42                               ` Leo
2011-03-29 20:54                                 ` Stefan Monnier
2011-03-30  1:08                                   ` Leo
2011-03-27  5:09       ` Leo
2011-03-27 17:34         ` Andreas Röhler
2011-03-28  3:38           ` Leo
2011-03-27 20:35         ` Stefan Monnier
2011-03-28  4:03           ` Leo
2011-03-28 14:02             ` Stefan Monnier
2011-03-28 14:40               ` Leo

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