-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Adding new features to the Typst template #9970
base: main
Are you sure you want to change the base?
Conversation
Hi @jgm, I don't want to bother you if this is already on your radar, but should I add you as a reviewer (for when you have time)? I'm not 100% sure of the norms for contributions here. |
Sorry for the delay on this! |
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.
Thanks for this PR, and sorry I took so long to review it. I made a number of queries and comments, which you can look over.
$if(mathfont)$ | ||
mathfont: ($for(mathfont)$"$mathfont$",$endfor$), | ||
$endif$ | ||
$if(codefont)$ | ||
codefont: ($for(codefont)$"$codefont$",$endfor$), | ||
$endif$ |
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.
Curious why you used $for$
here. mathfont
would normally be expected to be a string in pandoc templates.
$if(citecolor)$ | ||
citecolor: [$citecolor$], | ||
$endif$ |
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.
It works in content like this, with the escaped #
?
citecolor: [\#ffffff]
You've tested I assume?
mathfont: "New Computer Modern Math", | ||
codefont: "DejaVu Sans Mono", |
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.
Are these different from the fonts Typst uses by default? Would it not be better to allow these to be specified but stay with Typst's normal defaults by default?
linkcolor: ["#800000"], | ||
citecolor: ["#0000FF"], | ||
toccolor: ["#800000"], |
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.
Again, is this the same as Typst's default behavior? If not, why these colors? Would it perhaps be better to make Typst use its standard defaults unless these are specified?
author: authors.map(author => content-to-string(author.name)), | ||
author: authors.map(author => content-to-string(author.name)).join(", ", last: ", and "), |
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 don't like this because and
is an English word; anyone writing in another language would need to change the template. We could consider using &
, or just use commas.
leading: linestretch * 0.65em | ||
) |
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.
What does the leading
do here, and why is it needed with linestretch? Is the result the same as Typst's defaults when the default linestretch is used?
#text(weight: "bold", size: 1.5em)[#title] | ||
#text(weight: "bold", size: 1.5em)[#title #if thanks != none { | ||
footnote(thanks, numbering: "*") | ||
counter(footnote).update(n => n - 1) |
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 assume this is needed because otherwise the first note would be 2
? That's a bit unexpected when numbering
is explicitly specified, but maybe Typst works that way? (I'd consider it a bug if so.)
This implements the changes suggested in #9956, with the exception of the
filecolor
/urlcolor
one. These would require adding some regex to guess the link types. This is theoretically possible to do, but it wasn't clear to me that this is a good thing to put in a default template. Happy to adjust if you have thoughts on this.Some things to note:
#
if that was included. An alternative here is to just do something like:rgb(linkcolor.replace("\\#", "#"))
.