Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions .github/workflows/docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ permissions:
on:
workflow_dispatch:
push:
branches:
- main
branches: ['main', 'rel-*', 'ci/*']
pull_request:
merge_group:
schedule:
- cron: '0 18 * * *'

Expand All @@ -29,23 +30,29 @@ jobs:
with:
tool: [email protected] # Matched to rustls repo

- name: Generate API JSON data
run:
cargo run -p rustls-ffi-tools --bin docgen > website/static/api.json
- name: Verify API docs JSON data
run: |
if ! diff website/static/api.json <(cargo run -p rustls-ffi-tools --bin docgen 2>/dev/null); then
echo
echo "ERROR: Generated api.json differs from checked-in version"
echo "Run 'cargo run -p rustls-ffi-tools --bin docgen > website/static/api.json' and commit the changes"
exit 1
fi

- name: Generate site pages
run: |
cd website && zola build --output-dir ../target/website/

- name: Package and upload artifact
uses: actions/upload-pages-artifact@v4
uses: actions/upload-pages-artifact@v3
if: github.ref == 'refs/heads/main'
with:
path: ./target/website/

deploy:
name: Deploy
runs-on: ubuntu-latest
if: github.repository == 'rustls/rustls-ffi'
if: github.repository == 'rustls/rustls-ffi' && github.ref == 'refs/heads/main'
needs: generate
permissions:
pages: write
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@ librustls/cmake-build*
.vs
debian/usr
debian/DEBIAN
/website/static/api.json
/website/public
41 changes: 32 additions & 9 deletions tools/src/bin/docgen/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,39 +179,62 @@ fn process_doc_item(item: Node, src: &[u8]) -> Result<Option<Item>, Box<dyn Erro
"type_definition" => process_typedef_item(comment, feat_requirement, item, src)?,
"enum_specifier" => Item::from(EnumItem::new(comment, feat_requirement, item, src)?),
"declaration" => process_declaration_item(comment, feat_requirement, item, src)?,
_ => return Err(format!("unexpected item kind: {}", item.kind()).into()),
_ => return Err(format!("unexpected item kind: {kind}").into()),
}))
}

/// Convert the preceding sibling of a to-be-processed item into associated metadata
///
/// An item `Node` to be processed for documentation will typically have associated
/// metadata `Node`s preceding it in the parse tree. This function returns an optional
/// `Comment` and/or optional `Feature` from processing the sibling `prev` `Node`.
///
/// The potential cases we care about are:
/// * `prev` is not a comment, and not a feature requirement.
/// * `prev` is a Comment, and has no feature requirement before it.
/// * `prev` is a Comment, and has a feature requirement before it.
/// * `prev` is a bare feature requirement
///
/// cbindgen won't create a comment before a feature requirement so we don't have to
/// consider that case.
fn comment_and_requirement(
node: Node,
prev: Node,
src: &[u8],
) -> Result<(Option<Comment>, Option<Feature>), Box<dyn Error>> {
let mut maybe_comment = Comment::new(node, src).ok();
let prev_prev = prev.prev_named_sibling();
// In the simple case, `prev` is a comment and `prev_prev` may be a feature requirement.
if let Ok(comment) = Comment::new(prev, src) {
Copy link
Member

Choose a reason for hiding this comment

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

So IMO this should also touch the maybe_comment code below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you be more specific about the change you want to see? I've reworked this twice and I'm losing steam. It feels fairly unimportant in the context of the overall change + a utility that isn't part of the core crate at all.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I completely understand. I just think this change looks weird: you match if let Ok(comment) = Comment::new(prev, src) here and return for that case.

But directly below that you have another instance of Comment::new(prev, src), this time calling .ok() on it, storing it into maybe_comment. I think that will always be None, so a bunch of the conditionals below should fall away (if let (None, ..) = (&maybe_comment, ..) and also if maybe_comment.is_none()). That was sort of the point of my previous comment, and this version doesn't really address it.

return Ok((
Some(comment),
prev_prev.and_then(|prev_prev| Feature::new(prev_prev, src).ok()),
));
}

let mut maybe_comment = Comment::new(prev, src).ok();

// If node wasn't a comment, see if it was an expression_statement
// that itself was preceded by a comment. This skips over
// expression-like preprocessor attributes on function decls.
if let (None, "expression_statement", Some(prev)) =
(&maybe_comment, node.kind(), node.prev_sibling())
if let (None, "expression_statement", Some(prev_prev)) =
(&maybe_comment, prev.kind(), prev_prev)
{
maybe_comment = Comment::new(prev, src).ok();
maybe_comment = Comment::new(prev_prev, src).ok();
}

// If prev wasn't a comment, see if it was a feature requirement.
if maybe_comment.is_none() {
return Ok(match Feature::new(node, src).ok() {
return Ok(match Feature::new(prev, src).ok() {
Some(feat_req) => (None, Some(feat_req)),
None => (None, None),
});
}

// Otherwise, check the prev of the comment for a feature requirement
let Some(prev) = node.prev_named_sibling() else {
let Some(prev_prev) = prev_prev else {
return Ok((maybe_comment, None));
};

Ok((maybe_comment, Feature::new(prev, src).ok()))
Ok((maybe_comment, Feature::new(prev_prev, src).ok()))
}

#[derive(Debug, Default, Serialize)]
Expand Down
Loading