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

ioreg: Extend doc comment syntax #135

Merged
merged 2 commits into from Aug 28, 2014

Conversation

Projects
None yet
3 participants
@bgamari
Contributor

bgamari commented Aug 18, 2014

This reworks the parser to distinguish between inner, outer, and trailing doc comments.

  • /// is an outer comment, applying to the next item in the current block
  • //= is a trailing comment, applying to the previous item in the current block
  • //! is an inner comment, applying to the owner of the current block.
For instance, we might document the above example as follows,
```
ioregs!(UART = {
0x0 => reg32 cr { ///! Control register
0x0 => reg32 cr { //! Control register

This comment has been minimized.

@bharrisau

bharrisau Aug 18, 2014

Contributor

Looks nicer.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 18, 2014

edit: This is out of date. See head text.

I would like to get some input on the exact semantics here as the language reference provides no details in this area. I propose the following:

  • If the first element in a block is an outer doc string then each outer doc string in the block applies to the item that follows it.
  • Otherwise, each outer doc string applies to the item that precedes it
  • Inner doc strings always apply to the parent item

This allows the user to select whether he wants to place doc comments before (appropriate for long discussions) or after (appropriate for short descriptions) the items they describe. Admittedly this is a bit complicated.

For instance, we might document the above example as follows,
```
ioregs!(UART = {
0x0 => reg32 cr { ///! Control register
0x0 => reg32 cr { //! Control register
0 => rxe, /// Receive enable

This comment has been minimized.

@bharrisau

bharrisau Aug 18, 2014

Contributor

Feels like that should refer to the next line?

/// Receive enable
0    => rxe,
/// Transmit enable
1    => txe,
@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 18, 2014

If you want to stick with the trailing comment look - I'd suggest you use a new syntax.

0    => rxe,    //= Receive enable

I'm sure you could bikeshed the glyph to use - but at the end of the day /// is preceding comment, //! is parent comment, and you want to use a trailing comment.

@farcaller

This comment has been minimized.

Member

farcaller commented Aug 18, 2014

Isn't it the case that end-of-line comment is considered as child? (so that //! works for inline comments)

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 18, 2014

Nope, just tested with rustc -Z ast-json. The doc comment is attached to the following line.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 18, 2014

Actually, that was for an outer comment. The inner //! comment just gives an error.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 18, 2014

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 18, 2014

@bharrisau I think you are probably right; a totally separate syntax is probably the best way forward here. I'm torn between //= and //<. Opinions?

@farcaller

This comment has been minimized.

Member

farcaller commented Aug 18, 2014

//= is simpler to type.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 18, 2014

@farcaller sounds good to me.

@bgamari bgamari changed the title from ioreg: Add support for inner doc comments to ioreg: Elaborate doc comment syntax Aug 18, 2014

@bgamari bgamari changed the title from ioreg: Elaborate doc comment syntax to ioreg: Extend doc comment syntax Aug 18, 2014

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 18, 2014

How's this look?

@farcaller

This comment has been minimized.

Member

farcaller commented Aug 18, 2014

LGTM, although 14..16 => parity { //! Parity selection will look better with pre-comment (///)

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 18, 2014

@farcaller yeah, I wanted to demonstrate the fact that multi-line comments can occur anywhere. I was about to write something about it being bad taste use multi-line trailing comments but maybe I should just eliminate this altogether.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 18, 2014

Vladimir Pouzanov notifications@github.com writes:

LGTM, although 14..16 => parity { //! Parity selection will look better with pre-comment (///)

Eliminated.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 28, 2014

Can we merge this?

@bgamari bgamari force-pushed the bgamari:ioreg-docs branch from 6579851 to 00658fe Aug 28, 2014

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Needs a rebase now.

bgamari added some commits Aug 14, 2014

ioreg::parser: Rework doc comments
Add support for inner and trailing doc comments.

@bgamari bgamari force-pushed the bgamari:ioreg-docs branch from 00658fe to 7778c49 Aug 28, 2014

@bgamari

This comment has been minimized.

Contributor

bgamari commented Aug 28, 2014

@bharrisau done.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Good - should be good to merge once Travis confirms.

I didn't realise there were no tests for ioreg.

@bharrisau

This comment has been minimized.

Contributor

bharrisau commented Aug 28, 2014

Looks like the (1) in the ioreg tests is upsetting the snake case lint. Something to fix in a different PR.

bharrisau added a commit that referenced this pull request Aug 28, 2014

Merge pull request #135 from bgamari/ioreg-docs
ioreg: Extend doc comment syntax

@bharrisau bharrisau merged commit 42ac4c2 into hackndev:master Aug 28, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@bgamari bgamari deleted the bgamari:ioreg-docs branch Aug 28, 2014

@bgamari bgamari referenced this pull request Aug 28, 2014

Merged

systick: Refactoring. #152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment