[Suggestion] Do not attempt to define empty labels on interactive field creation

[akater]

EBDB manual, node 4.1 says

When entering field data, optional data can be skipped by entering a blank string, or by pressing ‘C-g’.

However, entering blank string and pressing C-g when adding labels causes different feedback from EBDB which looks like an inconsistency.

Steps to reproduce:

1. Try creating new record interactively, e.g. M-x ebdb-create-record

2. When prompted for label (say, phone label), enter empty string.

3. Emacs then reacts with “ is not a known label, define it?” — apparently, it attempts to define an empty label which doesn't make much sense and feels contradictory to what manual says

C-g would skip adding label (and the field). Entering blank string on Mail address would have had the same effect as C-g on entering Mail address.

Looks like empty string check in cl-defmethod ebdb-read :around ((class (subclass ebdb-field-labeled)) &optional slots obj) would cause the behaviour to be the same for blank string and C-g:

diff -u /home/akater/.emacs.d/elpa/ebdb-0.4.2/ebdb.el /home/akater/.emacs.d/elpa/ebdb-0.4.2/ebdb-patched.el
--- /home/akater/.emacs.d/elpa/ebdb-0.4.2/ebdb.el   2017-11-02 04:04:42.770130514 +0000
+++ /home/akater/.emacs.d/elpa/ebdb-0.4.2/ebdb-patched.el   2017-11-03 20:06:24.207872796 +0000
@@ -1026,8 +1026,9 @@
           (and obj (slot-value obj 'object-name))
           labels nil)
        slots (plist-put slots :object-name label)))
-    (if (or (member label labels)
-       (yes-or-no-p (format "%s is not a known label, define it? " label)))
+    (if (and (not (empty-string-p label))
+        (or (member label labels)
+        (yes-or-no-p (format "%s is not a known label, define it? " label))))
    (cl-call-next-method class slots obj)
       (signal 'ebdb-empty (list class)))))

Note: this would also cause ebdb-insert-field to abort on entering empty label.

[girzel]

Note: this would also cause ebdb-insert-field to abort on entering empty label

Hmm, do you think it would be better to simply enter an empty label, or to abort field creation altogether?

[akater]

I certainly wouldn't use "" labels (empty strings) anywhere. Actually, I've been avoiding empty label just so that I wouldn't have to deal with possible hassle of unmanageable label (at worst) or possible awkward rendering of completion buffer with such label in it (at best), even though I don't care much for labels in my particular db at all!

Support for unlabeled entities would be nice though, with persistent db entry looking like this

(ebdb-field-phone nil  ;; i'm not actually familiar with ebdb objects yet
  :object-name nil     ;; so this could be totally unreasonable
  :country-code 0
  :area-code 0
  :number "000")

and empty input creating such unlabeled field. In that case though, we have to ask for the phone number (or address, or whatever is label-able) first and for label second. This looks most reasonable to me but I'm not ready to provide the necessary patch myself.

[girzel]

It wouldn't have to ask for the label second -- we could still prompt for the label first, but simply interpret "" as nil. I need to look at the code more closely, but can't see why that wouldn't work.

Interpreting an empty string as "abort" is fine when you're inputting core field values, but labels aren't core field values.

[akater]

It wouldn't have to ask for the label second -- we could still prompt for the label first, but simply interpret "" as nil.

Suppose there's a facility for adding multiple phone numbers or addresses. (As far as I can remember, bbdb had it.) When label is requested first, user has to needlessly specify a label before he or she is able to say “no more phone numbers please”.

Label is a datum secondary to the number itself. Sooner or later this reversed entry order is going to be exposed as a source of some issue.

Interpreting an empty string as "abort" is fine when you're inputting core field values, but labels aren't core field values.

Agree.

[girzel]

Suppose there's a facility for adding multiple phone numbers or addresses. (As far as I can remember, bbdb had it.) When label is requested first, user has to needlessly specify a label before he or she is able to say “no more phone numbers please”.

That's a good point, and that's exactly what happens when creating a new record: it loops on numbers and addresses.

Okay, I'll swap the order so the label comes after other field data; that won't be hard to do.

[girzel]

Also, nil values for field-instance labels are fine, all code will fall back on the :human-readable option of the class itself.