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

Render Markdown in option docs for flakes and longDescription #539

Merged
merged 6 commits into from
Oct 28, 2022

Conversation

ncfavier
Copy link
Member

@ncfavier ncfavier commented Sep 9, 2022

Closes #535, closes #525

To do:

  • We're currently creating a new temporary file containing the xref lua filter for each Pandoc invocation. This is bad. I think it would generally be nicer to package the files we need (pandoc filters, flake_info.nix) alongside flake-info: we could set a DATADIR variable in the Nix build that is detected at compile time and, if unset, defaults to CARGO_MANIFEST_DIR so that cargo run still finds the files.
  • The pandoc crate does not let us define precisely the flavour of Markdown we want (Commonmark with a few extensions) because it does not expose all of Pandoc's formats and extensions (notably commonmark_x and +fenced_divs). For now I've set the format to markdown, which is hopefully a superset, but if finer control is needed we will have to invoke pandoc directly or fork the crate. At least manpage references seem to render fine. Ping @jtojnar @pennae
  • There seems to be a regression with nix-security. EDIT: this is a bug exposed by making FlakeEntry tagged: we don't support multiple licenses in flake packages, so previously flake packages with a list of licenses were parsed as apps instead.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

@github-actions github-actions bot temporarily deployed to pull request September 9, 2022 21:57 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 9, 2022 22:01 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 9, 2022 22:05 Inactive
Comment on lines 63 to 64
let roles_filter = Path::new(FILTERS_PATH).join("myst-reader/roles.lua");
let man_filter = Path::new(FILTERS_PATH).join("link-unix-man-references.lua");
Copy link

Choose a reason for hiding this comment

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

docbook-writer/rst-roles.lua defines additional roles for MD option docs that may be useful here (though probably not vital).

Copy link
Member Author

Choose a reason for hiding this comment

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

But we're writing HTML, not DocBook.

Copy link

@pennae pennae Sep 9, 2022

Choose a reason for hiding this comment

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

yes, but if you previously rendered eg envar and literal differently you'll probably have to handle those roles to not render them all the same way. (haven't checked, just guessing that this may happen)

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, we didn't do anything like that.

@github-actions github-actions bot temporarily deployed to pull request September 9, 2022 22:58 Inactive
@ncfavier ncfavier marked this pull request as ready for review September 10, 2022 14:20
@github-actions github-actions bot temporarily deployed to pull request September 10, 2022 14:21 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 10, 2022 14:25 Inactive
@ncfavier
Copy link
Member Author

Ah, markdown requires a blank line before lists whereas commonmark doesn't. That settles it, let's switch to commonmark.

@github-actions github-actions bot temporarily deployed to pull request September 10, 2022 16:56 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 10, 2022 17:01 Inactive
@ncfavier ncfavier requested review from ysndr and garbas September 10, 2022 17:15
@ncfavier ncfavier changed the title Render Markdown option docs in flakes Render Markdown option docs for flakes Sep 10, 2022
@github-actions github-actions bot temporarily deployed to pull request September 12, 2022 10:16 Inactive
Extension list taken from nixpkgs/doc/Makefile.
@github-actions github-actions bot temporarily deployed to pull request September 13, 2022 11:27 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 19, 2022 16:34 Inactive
@ncfavier ncfavier changed the title Render Markdown option docs for flakes Render Markdown in option docs for flakes and longDescription Sep 19, 2022
@ncfavier
Copy link
Member Author

I've added Markdown rendering for longDescription.

As a reminder/note to self, XSS sanitisation is done by Elm.

@ncfavier
Copy link
Member Author

@ysndr ping

@ncfavier
Copy link
Member Author

(or @garbas)

@ysndr
Copy link
Member

ysndr commented Oct 28, 2022

Looks good, and brings in some welcome cleanup
Thank you @ncfavier
Is there a way to see the result in the preview?

@ncfavier
Copy link
Member Author

Is there a way to see the result in the preview?

You can look at tmux or geomyidae.

@ncfavier ncfavier merged commit ba0b05b into main Oct 28, 2022
@ncfavier ncfavier deleted the markdown-options branch October 28, 2022 13:49
@ncfavier
Copy link
Member Author

ncfavier commented Oct 28, 2022

Ah, I shouldn't have merged so soon. The nixpkgs-unstable import is currently broken (but a fix is underway), so that channel will be rolled back a bit until the next update.

Also it seems like the flake import is hitting GitHub API rate limits. Fix: #553

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants