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

update spec to use latest docs-spec-template features #342

Conversation

kbroch-rivosinc
Copy link
Contributor

no spec content changes, just long list of workflow/infra change:

  • update Makefile (src/build dir; html output;)
  • update docs-resources to lastest commit
  • adoc attributes
    • include global attributes from docs-resources
    • fix others to reflect src/build
  • move spec files to src dir
  • update github actions to latest
  • install pre-commit (config file and github action)
  • update README.adoc
    • add MAINTAINERS, CONTRIBUTING, CODE_OF_CONDUCT files

relates to #340

no spec content changes, just long list of workflow/infra change:

* update Makefile (src/build dir; html output;)
* update docs-resources to lastest commit
* adoc attributes
  * include global attributes from docs-resources
  * fix others to reflect src/build
* move spec files to src dir
* update github actions to latest
* install pre-commit (config file and github action)
* update README.adoc
  * add MAINTAINERS, CONTRIBUTING, CODE_OF_CONDUCT files

relates to riscv-non-isa#340

Signed-off-by: Kevin Broch <[email protected]>
@ved-rivos
Copy link
Collaborator

ved-rivos commented Jun 3, 2024

@kbroch-rivosinc Thanks much!
Two questions related to the headers: a) the ISA manual renders without colors b) the text in braces seems (fctl) seems to get rendered with a much smaller font (and without color; so maybe we dont need color - like the ISA manual). Is this fixable.
Headers with this PR:
image
Headers in ISA manual:
image

kbroch-rivosinc added a commit to riscv/docs-resources that referenced this pull request Jun 3, 2024
@kbroch-rivosinc
Copy link
Contributor Author

kbroch-rivosinc commented Jun 3, 2024

@ved-rivos : the ISA manual uses a different theme then what is in the docs-resources repo. I'm not sure what the long term plan is for the ISA manual theme but for now I've copied the ISA theme to docs-resources riscv/docs-resources#16 and will modify the Makefile to use that one instead since it would make sense to me that it looks the same as the spec it will end up in.

new version looks like:
image

/cc @wmat

@ved-rivos
Copy link
Collaborator

@kbroch-rivosinc @wmat Thanks much! The headers look nice now. But this theme seems to generate tiny font for text in tables. Perhaps the ISA manual does not have as many tables but large parts of this spec is all in tables ! So now debating whether I should just live with your older theme and fix the header issue by removing the backticks on the text in parenthesis in the headers...

image

@kbroch-rivosinc
Copy link
Contributor Author

I wouldn't modify the content, if you want monospace for those commands in the headings (section titles).

The diff for tables is:

  • riscv-spec.yaml theme has table:font-size: 9
  • riscv-spec.yaml theme has table:font-size: 11.5

I can just change the fontsize to 11.5

@ved-rivos ved-rivos requested a review from pperesse June 4, 2024 11:38
@ved-rivos
Copy link
Collaborator

I can just change the fontsize to 11.5

Thanks! That looks good.

@ved-rivos
Copy link
Collaborator

@kbroch-rivosinc

  1. Is there a way to fix how & gets rendered so it looks like a "&"
    image
    Like this:
    image

@wmat
Copy link
Contributor

wmat commented Jun 5, 2024 via email

@kbroch-rivosinc
Copy link
Contributor Author

Try pointing docs-resources at the most recent commit. I had to add a font for sans-serif glyphs.

Unfortunately that didn't change anything. Ved's example is using asciidoc monospace which in the pdf style yaml file maps to https://sk.fonts2u.com/m-1mn-medium.font which is where that funky ampersand is coming from.

@ved-rivos : where did the MCG example come from? I'm guessing that it is a formula which is why it is rendering with a different font (non-monospaced) which allows for a nice looking ampersand.

@ved-rivos
Copy link
Collaborator

where did the MCG example come from?

I grabbed that from the PCIe spec PDF as an example of how I wish it rendered.

@ved-rivos
Copy link
Collaborator

Ved's example is using asciidoc monospace which in the pdf style yaml file maps to https://sk.fonts2u.com/m-1mn-medium.font which is where that funky ampersand is coming from.

This is the source for that:

(A >> 12) & ~msi_addr_mask = (msi_addr_pattern & ~msi_addr_mask)

@ved-rivos
Copy link
Collaborator

If I use latexmath formula (from the CBQRI spec) it renders better. I can do that to update these formulas to latexmath
image

(source: latexmath:[Effective-MCID = (RCID \ll P) \mid (MCID , \text{&} , ((1 \ll P) - 1))] )

@kbroch-rivosinc
Copy link
Contributor Author

@wmat : any reason we can't use a open source (apache) font like Roboto Mono
as the codespan font?

@wmat
Copy link
Contributor

wmat commented Jun 5, 2024

No reason we can't. It'll require installation into docs-resources as well as to any repos not relying on docs-resources, such as riscv-isa-manual.

@kbroch-rivosinc
Copy link
Contributor Author

No reason we can't. It'll require installation into docs-resources as well as to any repos not relying on docs-resources, such as riscv-isa-manual.

Ok, I'll put that PR in but first test it out on iommu.

