Improve specificity of errors in validate-txexpr #15

Merged
otherjoel merged 2 commits from recursive-validate into master 3 years ago
otherjoel commented 3 years ago (Migrated from github.com)

Makes validate-texpr recursive and tries to be as specific as possible about the problem:

;; This txexpr has two problems
(validate-txexpr `(fine-outer ((id "top") (class "42")) (div (p (span [["type" "1"]] ,(void) )))))
validate-txexpr: attribute key is not a symbol
  attribute key: "type"
  in: '(span (("type" "1")) #<void>)

;; After fixing the first one
(validate-txexpr `(fine-outer ((id "top") (class "42")) (div (p (span [[type "1"]] ,(void) )))))
validate-txexpr: element not a valid element (= txexpr, string, symbol, XML char, or cdata)
  element: #<void>
  in: '(span ((type "1")) #<void>)

Also fixes cases where errors can sometimes be misleading; for example, in the current version:

;; Problem is malformed attrs in p tag, but error points at 1st element of fine-outer
(validate-txexpr  '(fine-outer "no problem" "\n" (p [[href "#"] class "hidden"] "Hello!")))
validate-txexpr-attrs: contract violation
  expected: 
   in '(fine-outer "no problem" "\n" (p ((href "#") class "hidden") "Hello!")), list of 
attributes, each in the form '(symbol "string")
  given: "no problem"

In this version:

(validate-txexpr  '(fine-outer "no problem" "\n" (p [[href "#"] class "hidden"] "Hello!")))
validate-txexpr: attribute is not a list of the form '(symbol "string")
  attribute: 'class
  in: '(p ((href "#") class "hidden") "Hello!")
Makes `validate-texpr` recursive and tries to be as specific as possible about the problem: ```racket ;; This txexpr has two problems (validate-txexpr `(fine-outer ((id "top") (class "42")) (div (p (span [["type" "1"]] ,(void) ))))) validate-txexpr: attribute key is not a symbol attribute key: "type" in: '(span (("type" "1")) #<void>) ;; After fixing the first one (validate-txexpr `(fine-outer ((id "top") (class "42")) (div (p (span [[type "1"]] ,(void) ))))) validate-txexpr: element not a valid element (= txexpr, string, symbol, XML char, or cdata) element: #<void> in: '(span ((type "1")) #<void>) ``` Also fixes cases where errors can sometimes be misleading; for example, in the current version: ```racket ;; Problem is malformed attrs in p tag, but error points at 1st element of fine-outer (validate-txexpr '(fine-outer "no problem" "\n" (p [[href "#"] class "hidden"] "Hello!"))) validate-txexpr-attrs: contract violation expected: in '(fine-outer "no problem" "\n" (p ((href "#") class "hidden") "Hello!")), list of attributes, each in the form '(symbol "string") given: "no problem" ``` In this version: ```racket (validate-txexpr '(fine-outer "no problem" "\n" (p [[href "#"] class "hidden"] "Hello!"))) validate-txexpr: attribute is not a list of the form '(symbol "string") attribute: 'class in: '(p ((href "#") class "hidden") "Hello!") ```
mbutterick commented 3 years ago (Migrated from github.com)

Thanks. I like this. Could you add a few tests however?

  1. Test that a certain flaw raises the expected descriptive message.

  2. Test that a certain flaw raises the same message regardless of where it appears (e.g., even deeply nested).

  3. Would be possible or desirable to have a version of validate-txexpr (or a keyword option) that checks the whole txexpr for errors, and reports the list of errors at the end (as opposed to bailing out after the first error, which is the right behavior in most cases, but I can see the other kind being useful too)

Thanks. I like this. Could you add a few tests however? 1. Test that a certain flaw raises the expected descriptive message. 2. Test that a certain flaw raises the same message regardless of where it appears (e.g., even deeply nested). 3. Would be possible or desirable to have a version of `validate-txexpr` (or a keyword option) that checks the whole txexpr for errors, and reports the list of errors at the end (as opposed to bailing out after the first error, which is the right behavior in most cases, but I can see the other kind being useful too)
otherjoel commented 3 years ago (Migrated from github.com)
  1. Test that a certain flaw raises the expected descriptive message.
  2. Test that a certain flaw raises the same message regardless of where it appears (e.g., even deeply nested).

Would it make sense to replace the existing tests with these?

> 1. Test that a certain flaw raises the expected descriptive message. > 2. Test that a certain flaw raises the same message regardless of where it appears (e.g., even deeply nested). Would it make sense to replace the existing tests with these?
mbutterick commented 3 years ago (Migrated from github.com)

Add rather than replace, I think (to ensure that the upgrade doesn’t change the existing behavior))

Add rather than replace, I think (to ensure that the upgrade doesn’t change the existing behavior))
mbutterick commented 3 years ago (Migrated from github.com)

Great thanks!

Great thanks!
The pull request has been merged as 435c6e6f36.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b recursive-validate master
git pull origin recursive-validate

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff recursive-validate
git push origin master
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: mbutterick/txexpr#15
Loading…
There is no content yet.