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

Makefile changes to include Highlighter Sail #1608

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlfredoRodrigues4
Copy link

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 riscv/docs-resources#33 and it will also be necessary to include these files in the "REQUIRES" section of the Makefile.

@wmat
Copy link
Collaborator

wmat commented Aug 22, 2024

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
Copy link
Author

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
Copy link
Collaborator

wmat commented Aug 22, 2024

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
Copy link
Author

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
Copy link
Collaborator

wmat commented Aug 22, 2024

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
Copy link
Collaborator

wmat commented Aug 22, 2024

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
Copy link
Author

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

@abrodkin
Copy link

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.

Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

If this change moves forward, please replace the tabs with spaces to match the surrounding context, and add a newline at the end of the file.

@AlfredoRodrigues4 AlfredoRodrigues4 force-pushed the highlightingSail branch 2 times, most recently from 848120d to 89e755b Compare August 27, 2024 10:57
@AlfredoRodrigues4
Copy link
Author

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!

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.

4 participants