[COMMIT] Be clearer in our error messages, #'canonicalize-inst-pair, #'canonicalize-spec

APPROVE COMMIT NOTE: This patch has been committed. # HG changeset patch # User Aidan Kehoe <kehoea(a)parhasard.net&gt; # Date 1281883365 -3600 # Node ID 04811a268716dbb77524e2ab2364a243a1c5c29c # Parent 808131ba4a5737d4eb0ecee301c98cba84dab91c Be clearer in our error messages, #'canonicalize-inst-pair, #'canonicalize-spec lisp/ChangeLog addition: 2010-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; * specifier.el (canonicalize-inst-pair, canonicalize-spec): If a specifier tag set is correct, but an instantiator is not in an accepted format, don't error with the message "Invalid specifier tag set". Also, when we error, use error-symbols, for better structured error handling and more ease when testing. tests/ChangeLog addition: 2010-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; * automated/lisp-tests.el: (not, not, invalid-argument, invalid-argument): Check that error messages from the image specifier instantiator code are clearer than they used to be. diff -r 808131ba4a57 -r 04811a268716 lisp/ChangeLog --- a/lisp/ChangeLog Sun Aug 15 13:29:10 2010 +0100 +++ b/lisp/ChangeLog Sun Aug 15 15:42:45 2010 +0100 ＠＠ -1,3 +1,12 ＠＠ +2010-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * specifier.el (canonicalize-inst-pair, canonicalize-spec): + If a specifier tag set is correct, but an instantiator is not in + an accepted format, don't error with the message "Invalid + specifier tag set". + Also, when we error, use error-symbols, for better structured + error handling and more ease when testing. + 2010-07-24 Aidan Kehoe <kehoea(a)parhasard.net&gt; * cl-extra.el (concatenate): diff -r 808131ba4a57 -r 04811a268716 lisp/specifier.el --- a/lisp/specifier.el Sun Aug 15 13:29:10 2010 +0100 +++ b/lisp/specifier.el Sun Aug 15 15:42:45 2010 +0100 ＠＠ -105,20 +105,23 ＠＠ ;; this will signal an appropriate error. (check-valid-instantiator inst-pair specifier-type))) - ((and (valid-specifier-tag-p (car inst-pair)) - (valid-instantiator-p (cdr inst-pair) specifier-type)) + ((not (valid-instantiator-p (cdr inst-pair) specifier-type)) + (if noerror + t + (check-valid-instantiator (cdr inst-pair) specifier-type))) + + ((valid-specifier-tag-p (car inst-pair)) ;; case (b) (cons (list (car inst-pair)) (cdr inst-pair))) - ((and (valid-specifier-tag-set-p (car inst-pair)) - (valid-instantiator-p (cdr inst-pair) specifier-type)) + ((valid-specifier-tag-set-p (car inst-pair)) ;; case (c) inst-pair) (t (if noerror t - (signal 'error (list "Invalid specifier tag set" - (car inst-pair))))))) + (error 'invalid-argument "Invalid specifier tag set" + (car inst-pair)))))) (defun canonicalize-inst-list (inst-list specifier-type &optional noerror) "Canonicalize the given INST-LIST (a list of inst-pairs). ＠＠ -199,9 +202,14 ＠＠ (if (not (valid-specifier-locale-p (car spec))) ;; invalid locale. - (if noerror t - (signal 'error (list "Invalid specifier locale" (car spec)))) - + (if noerror + t + (if (consp (car spec)) + ;; If it's a cons, they're probably not passing a locale + (error 'invalid-argument + "Not a valid instantiator list" spec) + (error 'invalid-argument + "Invalid specifier locale" (car spec)))) ;; case (b) (let ((result (canonicalize-inst-list (cdr spec) specifier-type noerror))) diff -r 808131ba4a57 -r 04811a268716 tests/ChangeLog --- a/tests/ChangeLog Sun Aug 15 13:29:10 2010 +0100 +++ b/tests/ChangeLog Sun Aug 15 15:42:45 2010 +0100 ＠＠ -1,3 +1,10 ＠＠ +2010-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + * automated/lisp-tests.el: + (not, not, invalid-argument, invalid-argument): + Check that error messages from the image specifier instantiator + code are clearer than they used to be. + 2010-08-15 Aidan Kehoe <kehoea(a)parhasard.net&gt; * automated/lisp-tests.el: diff -r 808131ba4a57 -r 04811a268716 tests/automated/lisp-tests.el --- a/tests/automated/lisp-tests.el Sun Aug 15 13:29:10 2010 +0100 +++ b/tests/automated/lisp-tests.el Sun Aug 15 15:42:45 2010 +0100 ＠＠ -2374,6 +2374,35 ＠＠ (garbage-collect)))))) "checking we can amputate lists without crashing #'reduce") +(Assert (not (eq t (canonicalize-inst-list + `(((mswindows) . [string :data ,(make-string 20 0)]) + ((tty) . [string :data " "])) 'image t))) + "checking mswindows is always available as a specifier tag") + +(Assert (not (eq t (canonicalize-inst-list + `(((mswindows) . [nothing]) + ((tty) . [string :data " "])) + 'image t))) + "checking the correct syntax for a nothing image specifier works") + +(Check-Error-Message invalid-argument "^Invalid specifier tag set" + (canonicalize-inst-list + `(((,(gensym)) . [nothing]) + ((tty) . [string :data " "])) + 'image)) + +(Check-Error-Message invalid-argument "^Unrecognized keyword" + (canonicalize-inst-list + `(((mswindows) . [nothing :data "hi there"]) + ((tty) . [string :data " "])) 'image)) + +;; If we combine both the specifier inst list problems, we get the +;; unrecognized keyword error first, not the invalid specifier tag set +;; error. This is a little unintuitive; the specifier tag set thing is +;; processed first, and would seem to be more important. But anyone writing +;; code needs to solve both problems, it's reasonable to ask them to do it +;; in series rather than in parallel. + (when (featurep 'ratio) (Assert (not (eql '1/2 (read (prin1-to-string (intern "1/2"))))) "checking symbols with ratio-like names are printed distinctly") -- “Apart from the nine-banded armadillo, man is the only natural host of Mycobacterium leprae, although it can be grown in the footpads of mice.” -- Kumar & Clark, Clinical Medicine, summarising improbable leprosy research _______________________________________________ XEmacs-Patches mailing list XEmacs-Patches(a)xemacs.org http://calypso.tux.org/mailman/listinfo/xemacs-patches