riscv / riscv-isa-manual

RISC-V Instruction Set Manual
https://riscv.org/
Creative Commons Attribution 4.0 International
3.7k stars 643 forks source link

Makefile changes to include Highlighter Sail #1608

Open AlfredoRodrigues4 opened 2 months ago

AlfredoRodrigues4 commented 2 months ago

The Sail code in the RISC-V manuals suffers from poor readability and alignment issues. I discovered a repository (https://github.com/Alasdair/asciidoctor-sail) that offers a solution to enhance the presentation of Sail code by aligning it and highlighting keywords with colors. The benefits of this improvement will become increasingly apparent as new instructions are added to the manual, such as those proposed in Pull Request #1603.

image image

As demonstrated in the images above, the difference is substantial and represents a clear improvement to the manual. To implement these enhancements, some new files will need to be included in the doc's resources folder, as pointed out in the Pull Request https://github.com/riscv/docs-resources/pull/33 and it will also be necessary to include these files in the "REQUIRES" section of the Makefile.

wmat commented 2 months ago

Alasdair's solution is the preferred method for including SAIL code in asciidoc. We are working on documenting the way to do this so that any extension author can include SAIL code. In the PDF for the extension it talks about the ways to reference the SAIL index. I'd like to see it referenced remotely via a hash amd using include macros to pull in the code vs. adding SAIL code manually.

AlfredoRodrigues4 commented 2 months ago

Are you talking about this PDF “https://github.com/riscv/riscv-isa-manual/blob/main/src/extending.adoc”? Because I saw that you had already had contact with Alasdair's solution, and I found it strange that you weren't using it yet.

wmat commented 2 months ago

This PDF: https://github.com/Alasdair/asciidoctor-sail/blob/master/doc/built/sail_to_asciidoc.pdf And I'm working on using it but have been busy with other priorities.

AlfredoRodrigues4 commented 2 months ago

Thank you, I understand now and by chance I have already had contact with that PDF, I think this question is closed but I am already familiar with it, is there anything that can help you?

wmat commented 2 months ago

What I'd like to see happen is for the :sail-doc: variable to point at a remote version of the SAIL model, perhaps using a hash. I haven't yet experimented with this yet though.

wmat commented 2 months ago

All you need to do to use Alasdair's asciidoc extension is install it and add it to your toolchain with --require=asciidoctor-sail

AlfredoRodrigues4 commented 2 months ago

for sure, now I already understand it, but think that could more explicit, thanks!

abrodkin commented 2 months ago

Alasdair's solution is the preferred method for including SAIL code in asciidoc. We are working on documenting the way to do this so that any extension author can include SAIL code. In the PDF for the extension it talks about the ways to reference the SAIL index. I'd like to see it referenced remotely via a hash and using include macros to pull in the code vs. adding SAIL code manually.

Oh, that's nice. But we were not aware of that ongoing development. It was not clear there's work being done on "proper" inclusion of SAIL code for each instruction (is there a GitHub issue for tracking that?) as well as it's not really obvious that https://github.com/Alasdair/asciidoctor-sail is somehow related to the RISC-V ISA manual (we just googled it looking for SAIL highlighting).

That said, there seems to be dome disconnect and it would be good solve it, at least with documenting ongoing activities, their status and also make references between projects where applicable - that will allow us to make sure there's no duplication of development is happening.

Also, it's interesting but looks like even Documentation SIG is not aware of that development related to "proper" SAIL snippets inclusion, at least that's a feeling I have.

Adding @dhower-qc who works on https://github.com/riscv-software-src/riscv-unified-db and clearly is interested in getting SAIL snippets added in the .yaml description of instructions.

But thanks for the hint on how to enable highliting with existing documentation.

AlfredoRodrigues4 commented 2 months ago

I've already updated the Makefile, the goal here was to try to directly add the highlighted sail to the manual by making a simple change, instead of installing it manually, while @wmat solution is not finished this could be a good solution to improve the layout in the meantime!