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
Generate stdlib doc pages #432
Conversation
d179ae0
to
c90a06e
Compare
This PR will need rebasing for the dependency version bumps. Also, it's not clear to me if we want to merge this immediately into |
b08fe67
to
221bd5f
Compare
e00d2e4
to
75dcfa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but checking "request changes" to be sure about the target branch of this PR and the target branch for Nickel set in the flake (the two don't seem to agree). Otherwise, code-wise, it's an accept
@@ -8,7 +8,7 @@ | |||
|
|||
inputs.nixpkgs.url = "nixpkgs/nixos-unstable"; | |||
inputs.flake-utils.url = "github:numtide/flake-utils"; | |||
inputs.nickel.url = "github:tweag/nickel/stable"; | |||
inputs.nickel.url = "github:tweag/nickel"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it on purpose, or a left-over from tests? If we are going to target master, I suppose we should merge this PR in an unstable
branch, and not in master. Although we plan to release 1.0 very soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's left over from testing. It would probably make sense to merge this PR into an unstable
branch on nickel-lang.org
and change this input over before integrating into master
.
gatsby-node.js
Outdated
@@ -21,22 +23,29 @@ exports.createPages = async ({ actions, graphql, reporter }) => { | |||
frontmatter { | |||
slug | |||
} | |||
parent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this new node doing? Is it making accessible the name of the file or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's exactly what it does. Each node returned by the allMarkdownRemark
query has a parent
field, that is an instance of a File
node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it doesn't give me the file name; it allows me to check which instance of gatsby-source-filesystem
the markdownRemark
node comes from. This is probably left over from my first attempts of rendering the stdlib. I should probably get rid of it, since we're now using the JSON documentation export and rendering the markdown manually.
actions.createNode(newNode); | ||
actions.createParentChildLink({ parent: node, child: newNode }) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I general I feel like this new blobs of code in this file could maybe be made easier to understand with a bit of documentation. I think I understand what it is doing, mostly pre-processing of stdlib and user manual data and make them readily available as structured data to be used easily by the other pages, but not sure I would get everything again in 6 months 😅
const slug = data.stdlibSection.slug; | ||
const name = data.stdlibSection.name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicky nitpick: I think we can write this in one line with const {slug, name} = data.stdlibSection;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
@vkleen I've created an unstable branch and made it the target. You'll probably have to rebase, but otherwise LGTM. |
a7b8a6a
to
feec151
Compare
* Generate stdlib doc pages * Tweak nickel syntax highlighting * Use nickel stdlibJson * Link to functions in stdlib sidebar * Redirect /stdlib to /stdlib/array * Hack hierarchical documentation into ToC sidebar * Render nested documentation * Take care of unused variable warnings * Move prism highlight onEffect into the correct component * Prototype for new stdlib hierarchy * run npm install * Address comments from code review * Revert an experiment with rendering markdown nodes * Run nix flake update
This is a first attempt at integrating documentation for the Nickel standard library into the website.