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

Port over doc comments in route macros. #2022

Merged
merged 6 commits into from
Feb 24, 2021

Conversation

erikjohnston
Copy link
Contributor

@erikjohnston erikjohnston commented Feb 23, 2021

PR Type

Bug Fix / Feature

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

This allows documentation on the functions to appear in the generated
docs, e.g.:

/// The index page
async fn index() -> &'static str {
    "Hello!"
}

Closes #1389


I'm afraid I have no idea how to add tests for this beyond making sure this doesn't break when using docstrings.

This allows documentation on the functions to appear in the generated
docs, e.g.:

```rust
/// The index page
async fn index() -> &'static str {
    "Hello!"
}
```
@robjtede robjtede added A-codegen project: actix-web-codegen C-improvement Category: an improvement to existing functionality B-semver-patch and removed C-improvement Category: an improvement to existing functionality labels Feb 23, 2021
@robjtede
Copy link
Member

robjtede commented Feb 23, 2021

Does this handle multi line doc-comments? I don't recall if they're converted to one doc attribute with line breaks or multiple doc attrs.

These are converted into multiple attributes, and so we need to store
them all.
@erikjohnston
Copy link
Contributor Author

Does this handle multi line doc-comments? I don't recall if they're converted to one doc attribute with lie breaks or multiple doc attrs.

Huh, you're right, had no idea that is multi line comments worked. I've pushed a fix and manually tested.

Tangentially, I am wondering whether it'd be better to just copy all attributes over? It's probably not that useful, but would mean e.g. that people could have manually added #[allow(missing_docs)] to the structs.

@robjtede
Copy link
Member

Tangentially, I am wondering whether it'd be better to just copy all attributes over?

I'm apprehensive about that because it may seem strange that attributes meant for functions will not work on the generated struct. If you don't feel strongly that it should be done I'd like to wait until it is raised specifically.

@robjtede robjtede requested review from a team February 24, 2021 11:41
actix-web-codegen/src/route.rs Outdated Show resolved Hide resolved
actix-web-codegen/src/route.rs Outdated Show resolved Hide resolved
actix-web-codegen/src/route.rs Outdated Show resolved Hide resolved
actix-web-codegen/src/route.rs Outdated Show resolved Hide resolved
actix-web-codegen/src/route.rs Outdated Show resolved Hide resolved
erikjohnston and others added 2 commits February 24, 2021 11:51
Co-authored-by: Jonas Platte <jplatte@users.noreply.github.com>
@erikjohnston
Copy link
Contributor Author

Thank you both, my proc macro foo is still quite weak ❤️

@robjtede robjtede merged commit 42711c2 into actix:master Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen project: actix-web-codegen B-semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

proc-macro docstrings issue
3 participants