Typst training slides conversion#323
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA. |
alexandrebelloni
left a comment
There was a problem hiding this comment.
I have a very small patch to fix some issues that probably don't make sense to fix in the automated conversion, I'll apply after this is merged.
|
As discussed with other trainers, the new slides do not have the same size as the former one (on my machine, for debugging training, 12.2MB after, 8.9MB before). As it is suspected to be a reason for the lags observed in pdf viewers when displaying the new materials, I've taken a deeper look into it:
diff --git a/typst/local/bootlin/0.1.0/lib.typ b/typst/local/bootlin/0.1.0/lib.typ
index 3d7dc5b79a5f..7962cbf20b1c 100644
--- a/typst/local/bootlin/0.1.0/lib.typ
+++ b/typst/local/bootlin/0.1.0/lib.typ
@@ -515,10 +515,11 @@
set list(spacing: 0.6em)
show: touying-slides.with(
config-page(
- paper: "presentation-" + aspect-ratio,
+ // paper: "presentation-" + aspect-ratio,
margin: (x: 2em, top: 20mm, bottom: 6mm),
footer-descent: 0.5mm,
header-ascent: 1mm,
+ ..(width: 6.30in, height: 3.54in)
),
config-common(
slide-fn: slide,Unfortunately:
So IMHO it is preferable to keep this new size |
Tropicao
left a comment
There was a problem hiding this comment.
For the record, I did a quick local test at automating formatting and checked the impact on the branch (checked only for debugging training):
make full-debugging-slides.pdf && cp full-debugging-slides.pdf full-debugging-slides-pre.pdf
find . -name "*typ" -exec /usr/bin/typstyle -i {} \;
make full-debugging-slides.pdf
diff-pdf full-debugging-slides-pre.pdf full-debugging-slides.pdf -s -m --output-diff=diff.pdfAnd saw that only a single slide was affected by the reformatting, and the diff looked ok (slight size variation). It could be worth checking the impact for other trainings the impact, and if worth, running this formatting for the whole slides (that would allow to get rid of all the weird indentations, the blanks lines, etc). I'll try to add this as well in the github actions in the next steps.
None of my comments are blocking, every raised point can be handled post-merge. Thanks for your work
| @@ -1,5 +1,5 @@ | |||
| out/ | |||
There was a problem hiding this comment.
The intent behind this change is not really clear from the commit message, but I suspect that it does nothing.
There was a problem hiding this comment.
Before it meant "ignore PDFs in the root directory", now it means "ignore all PDFs". Are there any PDFs we don't want to ignore?
If there is a separator at the beginning or middle (or both) of the pattern, then the pattern is relative to the directory level of the particular .gitignore file itself. Otherwise the pattern may also match at any level below the .gitignore level.
— https://git-scm.com/docs/gitignore
There was a problem hiding this comment.
Ah, then yes, I see plenty of images stored as pdf in the repository, so this change may be undesired
| (cd $(OUTDIR); $(PDFTYPST) $(PDFTYPST_OPT) $(TRAINING_OPT)=$(SLIDES_TRAINING) $(TRAINER_OPT)$(TRAINER) $(SESSION_URL_OPT)$(SESSION_URL) $(basename $@).typ) | ||
| # Use cat instead of mv so that evince detects the change and reloads | ||
| # ('Maxime's feature'). | ||
| cat out/$@ > $@ |
There was a problem hiding this comment.
I know that it is not really related to your PR, but I'm a bit confused by the manipulation here:
- is it really for evince ? I guess that's rather for the makefile to detect changes, right?
- if so, that's rather that we have a broken rule, because
makeis supposed to detect change (but I can understand that we may break this with our way to deal with chapters). But wouldn't a baretouchwork then ? - could we remove 'Maxime's feature" ? It does not bring any info
That's ok to integrate it though, but I'm at least writing it down as a todo for the next improvements
There was a problem hiding this comment.
IIRC, it is not Makefile related, this is really related to the reader that might not reload the file after mv (?). To be checked again, because things may have changed since this quirk was first introduced.
There was a problem hiding this comment.
I know that it is not really related to your PR, but I'm a bit confused by the manipulation here:
* is it really for evince ? I guess that's rather for the _makefile_ to detect changes, right? * if so, that's rather that we have a broken rule, because `make` is supposed to detect change (but I can understand that we may break this with our way to deal with chapters). But wouldn't a bare `touch` work then ? * could we remove 'Maxime's feature" ? It does not bring any infoThat's ok to integrate it though, but I'm at least writing it down as a todo for the next improvements
This is required, using cat updates an existing file vs mv that creates a new file so if you work on slides with the pdf opened by evince/papers, updating the file works while creating a new file doesn't, probably because the fd still points to the old file that has been removed.
There was a problem hiding this comment.
This trick can be dropped now (sorry Maxime's feature, we'll remember you). On my system Papers (replacement of evince) reloads if I open out/full-linux-kernel-slides.pdf then update it with make full-linux-kernel-slides.pdf.
The fact that evince didn't reload previously was an issue with how the file was generated by LaTex.
There was a problem hiding this comment.
This is required, using cat updates an existing file vs mv that creates a new file so if you work on slides with the pdf opened by evince/papers, updating the file works while creating a new file doesn't, probably because the fd still points to the old file that has been removed.
Ok, thanks for the clarification
This trick can be dropped now (sorry Maxime's feature, we'll remember you). On my system Papers (replacement of evince) reloads if I open out/full-linux-kernel-slides.pdf then update it with make full-linux-kernel-slides.pdf.
TIL about Papers
There was a problem hiding this comment.
@tleb have you tried with the top level full-linux-kernel-slides.pdf instead?
There was a problem hiding this comment.
I just tested with papers and evince: no problem, pdf readers get the update automatically, even with a mv.
This hack can be dropped.
| @@ -0,0 +1,374 @@ | |||
| #import "@local/bootlin:0.1.0": * | |||
There was a problem hiding this comment.
As discussed already, the new slides look good overal (focusing on the debugging training on my side)l. Just a few nits here and there that I'm repeating here to keep those saved as a todo list:
- a few kernel links are dead (eg 51, 67). The code is making those point on the
latestversion, which may be not desirable, as it can make the links silently rot - there are quite a lot of spaces added for items between parenthesis (43, 54, 67, 75, 79, 84, 85)
- a raw block is broken (62)
- many raw blocks are using
consolefor the type, but it does not trigger any syntax color. It should rather besh(172, 175, 185..) - the text in raw code block seems to be very close to the raw block borders. It may be worth adjusting
inset - a few spacing oddities around raw inline (286, 287)
| #let last_update = datetime.today() | ||
|
|
||
| // ── Couleurs ─────────────────────────────────────────────────────── | ||
| #let bootlin-orange = rgb("#FF631A") |
There was a problem hiding this comment.
We want bold that is bold not orange. :-) Maybe making it available under a function like #boolin()?
| line(length: 100%, stroke: 0.2pt + black) | ||
| v(-0.9em) | ||
| h(0.5em) + utils.call-or-display(self, self.store.footer) + h(1fr) + utils.call-or-display(self, self.store.footer-right) + h(0.5em) | ||
| } |
| ), | ||
| config-methods( | ||
| init: (self: none, body) => { | ||
| set text(font: "Latin Modern Sans", size: 20pt) |
| ..args, | ||
| ) | ||
| body | ||
| } |
There was a problem hiding this comment.
No files have newlines at the end. This is uncommon in our world (why I'm not sure). This means my editor automatically adds one and that makes a diff. Notice how GH shows it with a stop-sign symbol.
|
In the kernel training I observe that most pictures look bigger than before which makes the reading uncomfortable. Perhaps the font is also a bit larger than before, which overflows some lines. |
(I can't target a specific message to reply to as it's not a review comment.) Can you point at examples? Only one I could find is p226 which is an issue with the slide's |
Indeed, it is image dependent, I found one or two slightly larger but it is actually ok, please discard this comment, we will figure it out manually later, if we even need to eventually act. |
+ Deleted networking .tex counterparts Buildable. At this stage, the buildable full training slides are the following: - Audio - Buildroot - Debugging - Embedded-linux - Linux-kernel - Networking Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
+ Deleted preempt-rt .tex counterparts Buildable. At this stage, the buildable full training slides are the following: - Audio - Buildroot - Debugging - Embedded-linux - Linux-kernel - Networking - Preempt-rt Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
+ Deleted yocto .tex counterparts Buildable. At this stage, the buildable full training slides are the follwoing: - Audio - Buildroot - Debugging - Embedded-linux - Linux-kernel - Networking - Preempt-rt - Yocto Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
- Cleaned-up the main bootlin package - Centralized the functions scattered across the different packages (Yocto, utils) - Deleted utils and yocto packages folders (unused) - Fixed the different dead links in the common file and the spacing around each function Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
- The previous logic was quite flawed, as it required having every single .typ file listed in the .mk file. Which led to some issues, notably regarding the Security training, which had the case of multiple .typ files in the same directory (some which were included and not listed in the .mk file). So, the updated logic copies every .typ file found in the folders (except the first-slide folder, which contains the trainer slides). Also, to detect whether the training should be build in Typst or LaTeX, it now verifies whether a chapter of a dedicated training has been converted (example: security-hw-devices). By doing so, it easily filtrates the yet-to-be-converted slides and the updated ones. - I also added another typst option suggested by Théo Lebrun, which is " --font-path=typst/fonts " , this prevent issues regarding the font. Which varied across different computers. Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
The .mk file had duplicated lines, I deleted them Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Fixed *.pdf -> /*.pdf Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Resolved convoluted logic. Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
- Modified preset spaces in-between list items in bootlin-theme - Updated logo-square-full.svg (Previously Very high resolution image) - Shrunk text size for common function Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
- Updated list spacing, text size and footnote size Signed-off-by: Ali Adam Labadi <aliadam.labadi@bootlin.com>
Add the typst binary to the container to allow building slide decks that are converted from LaTeX to typst. Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>


Converted the slides of 8 trainings: Audio, Buildroot, Debugging, Embedded-linux, Kernel, Networking, Preempt-RT and Yocto.
As well as common shared files and bootlin packages.