Skip to content

Tutorial review learning #33

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

Merged
merged 12 commits into from
Aug 7, 2024

Conversation

moschmdt
Copy link
Contributor

@moschmdt moschmdt commented Aug 1, 2024

Reviewed the tutorials 5, 6, 7 and 8 - they should be in line with the word/pdf version.

I removed the WIP warning for those tutorials. Should we link to the old PDFs nonetheless?

The manual chapter has just some linting adjustments.

@garfieldnate
Copy link
Collaborator

Thank you for working on this!

  • It looks like 5 still has a bunch of linting warnings that failed the build. Not sure if those prevent the site from building after merging.
  • Yes, I would like to provide the PDF versions as a courtesy to those that want them. Longer-term, I'd like to generate the PDFs from this markdown source so that we don't have two versions floating around. The tutorials aren't modified much, so it's not as big an issue as it is with the manual.

@moschmdt moschmdt force-pushed the tutorial_review_learning branch from 4e523d3 to 8bb051f Compare August 2, 2024 10:34
@moschmdt
Copy link
Contributor Author

moschmdt commented Aug 2, 2024

I just added the admonitions plugin with some suggested default configuration to get rid of the HTML footnotes in the Markdown.

This also adds the missing figure "6.1"
@moschmdt
Copy link
Contributor Author

moschmdt commented Aug 2, 2024

I just downloaded an example page as PDF via the print dialogue of the browser and it actually looks very reasonable. The links are also working "out-of-the-box".
Semantic Memory - Soar Home.pdf

Maybe a hint/ info point referencing the possibility would already be sufficient?

@garfieldnate
Copy link
Collaborator

That PDF does look great! I didn't realize mkdocs had such good print styling, but I suppose it makes sense.

The PDFs are distributed with Soar, so I would want to figure out a way to generate them automatically. Headless chrome or selenium or something. I tried wkhtmltopdf, but it ignored all styling and the output looked awful, so that possibility is eliminated. I think this is a good path, though.

I like the look of the admonitions, as well.

Part of the reason I marked all of these pages as "under construction" was because the images are not usable when the site is in dark mode. They either need a light background or to be reworked to update automatically for dark mode (the latter is a bit of a production).

@moschmdt
Copy link
Contributor Author

moschmdt commented Aug 5, 2024

Ah, that is unfortunate that the headless mode does not work.

For now, all modified files in this PR have valid images with either a white background or dynamic color switching. Should I revert the warning removal nonetheless or add an admonition providing a hint for the PDF printing possibility?

@garfieldnate
Copy link
Collaborator

rl-optrace.svg appears to be an invalid SVG (at least according to GitHub).

Yes, I would leave the admonition in for now; you can change it to be more specific about dark mode being an issue, if you like.

@moschmdt
Copy link
Contributor Author

moschmdt commented Aug 6, 2024

Do you know if there is a new command for watch --chunks --print? (Tutorial V: Planning and learning) I get the following error message in the debugger:

no such option: print

@garfieldnate
Copy link
Collaborator

When I put in watch --chunks I get Now printing when chunks fire., so maybe you just need to remove the --print.

GitHub still says that SVG file is malformed.

@moschmdt
Copy link
Contributor Author

moschmdt commented Aug 6, 2024

Interesting... Do you see some kind of error message since I don't get any warning? I just checked the local version in both light and dark mode. It appears to be fine for mkdocs.

@garfieldnate
Copy link
Collaborator

Actually I think the image is fine :D It shows an error in the commits view, but shows properly in the "Files Changed" view.

Is this ready merge?

@moschmdt
Copy link
Contributor Author

moschmdt commented Aug 7, 2024

Is this ready merge?

Yes!

@garfieldnate
Copy link
Collaborator

Great, thanks for the all work!

@garfieldnate garfieldnate merged commit 4cc1afc into SoarGroup:main Aug 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants