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

CSS tweaks, initial templates, link fixer script, tree struct preparation #5

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

ElementW
Copy link
Contributor

No description provided.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you take this from? Is there a license for this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of Google's Material icons, which hugo-book also uses for the edit icon (pencil). I don't know if hugo-book has attribution, but the license is Apache-2.0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, upstream documentation says this about attribution:

Feel free to remix and re-share these icons and documentation in your products. We'd love attribution in your app's about screen, but it's not required.

Seems fine as is then, since it's a minor use.

Comment on lines 21 to 26
.markdown {
table tr {
&:nth-child(2n) {
background: var(--table-2n-background);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have mixed feelings about this. Alternating row colors are really useful on large tables (such as the title list). However for smaller tables and for sequential structures like IPC command tables, non-alternating colors are the right choice indeed. Is there a way we could opt into alternating row colors for individual tables that benefit from it (without forcing those tables to be HTML)?

Doesn't need to happen now, just curious if we could support both.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, just to double-check: Was the intent here indeed to disable alternating row colors, or is this an unintended effect for the light theme?

Copy link
Contributor Author

@ElementW ElementW Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an unintended effect, the default book theme has alternating colors, but as usual with people that have very little design and color theory, both are offset a fixed number of RGB units from the background color, which results in a very different effect between light and dark themes because of the human eyes' nonlinear sensitivity.

This is just here to fix that, but the point about being opt-in is a good one, although I have no idea on how to make this fit in nicely with Markdown, is there a way to specify table classes in this dialect?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a way to specify table classes in this dialect?

Doesn't seem like it, but goldmark has no problem with wrapping the entire markdown table in a <div>. So we can just do that and then define a CSS rule for .withalternatingrows > table > ... :)

Comment on lines +4 to +5
BookViewPath = 'blob/main'
BookViewSuffix = '?plain=1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do these do? I couldn't find any reference to these in themes/hugo-book.

Copy link
Contributor Author

@ElementW ElementW Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used in layouts/partials/docs/footer.html for the custom "View in git" links.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, of course.

@neobrain
Copy link
Member

Thanks! I merged the styling-related patches with minor tweaks into main. The rest fall in three categories:

  • Link-fixing script and the actual fixes for the links
  • Navigation sidebar. This doesn't seem to be very functional without moving the files into corresponding folders, so let's move this to a dedicated PR.
  • Preparation for IPC/... templates. This seems unrelated to the other changes, can we get a dedicated PR for these along with a few example uses?

The first one can probably already be merged, haven't quite got around to look into it more closely yet though.

@neobrain
Copy link
Member

neobrain commented Sep 30, 2024

Link-fixing script and the actual fixes for the links

The script is working great in isolation (combined with f724a9e). If we lowercase the page names when indexing into pages, we can automatically have it fix filenames as well:
pages[name] = path => pages[name.lower()] = path
target_page = pages[target.split("/")[-1]] => target_page = pages[target.split("/")[-1].lower()]

Don't HTML links need to be updated as well? Would be nice to have a second regex in the script to match those as well.

Also, render-link.html now seems to warn about pages that link via ".":

WARN Link to 'Home_Menu' has unmatching case ('.') in 'Themes'

@@ -0,0 +1,375 @@
# Created by https://www.toptal.com/developers/gitignore/api/hugo,python,pycharm+all,visualstudiocode,kate,emacs,vim
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Half of this doesn't really apply to this repo. I'd rather have a cherry-picked set of ignores than ignoring every eventuality "just because".

For the time being I added Hugo's outputs to .gitignore. Personal configuration can also be written to a global ignores file, which git typically reads from .config/git/ignore (see also core.excludesFile).

@neobrain
Copy link
Member

neobrain commented Oct 1, 2024

Don't HTML links need to be updated as well? Would be nice to have a second regex in the script to match those as well.

Discard this point. I realized I already pushed the {{ href }} shortcode, which errors on links that aren't marked as broken. We should ensure both <a {{% href=... %}}> and [](...) expect consistent paths though.

Also, adding [Test](GPU/Internal_Registers "wikilink") to content/Title_list/DLC.md doesn't print a warning despite not using "../". Is this intended? It's surprising that either GPU/Internal_Registers and ../GPU/Internal_Registers work.

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