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

top-level doc comment should be allowed in zig fmt #2288

Closed
tiehuis opened this issue Apr 16, 2019 · 4 comments · Fixed by #3697
Closed

top-level doc comment should be allowed in zig fmt #2288

tiehuis opened this issue Apr 16, 2019 · 4 comments · Fixed by #3697
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@tiehuis
Copy link
Member

tiehuis commented Apr 16, 2019

Take the following program:

/// Top-level documentation.

/// This is A
pub const A = usize;

After applying zig fmt this is turned into:

/// Top-level documentation.
/// This is A
pub const A = usize;

I don't think we should collapse top-level documentation at the beginning of the ast. I note that we have Error.UnattachedDocComment in zig/ast.zig. I do think it still makes sense to emit an error for documentation which isn't attached in the middle of a file, as I'm not sure how it could be interpreted in a meaningful way.

Related, but running zig fmt on a file with only a doc comment deletes it.

@tiehuis
Copy link
Member Author

tiehuis commented Apr 16, 2019

This assumes we want top-level documentation. There may be alternatives that are better suited and we can play with more once the documentation generator is started.

For example, since a file is semantically a struct (by #1047), would we also allow this sort of top-level documentation as allowed as the first node in a struct ast. It may be better to handle these differently even if semantically they are the same, so have the least surprising behaviour.

@andrewrk andrewrk added this to the 0.5.0 milestone Apr 16, 2019
@andrewrk andrewrk added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. standard library This issue involves writing Zig code for the standard library. labels Apr 16, 2019
@tiehuis tiehuis mentioned this issue May 2, 2019
5 tasks
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 20, 2019
@Vexu
Copy link
Member

Vexu commented Oct 11, 2019

Are top level doc comments something we want now that documentation generation is here?

For normal containers and namespaces the need for top level doc comments can easily be avoided. However we'll probably need them if we want to document a package in its root file.

@Vexu
Copy link
Member

Vexu commented Oct 12, 2019

If we want container level doc comments we should probably copy rust and have a different kind of comment for them since properly parsing

/// container doc comment

/// declaration doc comment
const A = ...

requires the parser to understand whitespace whereas

//! container doc comment

/// declaration doc comment
const A = ...

doesn't.

@andrewrk
Copy link
Member

I agree with copying rust here. It's unfortunate that there will be 2 ways to add doc comments for a container which is not the file, but maybe zig fmt can canonicalize that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants