-
Notifications
You must be signed in to change notification settings - Fork 98
fix: generate placeholder text for missing toTex implementations
#518
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
base: main
Are you sure you want to change the base?
Conversation
|
@david-christiansen With this change, the
|
david-christiansen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good start! Thanks!
But I think it's important that authors don't have to proofread their document to know if something went wrong. The best situation is that they get error messages and a non-zero return code, but also output that is as readable as possible.
| let some descr := (← readThe ExtensionImpls).getBlock? b.name | ||
| | panic! s!"Unknown block {b.name} while rendering.\n\nKnown blocks: {(← readThe ExtensionImpls).blockDescrs.toArray |>.map (·.fst) |>.qsort (·.toString < ·.toString)}" | ||
| let some impl := descr.toTeX | ||
| | TeX.logError s!"Block {b.name} doesn't support TeX" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this error message should be retained. It's important to prominently tell authors that their document isn't as they expect, without them needing to proofread the resulting text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
| let some descr := (← readThe ExtensionImpls).getInline? i.name | ||
| | panic! s!"Unknown inline {i.name} while rendering.\n\nKnown inlines: {(← readThe ExtensionImpls).inlineDescrs.toArray |>.map (·.fst) |>.qsort (·.toString < ·.toString)}" | ||
| let some impl := descr.toTeX | ||
| | TeX.logError s!"Inline {i.name} doesn't support TeX" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, I think this error message should be retained too. It's good to emit better TeX though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
src/verso/Verso/Doc/TeX.lean
Outdated
| def header [Monad m] (name : TeX) : TeXT g m TeX := do | ||
| let opts ← options | ||
| let some i := opts.headerLevel | ||
| | logError s!"No more header nesting available at {name.asString}"; return \TeX{\textbf{\Lean{name}}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also be a proper error, and it should insert a notice like the other two into the document that is unmistakable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
Thanks for the big list of things that need fixing! That'll be really helpful to check off as it goes. |
|
I'm away from the computer for a few days but will look again as soon as I'm back. Thanks again! |
Generate placeholder text for missing
toTeximplementations instead failing.