Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loose and strict validation #314

Closed
ribose-jeffreylau opened this issue Jul 12, 2021 · 11 comments
Closed

Loose and strict validation #314

ribose-jeffreylau opened this issue Jul 12, 2021 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@ribose-jeffreylau
Copy link

Relevant line in build log: https://github.com/CalConnect/csd-transcription-systems/runs/3043780696?check_suite_focus=true#step:5:550

The above seems to cause the expected *.presentation.xml to not build, thus failing to output subsequent PDF / HTML / DOC files. (Strangely enough, the process exited with 0.)

Only happens after isodoc has been upgraded from 1.6.7.* to 1.7.0.

@ribose-jeffreylau ribose-jeffreylau added the bug Something isn't working label Jul 12, 2021
@opoudjis
Copy link
Contributor

Pull current standoc and isodoc gems from github, those issues have likely been remedied already there.

@ribose-jeffreylau
Copy link
Author

Thanks @opoudjis . I found the culprit. The error is due to missing definition of a referenced term.

This line of error got lost in the sea of other logs: https://github.com/CalConnect/csd-transcription-systems/runs/3043780696?check_suite_focus=true#step:5:438

In this case, one of the ways to fix it is just changing terms:[languages] to terms:[language] but then it breaks the English grammar, since it should read "languages" in the context of the sentence.
Or, amend it to terms:[language]s but it'll be rendered as "language(3.2)s" which looks odd.

The standard practice (pun intended) in this case seems to be "languages(3.2)", but I'm not sure how it can be achieved with metanorma.

In general, I think metanorma-cli should bail as soon as it hits an error like the above, instead of continuing and returning 0. And then informative error messages can follow (might be a separate issue though).

cc @ronaldtse

@ribose-jeffreylau
Copy link
Author

Perhaps I've been doing it wrong? Maybe the solution is just

_languages_ (<<term-language>>)

Anyway, the suggestion that the CLI should fail fast still stands.

@ronaldtse
Copy link
Contributor

The new "concepts mention" functionality is now available:
https://www.metanorma.org/blog/2021-07-06/concepts-in-metanorma/

The actual failure is due to the plural form not being found by Metanorma.

In this case, we should use:

{{language,languages}}

@opoudjis
Copy link
Contributor

The macro is term:[], not terms[], as a synonym of {{ }}.

When you enter a term:[] reference to a missing string, the error message

AsciiDoc Input: (XML Line 000039): Error: Term reference in term[fred] missing: "fred" is not defined in document

is generated, and added to the error log *.err, which is output to disk; the document also contains, at the place where the incorrect anchor was located, the text:

term fred not resolved via ID fred

The error messages are, in fact, informative. While it is easy for me to abort on finding an unresolved anchor, I do not at all agree that execution should abort: being able to see a generated document containing clear error messages is going to be far more helpful in debugging, I have found, than going through an error log. Will not action without go-ahead from @ronaldtse.

@ribose-jeffreylau
Copy link
Author

The error messages are, in fact, informative. While it is easy for me to abort on finding an unresolved anchor, I do not at all agree that execution should abort: being able to see a generated document containing clear error messages is going to be far more helpful in debugging, I have found, than going through an error log.

Thanks @opoudjis , that's a fair point.

Instead of aborting, would returning a non-zero exit status work? That serves to signal that the build is not error-free, and will not erroneously let CI pipelines march ahead with errors still in the document.

@ronaldtse
Copy link
Contributor

ronaldtse commented Jul 20, 2021

I agree that a non-zero exit status should apply on any type of error, being consistent with metanorma/metanorma-cli#89, metanorma/metanorma-cli#151, metanorma/metanorma-cli#164 among others.

Perhaps there could also be a --strict (or a --loose) mode where --strict dies at the first error (or where --loose suppresses the non-zero status on minor errors).

@ribose-jeffreylau
Copy link
Author

Having --strict --loose sounds it might work.

@opoudjis
Copy link
Contributor

That's going to take some juggling of metanorma (and the individual gems within it), and rearchitecting. I'm making it medium priority, and I'd much rather @abunashir work on it...

@ronaldtse ronaldtse moved this to Triage in Metanorma Nov 14, 2021
@ronaldtse ronaldtse moved this from Triage to md in Metanorma Nov 14, 2021
@ronaldtse ronaldtse moved this from md to Med priority in Metanorma Nov 14, 2021
@opoudjis opoudjis changed the title Build error (missing .to_xml for nil:NilClass) Loose and strict validation Oct 26, 2022
@opoudjis opoudjis moved this from 🏕 Med priority to 🏔 High priority in Metanorma Jan 8, 2024
@opoudjis
Copy link
Contributor

opoudjis commented Jan 8, 2024

Error priorities have now been implemented, 0, 1, 2, with 0 currently fatal.

@opoudjis
Copy link
Contributor

This issue is essentially metanorma/metanorma-cli#301, which is implementing non-zero exit. I am closing this ticket in favour of that, but passing on the request for --strict and --loose to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

3 participants