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

Implement docstrings #267

Merged
merged 14 commits into from
Sep 29, 2021
Merged

Implement docstrings #267

merged 14 commits into from
Sep 29, 2021

Conversation

emilyaherbert
Copy link
Contributor

@emilyaherbert emilyaherbert commented Sep 24, 2021

Closes #149

For the test case:

script;

/// Enum representing either a number or a string
///
/// # Examples
///
/// `NumberOrString::Number(42)`
/// `NumberOrString::String("foo")`
enum NumberOrString {
	/// The `Number` variant in `NumberOrString`
	Number: u64,
	/// The `String` variant in `NumberOrString`
	String: str[4],
}

/// Struct holding:
///
/// 1. A `value` of type `NumberOrString`
/// 2. An `address` of type `byte`
struct Data {
	/// The `value` field in `Data`
	value: NumberOrString,
	/// The `address` field in `Data`
	address: byte,
}

/// The main function that does all the things!
fn main() -> u64 {
  let mut data = Data { 
                  value: NumberOrString::Number(20),
                  address: 0b00001111,
                };

  return 20;
}

It produces vec:

{
    "struct.Data.value": [
        "/// The `value` field in `Data`",
    ],
    "fn.main": [
        "/// The main function that does all the things!",
    ],
    "enum.NumberOrString": [
        "/// Enum representing either a number or a string",
        "///",
        "/// # Examples",
        "///",
        "/// `NumberOrString::Number(42)`",
        "/// `NumberOrString::String(\"foo\")`",
    ],
    "enum.NumberOrString.String": [
        "/// The `String` variant in `NumberOrString`",
    ],
    "struct.Data.address": [
        "/// The `address` field in `Data`",
    ],
    "struct.Data": [
        "/// Struct holding:",
        "///",
        "/// 1. A `value` of type `NumberOrString`",
        "/// 2. An `address` of type `byte`",
    ],
    "enum.NumberOrString.Number": [
        "/// The `Number` variant in `NumberOrString`",
    ],
}

It doesn't actually do anything with the vec, it just kind of keeps it around for now. I suppose implementing something with it will be a seperate issue.

@emilyaherbert emilyaherbert self-assigned this Sep 24, 2021
@emilyaherbert emilyaherbert added compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request labels Sep 24, 2021
@emilyaherbert emilyaherbert force-pushed the emilyaherbert-149/docstrings branch from fddca66 to ae2e851 Compare September 24, 2021 14:59
@emilyaherbert emilyaherbert marked this pull request as ready for review September 24, 2021 20:33
@emilyaherbert emilyaherbert mentioned this pull request Sep 24, 2021
@sezna
Copy link
Contributor

sezna commented Sep 24, 2021

This looks really good -- I think, for the sake of @leviathanbeak's sanity and usability, we should probably remove the /// from the strings and store it as one long string instead of separate lines. That way he can ingest the documentation directly instead of doing further processing.

cc @leviathanbeak -- what do you think about this for cargo doc?

@adlerjohn
Copy link
Contributor

and store it as one long string instead of separate lines

Wouldn't that result in information loss? There's cases where documentation should be in different lines (e.g. there's an inline code block).

@sezna
Copy link
Contributor

sezna commented Sep 25, 2021

and store it as one long string instead of separate lines

Wouldn't that result in information loss? There's cases where documentation should be in different lines (e.g. there's an inline code block).

We could keep newline characters, my meaning is just to not keep them as separate items in an array.

@leviathanbeak
Copy link
Contributor

@sezna yeah makes sense, no need to keep the "///" just for me to clean them up - while having separate (new)lines makes sense as well. nice job @emilyaherbert

@emilyaherbert
Copy link
Contributor Author

{
    "struct.Data.address": [
        "The `address` field in `Data`",
    ],
    "fn.main": [
        "The main function that does all the things!",
    ],
    "struct.Data.value": [
        "The `value` field in `Data`",
    ],
    "struct.Data": [
        "Struct holding:",
        "",
        "1. A `value` of type `NumberOrString`",
        "2. An `address` of type `byte`",
    ],
    "enum.NumberOrString.Number": [
        "The `Number` variant in `NumberOrString`",
    ],
    "enum.NumberOrString.String": [
        "The `String` variant in `NumberOrString`",
    ],
    "enum.NumberOrString": [
        "Enum representing either a number or a string",
        "",
        "# Examples",
        "",
        "`NumberOrString::Number(42)`",
        "`NumberOrString::String(\"foo\")`",
    ],
}

core_lang/src/lib.rs Outdated Show resolved Hide resolved
@sezna
Copy link
Contributor

sezna commented Sep 28, 2021

We could keep newline characters, my meaning is just to not keep them as separate items in an array.

Could we still do this? Just a .join("\n") is sufficient

sezna
sezna previously approved these changes Sep 29, 2021
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

approved with nits if you want to take them

core_lang/src/parse_tree/declaration/mod.rs Outdated Show resolved Hide resolved
core_lang/src/parse_tree/declaration/mod.rs Outdated Show resolved Hide resolved
@emilyaherbert emilyaherbert requested a review from sezna September 29, 2021 15:23
@emilyaherbert emilyaherbert merged commit 10637f5 into master Sep 29, 2021
@emilyaherbert emilyaherbert deleted the emilyaherbert-149/docstrings branch September 29, 2021 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement docstrings
4 participants