Is there any reason why riscv-isa-manual isn't using docs-resources? If not I can put a PR in to have it start using it.

I'd also like to resolve the minor differences between the two pdf styles yamls so that all specs use the same instead of different if they are in their separate repo vs. the isa-manual.

@wmat
Copy link
Contributor

wmat commented Jun 5, 2024

I asked Andrew and he said to go ahead and templatize riscv-isa-manual so long as it doesn't break anything. I may have an issue out there for this already in docs-sig.

@ved-rivos
Copy link
Collaborator

any reason we can't use a open source (apache) font like Roboto Mono

Jetbrains Mono seems like a popular font for code - better ligatures support and disambiguation between things like O and 0.

@ved-rivos
Copy link
Collaborator

@kbroch-rivosinc another nit is the bullets - they are rendered as somewhat tiny.
image

A bit larger bullets would make it look better:
image
Not sure if this is a easy fix.

@ved-rivos
Copy link
Collaborator

And one last nit is on list of figures and tables. They show up a bit wavy when going from one digit to two.
image
and not aligned like below:
image

Also not sure if this would be an easy fix.

@ved-rivos
Copy link
Collaborator

And very last nit. In previous template the TOC used to have these nice ... between text and page number:
image

With new theme those dots are gone and is now harder to eyeball section to page number
image

Also also the page numbers in new theme seem to have variable width causing that column of numbers to look visually wavy

@ved-rivos
Copy link
Collaborator

ved-rivos commented Jun 6, 2024

The icon for the informative note used to have a nice circled i before and now its rendered as a note. The circled i was more pleasant. This is subjective. So please decide what looks good to you :). I like the first one.
Before:
image

New template:
image

@ved-rivos
Copy link
Collaborator

One nit is about how the bibliography entries get rendered. When they wrap the, second line starts from under the reference number.
image

Instead of being aligned like following:
image

It looks much nicer aligned.

@wmat
Copy link
Contributor

wmat commented Jun 6, 2024

I believe I can address the majority of your concerns in the theme. However, some of these things were expressly requested to be removed, i.e. the toc-dot-leader

I can turn it back on if you think it's better though.

@ved-rivos
Copy link
Collaborator

I believe I can address the majority of your concerns in the theme. However, some of these things were expressly requested to be removed, i.e. the toc-dot-leader

Thanks! Specifically the dot-leader if that was a conscious decision then I dont feel strongly about reverting it. If the following could be fixed that would be great:

  • Jetbrains mono font for codespan
  • Waviness of the page number column in TOC
  • Waviness and alignment of the list of figures and tables
  • Slightly larger bullets and sub-bullets
  • note icon
  • alignment/justification of the bibligraphy entries.

@kbroch-rivosinc
Copy link
Contributor Author

@ved-rivos : IMO this pdf style discussion has grown outside the scope of this PR. I'd like to take a more systematic review of the pdf theme and have filed this issue to do so: riscv-admin/docs-sig#50 I've added this PR to the issue to make sure this discussion is considered

In the meantime, I'd like to get riscv-admin/docs-sig#48 done so this and all specs can incorporate the unified (isa-manual) style.

@ved-rivos
Copy link
Collaborator

Okay. Looks like we will miss the getting these fixes for the planned publication of the PDF for 6/15.

@kbroch-rivosinc
Copy link
Contributor Author

With this commit 5d7181e I'm done with what I was trying to accomplish with this PR.

@ved-rivos
Copy link
Collaborator

Thanks. Would asciidoc allow overriding the theme through the header so I can fix the other things like the bullet size and such locally?

@wmat
Copy link
Contributor

wmat commented Jun 7, 2024 via email

@ved-rivos
Copy link
Collaborator

I will address those things tomorrow, Ved.

Thanks much. I only have a goal to publish an updated revision by 6/15. I will wait for your update!

@wmat
Copy link
Contributor

wmat commented Jun 7, 2024

Hi @ved-rivos , I moved the docs-resources to the latest and it addresses most of your concerns I think. The dot-leader is back in the TOC and the note admonition is the old circled i again. I don't see a List of Figures in the spec yet. Also, I'm still trying to figure out the bibliography alignment.

@ved-rivos
Copy link
Collaborator

@wmat Thanks a bunch! I will apply this PR then.

@ved-rivos ved-rivos merged commit e4abe4f into riscv-non-isa:main Jun 7, 2024
3 checks passed
@wmat
Copy link
Contributor

wmat commented Jun 7, 2024

Sorry, I pushed to main w/o a PR.

@ved-rivos
Copy link
Collaborator

@wmat
Please see the list of figures and tables in https://github.com/riscv-non-isa/riscv-iommu/actions/runs/9421593666

I see the following issues still with the new theme:

  1. Bullets are tiny
  2. The note admonition did not change to circles
  3. The mono text is not updated - still see the oddly rendered "&" and "~"
  4. No dot leader in TOC
    So looks like the build is not picking up your latest theme?

@wmat
Copy link
Contributor

wmat commented Jun 7, 2024

Shoot, you're right. OK, now that there is only 1 theme is docs-resources I'll update it with your recommendations.

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.

3 participants