bug#29923: 25.3; write-abbrev-file inserts many empty tables

bug#29923: 25.3; write-abbrev-file inserts many empty tables

[Allen Li]

write-abbrev-file write a lot of empty tables.  e.g.

  (define-abbrev-table 'Buffer-menu-mode-abbrev-table '())

  (define-abbrev-table 'Info-edit-mode-abbrev-table '())

  (define-abbrev-table 'awk-mode-abbrev-table '())

  (define-abbrev-table 'bibtex-mode-abbrev-table '())

  (define-abbrev-table 'bookmark-bmenu-mode-abbrev-table '())

  (define-abbrev-table 'bookmark-edit-annotation-mode-abbrev-table '())

  (define-abbrev-table 'c++-mode-abbrev-table '())

  (define-abbrev-table 'c-mode-abbrev-table '())

It would be better (e.g., for managing abbrevs together with Emacs
configuration with version control) if empty tables were not written.

In GNU Emacs 25.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.26)
 of 2017-12-04 built on arojas
Windowing system distributor 'The X.Org Foundation', version 11.0.11905000
Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

bug#29923: [PATCH] Skip writing empty abbrev tables

[Allen Li]

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

Patch attached

[-- Attachment #2: 0001-Skip-writing-empty-abbrev-tables.patch --]
[-- Type: text/x-patch, Size: 3430 bytes --]

From fa3e3ed1f55fd2d973826dbd21b5b59072b86504 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 31 Dec 2017 20:33:21 -0800
Subject: [PATCH] Skip writing empty abbrev tables

Fixes bug#29923

* lisp/abbrev.el (write-abbrev-file): Pass SKIPEMPTY to
insert-abbrev-table-description.
(insert-abbrev-table-description): Add SKIPEMPTY optional
parameter. Skip inserting empty tables if SKIPEMPTY is non-nil.
---
 lisp/abbrev.el | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index 5faffd4e17..9ba114e6a0 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -34,6 +34,7 @@
 
 (eval-when-compile (require 'cl-lib))
 (require 'obarray)
+(require 'seq)
 
 (defgroup abbrev-mode nil
   "Word abbreviations mode."
@@ -254,7 +255,7 @@ write-abbrev-file
 		     (lambda (s1 s2)
 		       (string< (symbol-name s1)
 				(symbol-name s2)))))
-	(insert-abbrev-table-description table nil))
+	(insert-abbrev-table-description table nil t))
       (when (unencodable-char-position (point-min) (point-max) 'utf-8)
 	(setq coding-system-for-write
 	      (if (> emacs-major-version 24)
@@ -933,35 +934,40 @@ abbrev--describe
       (prin1 (symbol-function sym)))
     (terpri)))
 
-(defun insert-abbrev-table-description (name &optional readable)
+(defun insert-abbrev-table-description (name &optional readable skipempty)
   "Insert before point a full description of abbrev table named NAME.
 NAME is a symbol whose value is an abbrev table.
 If optional 2nd arg READABLE is non-nil, a human-readable description
 is inserted.  Otherwise the description is an expression,
 a call to `define-abbrev-table', which would
 define the abbrev table NAME exactly as it is currently defined.
+If optional arg SKIPEMPTY is non-nil, skip insertion if table is empty.
 
 Abbrevs marked as \"system abbrevs\" are omitted."
   (let ((table (symbol-value name))
         (symbols ()))
-    (mapatoms (lambda (sym) (if (symbol-value sym) (push sym symbols))) table)
-    (setq symbols (sort symbols 'string-lessp))
-    (let ((standard-output (current-buffer)))
-      (if readable
-	  (progn
-	    (insert "(")
-	    (prin1 name)
-	    (insert ")\n\n")
-	    (mapc 'abbrev--describe symbols)
-	    (insert "\n\n"))
-	(insert "(define-abbrev-table '")
-	(prin1 name)
-	(if (null symbols)
-	    (insert " '())\n\n")
-	  (insert "\n  '(\n")
-	  (mapc 'abbrev--write symbols)
-	  (insert "   ))\n\n")))
-      nil)))
+    (mapatoms (lambda (sym)
+                (if (and (symbol-value sym) (not (abbrev-get sym :system)))
+                    (push sym symbols)))
+              table)
+    (when (or symbols (not skipempty))
+      (setq symbols (sort symbols 'string-lessp))
+      (let ((standard-output (current-buffer)))
+        (if readable
+            (progn
+              (insert "(")
+              (prin1 name)
+              (insert ")\n\n")
+              (mapc 'abbrev--describe symbols)
+              (insert "\n\n"))
+          (insert "(define-abbrev-table '")
+          (prin1 name)
+          (if (null symbols)
+              (insert " '())\n\n")
+            (insert "\n  '(\n")
+            (mapc 'abbrev--write symbols)
+            (insert "   ))\n\n")))
+        nil))))
 
 (defun define-abbrev-table (tablename definitions
                                       &optional docstring &rest props)
-- 
2.15.1

bug#29923: 25.3; write-abbrev-file inserts many empty tables

[Allen Li]

Could I get this looked at?

bug#29923: [PATCH] Skip writing empty abbrev tables

[Noam Postavsky]

Allen Li <vianchielfaura@gmail.com> writes:

> Subject: [PATCH] Skip writing empty abbrev tables
>
> Fixes bug#29923
>
> * lisp/abbrev.el (write-abbrev-file): Pass SKIPEMPTY to
> insert-abbrev-table-description.
> (insert-abbrev-table-description): Add SKIPEMPTY optional
> parameter. Skip inserting empty tables if SKIPEMPTY is non-nil.

  
>  (eval-when-compile (require 'cl-lib))
>  (require 'obarray)
> +(require 'seq)

You didn't end up using seq, as far as I can tell.
  
> -(defun insert-abbrev-table-description (name &optional readable)
> +(defun insert-abbrev-table-description (name &optional readable skipempty)
>    "Insert before point a full description of abbrev table named NAME.
>  NAME is a symbol whose value is an abbrev table.
>  If optional 2nd arg READABLE is non-nil, a human-readable description
>  is inserted.  Otherwise the description is an expression,
>  a call to `define-abbrev-table', which would
>  define the abbrev table NAME exactly as it is currently defined.
> +If optional arg SKIPEMPTY is non-nil, skip insertion if table is empty.
>  
>  Abbrevs marked as \"system abbrevs\" are omitted."

> -    (mapatoms (lambda (sym) (if (symbol-value sym) (push sym symbols))) table)
> +    (mapatoms (lambda (sym)
> +                (if (and (symbol-value sym) (not (abbrev-get sym :system)))
                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Looks like the second check you added implements the "Abbrevs marked as
\"system abbrevs\" are omitted" part.  I guess that actually fixes an
additional bug?  Worth mentioning in the commit message, I think.

bug#29923: [PATCH] Skip writing empty abbrev tables

[Allen Li]

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

On Sun, Sep 9, 2018 at 1:43 PM Noam Postavsky <npostavs@gmail.com> wrote:
>
> Allen Li <vianchielfaura@gmail.com> writes:
>
> > Subject: [PATCH] Skip writing empty abbrev tables
> >
> > Fixes bug#29923
> >
> > * lisp/abbrev.el (write-abbrev-file): Pass SKIPEMPTY to
> > insert-abbrev-table-description.
> > (insert-abbrev-table-description): Add SKIPEMPTY optional
> > parameter. Skip inserting empty tables if SKIPEMPTY is non-nil.
>
>
> >  (eval-when-compile (require 'cl-lib))
> >  (require 'obarray)
> > +(require 'seq)
>
> You didn't end up using seq, as far as I can tell.

Thanks, removed.

>
> > -(defun insert-abbrev-table-description (name &optional readable)
> > +(defun insert-abbrev-table-description (name &optional readable skipempty)
> >    "Insert before point a full description of abbrev table named NAME.
> >  NAME is a symbol whose value is an abbrev table.
> >  If optional 2nd arg READABLE is non-nil, a human-readable description
> >  is inserted.  Otherwise the description is an expression,
> >  a call to `define-abbrev-table', which would
> >  define the abbrev table NAME exactly as it is currently defined.
> > +If optional arg SKIPEMPTY is non-nil, skip insertion if table is empty.
> >
> >  Abbrevs marked as \"system abbrevs\" are omitted."
>
> > -    (mapatoms (lambda (sym) (if (symbol-value sym) (push sym symbols))) table)
> > +    (mapatoms (lambda (sym)
> > +                (if (and (symbol-value sym) (not (abbrev-get sym :system)))
>                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Looks like the second check you added implements the "Abbrevs marked as
> \"system abbrevs\" are omitted" part.  I guess that actually fixes an
> additional bug?  Worth mentioning in the commit message, I think.

The system abbrev omitting worked (if readable is nil); it's
implemented in abbrev--write.
However, that doesn't allow us to skip writing a table if it only
contains system abbrevs;
we'll still see a table with abbrevs, write the opening of the
define-abbrev-table form,
and then realize in abbrev-write that all of the abbrevs are system abbrevs.

I noticed a bug in my patch where it would skip system abbrevs if
readable was non-nil
when it did not skip system abbrevs previously.  I fixed this and also
fixed the docstring
to match what I believe is the intended behavior.

[-- Attachment #2: 0001-Skip-writing-empty-abbrev-tables.patch --]
[-- Type: text/x-patch, Size: 2451 bytes --]

From 114b8ae71b8a87d4459c45bc8d571a534912a807 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 31 Dec 2017 20:33:21 -0800
Subject: [PATCH] Skip writing empty abbrev tables

Fixes bug#29923

* lisp/abbrev.el (insert-abbrev-table-description):
Skip inserting empty tables when READABLE is non-nil.
Clarify behavior in documentation string.
---
 lisp/abbrev.el | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index cddce8f529..ac01363357 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -938,27 +938,32 @@ insert-abbrev-table-description
 a call to `define-abbrev-table', which would
 define the abbrev table NAME exactly as it is currently defined.
 
-Abbrevs marked as \"system abbrevs\" are omitted."
+If READABLE is nil, abbrevs marked as \"system abbrevs\" and
+empty abbrev tables are omitted."
   (let ((table (symbol-value name))
         (symbols ()))
-    (mapatoms (lambda (sym) (if (symbol-value sym) (push sym symbols))) table)
-    (setq symbols (sort symbols 'string-lessp))
-    (let ((standard-output (current-buffer)))
-      (if readable
-	  (progn
-	    (insert "(")
-	    (prin1 name)
-	    (insert ")\n\n")
-	    (mapc 'abbrev--describe symbols)
-	    (insert "\n\n"))
-	(insert "(define-abbrev-table '")
-	(prin1 name)
-	(if (null symbols)
-	    (insert " '())\n\n")
-	  (insert "\n  '(\n")
-	  (mapc 'abbrev--write symbols)
-	  (insert "   ))\n\n")))
-      nil)))
+    (mapatoms (lambda (sym)
+                (if (and (symbol-value sym) (or readable (not (abbrev-get sym :system))))
+                    (push sym symbols)))
+              table)
+    (when symbols
+      (setq symbols (sort symbols 'string-lessp))
+      (let ((standard-output (current-buffer)))
+        (if readable
+            (progn
+              (insert "(")
+              (prin1 name)
+              (insert ")\n\n")
+              (mapc 'abbrev--describe symbols)
+              (insert "\n\n"))
+          (insert "(define-abbrev-table '")
+          (prin1 name)
+          (if (null symbols)
+              (insert " '())\n\n")
+            (insert "\n  '(\n")
+            (mapc 'abbrev--write symbols)
+            (insert "   ))\n\n")))
+        nil))))
 
 (defun define-abbrev-table (tablename definitions
                                       &optional docstring &rest props)
-- 
2.19.0.397.gdd90340f6a-goog

bug#29923: [PATCH] Skip writing empty abbrev tables

[Noam Postavsky]

Allen Li <darkfeline@felesatra.moe> writes:

> The system abbrev omitting worked (if readable is nil); it's
> implemented in abbrev--write.
> However, that doesn't allow us to skip writing a table if it only
> contains system abbrevs;
> we'll still see a table with abbrevs, write the opening of the
> define-abbrev-table form,
> and then realize in abbrev-write that all of the abbrevs are system abbrevs.

Ah, it would be good to have this information in the commit message, as
the patch is a bit confusing to read otherwise.

> I noticed a bug in my patch where it would skip system abbrevs if
> readable was non-nil
> when it did not skip system abbrevs previously.  I fixed this and also
> fixed the docstring
> to match what I believe is the intended behavior.

Okay, looks good, could you please enhance the commit message as
mentioned above?

bug#29923: [PATCH] Skip writing empty abbrev tables

[Allen Li]

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

On Tue, Sep 18, 2018 at 3:53 PM Noam Postavsky <npostavs@gmail.com> wrote:
>
> Allen Li <darkfeline@felesatra.moe> writes:
>
> > The system abbrev omitting worked (if readable is nil); it's
> > implemented in abbrev--write.
> > However, that doesn't allow us to skip writing a table if it only
> > contains system abbrevs;
> > we'll still see a table with abbrevs, write the opening of the
> > define-abbrev-table form,
> > and then realize in abbrev-write that all of the abbrevs are system abbrevs.
>
> Ah, it would be good to have this information in the commit message, as
> the patch is a bit confusing to read otherwise.

Done.  I have also removed the system abbrev check in abbrev--write since
it is redundant now and this is the only place it is called.

[-- Attachment #2: 0001-Skip-writing-empty-abbrev-tables.patch --]
[-- Type: text/x-patch, Size: 4258 bytes --]

From 29b64ffe68890ece089d5e1050c55eb7c2547d36 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 31 Dec 2017 20:33:21 -0800
Subject: [PATCH] Skip writing empty abbrev tables

Fixes bug#29923

insert-abbrev-table-description with a non-nil READABLE inserts Lisp
forms suitable for evaluation to restore the defined abbrevs.  We
don't have to insert a form for tables that do not have any abbrevs.

To implement this, we need to filter out out system abbrevs before
checking if a table is empty, because system abbrevs were previously
skipped in the abbrev--write call, at which point we would already
have started inserting the beginning of a table definition form.

* lisp/abbrev.el (insert-abbrev-table-description):
Skip inserting empty tables when READABLE is non-nil.
Clarify behavior in documentation string.
* lisp/abbrev.el (abbrev--write): Remove system abbrev check.
---
 lisp/abbrev.el | 75 ++++++++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index cddce8f529..fd44cce4ae 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -896,24 +896,22 @@ unexpand-abbrev
 
 (defun abbrev--write (sym)
   "Write the abbrev in a `read'able form.
-Only writes the non-system abbrevs.
 Presumes that `standard-output' points to `current-buffer'."
-  (unless (or (null (symbol-value sym)) (abbrev-get sym :system))
-    (insert "    (")
-    (prin1 (symbol-name sym))
-    (insert " ")
-    (prin1 (symbol-value sym))
-    (insert " ")
-    (prin1 (symbol-function sym))
-    (insert " :count ")
-    (prin1 (abbrev-get sym :count))
-    (when (abbrev-get sym :case-fixed)
-      (insert " :case-fixed ")
-      (prin1 (abbrev-get sym :case-fixed)))
-    (when (abbrev-get sym :enable-function)
-      (insert " :enable-function ")
-      (prin1 (abbrev-get sym :enable-function)))
-    (insert ")\n")))
+  (insert "    (")
+  (prin1 (symbol-name sym))
+  (insert " ")
+  (prin1 (symbol-value sym))
+  (insert " ")
+  (prin1 (symbol-function sym))
+  (insert " :count ")
+  (prin1 (abbrev-get sym :count))
+  (when (abbrev-get sym :case-fixed)
+    (insert " :case-fixed ")
+    (prin1 (abbrev-get sym :case-fixed)))
+  (when (abbrev-get sym :enable-function)
+    (insert " :enable-function ")
+    (prin1 (abbrev-get sym :enable-function)))
+  (insert ")\n"))
 
 (defun abbrev--describe (sym)
   (when (symbol-value sym)
@@ -938,27 +936,32 @@ insert-abbrev-table-description
 a call to `define-abbrev-table', which would
 define the abbrev table NAME exactly as it is currently defined.
 
-Abbrevs marked as \"system abbrevs\" are omitted."
+If READABLE is nil, abbrevs marked as \"system abbrevs\" and
+empty abbrev tables are omitted."
   (let ((table (symbol-value name))
         (symbols ()))
-    (mapatoms (lambda (sym) (if (symbol-value sym) (push sym symbols))) table)
-    (setq symbols (sort symbols 'string-lessp))
-    (let ((standard-output (current-buffer)))
-      (if readable
-	  (progn
-	    (insert "(")
-	    (prin1 name)
-	    (insert ")\n\n")
-	    (mapc 'abbrev--describe symbols)
-	    (insert "\n\n"))
-	(insert "(define-abbrev-table '")
-	(prin1 name)
-	(if (null symbols)
-	    (insert " '())\n\n")
-	  (insert "\n  '(\n")
-	  (mapc 'abbrev--write symbols)
-	  (insert "   ))\n\n")))
-      nil)))
+    (mapatoms (lambda (sym)
+                (if (and (symbol-value sym) (or readable (not (abbrev-get sym :system))))
+                    (push sym symbols)))
+              table)
+    (when symbols
+      (setq symbols (sort symbols 'string-lessp))
+      (let ((standard-output (current-buffer)))
+        (if readable
+            (progn
+              (insert "(")
+              (prin1 name)
+              (insert ")\n\n")
+              (mapc 'abbrev--describe symbols)
+              (insert "\n\n"))
+          (insert "(define-abbrev-table '")
+          (prin1 name)
+          (if (null symbols)
+              (insert " '())\n\n")
+            (insert "\n  '(\n")
+            (mapc 'abbrev--write symbols)
+            (insert "   ))\n\n")))
+        nil))))
 
 (defun define-abbrev-table (tablename definitions
                                       &optional docstring &rest props)
-- 
2.19.0

bug#29923: [PATCH] Skip writing empty abbrev tables

[Andreas Röhler]

On 19.09.2018 04:55, Allen Li wrote:
> On Tue, Sep 18, 2018 at 3:53 PM Noam Postavsky <npostavs@gmail.com> wrote:
>>
>> Allen Li <darkfeline@felesatra.moe> writes:
>>
>>> The system abbrev omitting worked (if readable is nil); it's
>>> implemented in abbrev--write.
>>> However, that doesn't allow us to skip writing a table if it only
>>> contains system abbrevs;
>>> we'll still see a table with abbrevs, write the opening of the
>>> define-abbrev-table form,
>>> and then realize in abbrev-write that all of the abbrevs are system abbrevs.
>>
>> Ah, it would be good to have this information in the commit message, as
>> the patch is a bit confusing to read otherwise.
> 
> Done.  I have also removed the system abbrev check in abbrev--write since
> it is redundant now and this is the only place it is called.
> 

AFAIU

add-mode-abbrev expect a respective abbrev-table existing.

(add-abbrev
    (if only-global-abbrevs
        global-abbrev-table
      (or local-abbrev-table
	 (error "No per-mode abbrev table")))

Will this still work?

Cheers,
Andreas

bug#29923: [PATCH] Skip writing empty abbrev tables

[Eli Zaretskii]

> From: Allen Li <darkfeline@felesatra.moe>
> Date: Tue, 18 Sep 2018 19:55:38 -0700
> Cc: Allen Li <vianchielfaura@gmail.com>, 29923@debbugs.gnu.org
> 
> > Ah, it would be good to have this information in the commit message, as
> > the patch is a bit confusing to read otherwise.
> 
> Done.  I have also removed the system abbrev check in abbrev--write since
> it is redundant now and this is the only place it is called.

I think this change in behavior warrants a NEWS entry, and I think the
description in the ELisp manual should also be updated.

> To implement this, we need to filter out out system abbrevs before
                                       ^^^^^^^
A typo.

Thanks.

bug#29923: [PATCH] Skip writing empty abbrev tables

[Allen Li]

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

Attached new patch

On Wed, Sep 19, 2018 at 6:44 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Allen Li <darkfeline@felesatra.moe>
> > Date: Tue, 18 Sep 2018 19:55:38 -0700
> > Cc: Allen Li <vianchielfaura@gmail.com>, 29923@debbugs.gnu.org
> >
> > > Ah, it would be good to have this information in the commit message, as
> > > the patch is a bit confusing to read otherwise.
> >
> > Done.  I have also removed the system abbrev check in abbrev--write since
> > it is redundant now and this is the only place it is called.
>
> I think this change in behavior warrants a NEWS entry, and I think the
> description in the ELisp manual should also be updated.
>
> > To implement this, we need to filter out out system abbrevs before
>                                        ^^^^^^^
> A typo.

Done

>
> Thanks.

[-- Attachment #2: 0001-Skip-writing-empty-abbrev-tables.patch --]
[-- Type: text/x-patch, Size: 6979 bytes --]

From 1510152169b6cc8b5cb1d0daa59578cec1373f13 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 31 Dec 2017 20:33:21 -0800
Subject: [PATCH] Skip writing empty abbrev tables

Fixes bug#29923

insert-abbrev-table-description with a non-nil READABLE inserts Lisp
forms suitable for evaluation to restore the defined abbrevs.  We
don't have to insert a form for tables that do not have any abbrevs.

To implement this, we need to filter out system abbrevs before
checking if a table is empty, because system abbrevs were previously
skipped in the abbrev--write call, at which point we would already
have started inserting the beginning of a table definition form.

* lisp/abbrev.el (insert-abbrev-table-description):
Skip inserting empty tables when READABLE is non-nil.
Clarify behavior in documentation string.
* lisp/abbrev.el (abbrev--write): Remove system abbrev check.
---
 doc/lispref/abbrevs.texi | 16 ++++----
 etc/NEWS                 |  6 +++
 lisp/abbrev.el           | 85 +++++++++++++++++++++-------------------
 3 files changed, 60 insertions(+), 47 deletions(-)

diff --git a/doc/lispref/abbrevs.texi b/doc/lispref/abbrevs.texi
index 087e694520..32d1d8d036 100644
--- a/doc/lispref/abbrevs.texi
+++ b/doc/lispref/abbrevs.texi
@@ -112,17 +112,19 @@ Abbrev Tables
 @code{define-abbrev-table} adds the new abbrev table name to this list.
 @end defvar
 
-@defun insert-abbrev-table-description name &optional human
+@defun insert-abbrev-table-description name &optional readable
 This function inserts before point a description of the abbrev table
 named @var{name}.  The argument @var{name} is a symbol whose value is an
 abbrev table.  @c The return value is always @code{nil}.
 
-If @var{human} is non-@code{nil}, the description is human-oriented.
-System abbrevs are listed and identified as such.  Otherwise the
-description is a Lisp expression---a call to @code{define-abbrev-table}
-that would define @var{name} as it is currently defined, but without
-the system abbrevs.  (The mode or package using @var{name} is supposed
-to add these to @var{name} separately.)
+If @var{readable} is non-@code{nil}, the description is
+human-oriented.  System abbrevs are listed and identified as such.
+Otherwise the description is a Lisp expression, a call to
+@code{define-abbrev-table}, that would define @var{name} as it is
+currently defined, but without the system abbrevs (the mode or package
+using @var{name} is supposed to add these to @var{name} separately).
+However, if the resulting expression would not define any abbrevs,
+nothing is inserted.
 @end defun
 
 @node Defining Abbrevs
diff --git a/etc/NEWS b/etc/NEWS
index bc6791b05b..9032f90643 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -246,6 +246,12 @@ case does not match.
 'write-abbrev-file' now writes special properties like ':case-fixed'
 for abbrevs that have them.
 
++++
+** 'insert-abbrev-table-description' skips empty tables.
+'insert-abbrev-table-description' skips inserting empty tables when
+inserting non-readable tables.  By extension, this makes
+'write-abbrev-file' skip writing empty tables.
+
 +++
 ** The new functions and commands 'text-property-search-forward' and
 'text-property-search-backward' have been added.  These provide an
diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index cddce8f529..e1fd366ba9 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -896,24 +896,22 @@ unexpand-abbrev
 
 (defun abbrev--write (sym)
   "Write the abbrev in a `read'able form.
-Only writes the non-system abbrevs.
 Presumes that `standard-output' points to `current-buffer'."
-  (unless (or (null (symbol-value sym)) (abbrev-get sym :system))
-    (insert "    (")
-    (prin1 (symbol-name sym))
-    (insert " ")
-    (prin1 (symbol-value sym))
-    (insert " ")
-    (prin1 (symbol-function sym))
-    (insert " :count ")
-    (prin1 (abbrev-get sym :count))
-    (when (abbrev-get sym :case-fixed)
-      (insert " :case-fixed ")
-      (prin1 (abbrev-get sym :case-fixed)))
-    (when (abbrev-get sym :enable-function)
-      (insert " :enable-function ")
-      (prin1 (abbrev-get sym :enable-function)))
-    (insert ")\n")))
+  (insert "    (")
+  (prin1 (symbol-name sym))
+  (insert " ")
+  (prin1 (symbol-value sym))
+  (insert " ")
+  (prin1 (symbol-function sym))
+  (insert " :count ")
+  (prin1 (abbrev-get sym :count))
+  (when (abbrev-get sym :case-fixed)
+    (insert " :case-fixed ")
+    (prin1 (abbrev-get sym :case-fixed)))
+  (when (abbrev-get sym :enable-function)
+    (insert " :enable-function ")
+    (prin1 (abbrev-get sym :enable-function)))
+  (insert ")\n"))
 
 (defun abbrev--describe (sym)
   (when (symbol-value sym)
@@ -934,31 +932,38 @@ insert-abbrev-table-description
   "Insert before point a full description of abbrev table named NAME.
 NAME is a symbol whose value is an abbrev table.
 If optional 2nd arg READABLE is non-nil, a human-readable description
-is inserted.  Otherwise the description is an expression,
-a call to `define-abbrev-table', which would
-define the abbrev table NAME exactly as it is currently defined.
-
-Abbrevs marked as \"system abbrevs\" are omitted."
+is inserted.
+
+If READABLE is nil, an expression is inserted.  The expression is
+a call to `define-abbrev-table' that when evaluated will define
+the abbrev table NAME exactly as it is currently defined.
+Abbrevs marked as \"system abbrevs\" are ignored.  If the
+resulting expression would not define any abbrevs, nothing is
+inserted."
   (let ((table (symbol-value name))
         (symbols ()))
-    (mapatoms (lambda (sym) (if (symbol-value sym) (push sym symbols))) table)
-    (setq symbols (sort symbols 'string-lessp))
-    (let ((standard-output (current-buffer)))
-      (if readable
-	  (progn
-	    (insert "(")
-	    (prin1 name)
-	    (insert ")\n\n")
-	    (mapc 'abbrev--describe symbols)
-	    (insert "\n\n"))
-	(insert "(define-abbrev-table '")
-	(prin1 name)
-	(if (null symbols)
-	    (insert " '())\n\n")
-	  (insert "\n  '(\n")
-	  (mapc 'abbrev--write symbols)
-	  (insert "   ))\n\n")))
-      nil)))
+    (mapatoms (lambda (sym)
+                (if (and (symbol-value sym) (or readable (not (abbrev-get sym :system))))
+                    (push sym symbols)))
+              table)
+    (when symbols
+      (setq symbols (sort symbols 'string-lessp))
+      (let ((standard-output (current-buffer)))
+        (if readable
+            (progn
+              (insert "(")
+              (prin1 name)
+              (insert ")\n\n")
+              (mapc 'abbrev--describe symbols)
+              (insert "\n\n"))
+          (insert "(define-abbrev-table '")
+          (prin1 name)
+          (if (null symbols)
+              (insert " '())\n\n")
+            (insert "\n  '(\n")
+            (mapc 'abbrev--write symbols)
+            (insert "   ))\n\n")))
+        nil))))
 
 (defun define-abbrev-table (tablename definitions
                                       &optional docstring &rest props)
-- 
2.19.0

bug#29923: [PATCH] Skip writing empty abbrev tables

[Allen Li]

On Wed, Sep 19, 2018 at 5:59 AM Andreas Röhler
<andreas.roehler@easy-emacs.de> wrote:
>
> AFAIU
>
> add-mode-abbrev expect a respective abbrev-table existing.
>
> (add-abbrev
>     (if only-global-abbrevs
>         global-abbrev-table
>       (or local-abbrev-table
>          (error "No per-mode abbrev table")))
>
> Will this still work?

add-mode-abbrev is unaffected.  Not writing empty abbrev tables is the
same as if the user doesn't have an abbrev file in the first place,
which obviously doesn't break add-mode-abbrev.

>
> Cheers,
> Andreas
>
>

bug#29923: [PATCH] Skip writing empty abbrev tables

[Eli Zaretskii]

> From: Allen Li <darkfeline@felesatra.moe>
> Date: Wed, 26 Sep 2018 09:01:45 +0000
> Cc: Noam Postavsky <npostavs@gmail.com>, Allen Li <vianchielfaura@gmail.com>, 29923@debbugs.gnu.org
> 
> Attached new patch

Thanks, I pushed this with a couple of minor changes.

Please in the future mention all changed files in the log message.