Skip to content

Add more documentation do Syntax nodes #2153

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

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

kimdv
Copy link
Contributor

@kimdv kimdv commented Sep 5, 2023

Part of #1528

I've started to add more documentation to the syntax nodes.
Some of them are pretty trivial and not sure if that is the path we want.

Like the effect specifiers, we could generate them if we wan't them.
I think they give some value specially places where we use asyncSpecifier etc. Here are two valid options.
We could generate

The `async` or `reasync` keyword.

Then they could be able to click on them and navigate to it.

When generating this, we could make rule, that if we define the documentation then it overrules the generated ones

@kimdv kimdv requested a review from ahoppen as a code owner September 5, 2023 18:00
@kimdv kimdv force-pushed the kimdv/add-some-documentation branch from dc8e6a7 to f5ba1b1 Compare September 5, 2023 18:03
Copy link
Contributor

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

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

Just a few suggestions from my side ...

@kimdv kimdv force-pushed the kimdv/add-some-documentation branch from f5ba1b1 to 5b86298 Compare September 5, 2023 20:18
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me. Thanks @kimdv. I wonder if cases like

The `actor` keyword

can be handled as part of #1987

CC @natikgadzhi

@natikgadzhi
Copy link
Contributor

@ahoppen @kimdv yep! I haven't jumped into #1987 fully yet, but it does feel like when a child has just one / few possible kinds or specific tokens — we could generate documentation on those automatically based on those kinds, without hard-coding the specific doc string for each child. Thank you for nudging me here! <3

@kimdv
Copy link
Contributor Author

kimdv commented Sep 6, 2023

Mostly looks good to me. Thanks @kimdv. I wonder if cases like

The `actor` keyword

can be handled as part of #1987

CC @natikgadzhi

Will remove those cases again.
Then it can be handled in #1987.

@kimdv kimdv force-pushed the kimdv/add-some-documentation branch 2 times, most recently from 40ae2eb to 71196c6 Compare September 20, 2023 19:39
@kimdv kimdv requested a review from bnbarham as a code owner September 20, 2023 19:39
@kimdv
Copy link
Contributor Author

kimdv commented Sep 20, 2023

@ahoppen added more documentation.
Should we review and merge this or should I add more?

Can also add more in a separate PR if you want?

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Yes, let’s merge this. Just looks like the change to CodeGeneration/Package.swift is unrelated.

@kimdv kimdv force-pushed the kimdv/add-some-documentation branch from 71196c6 to 8095caf Compare September 21, 2023 05:44
@kimdv
Copy link
Contributor Author

kimdv commented Sep 21, 2023

@swift-ci please test

@kimdv kimdv enabled auto-merge September 21, 2023 05:47
@kimdv
Copy link
Contributor Author

kimdv commented Sep 21, 2023

@swift-ci please test windows

@kimdv kimdv merged commit b2cbece into swiftlang:main Sep 21, 2023
@kimdv kimdv deleted the kimdv/add-some-documentation branch September 21, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants