Add ability to define file-level annotations #190

Closed
kaelig opened this Issue Aug 27, 2014 · 32 comments

Comments

Projects
None yet
7 participants
@kaelig

kaelig commented Aug 27, 2014

In the SassDoc for Guss, which is essentially multiple libraries put together, I wanted each library to be part of its own group (making a clear separation between, say, CSS3 mixins and typography styles).

I ended up writing @group my-component a lot, since it had to be in all of the mixins / variables / functions that I documented. For example, in this file, you'll find @group typography 16 times.

To be more dry, is there something we could do about it, like a comment at the beginning of the file:

/**
 * @group typography
 */

That would set the group of all the mixins/variables/functions in the file to "typography" (unless set otherwise)?

This implementation is barely a suggestion. The important part of this open ticket is the problem of code repetition, not the bit of code above.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Aug 27, 2014

Member

I can only +1 on this one, although I did not have has many items as you in my libs, I already found it quite tedious (and error prone) to add groups to each of them.

I guess we would need a different syntax for per item group and per file group.

/**
 * @global-group typography
 */

This features would obviously had a bit of complexity to the code, but for good.
That might be a job for sassdoc-filter.
There's already the file property which can help in this process.

Member

pascalduez commented Aug 27, 2014

I can only +1 on this one, although I did not have has many items as you in my libs, I already found it quite tedious (and error prone) to add groups to each of them.

I guess we would need a different syntax for per item group and per file group.

/**
 * @global-group typography
 */

This features would obviously had a bit of complexity to the code, but for good.
That might be a job for sassdoc-filter.
There's already the file property which can help in this process.

@alademann

This comment has been minimized.

Show comment
Hide comment
@alademann

alademann Aug 27, 2014

Contributor

To me, this relates to the discussion about Documenting BEM / Modules... Being able to define a group of related "things" with one docblock comment would be sweeet.

Contributor

alademann commented Aug 27, 2014

To me, this relates to the discussion about Documenting BEM / Modules... Being able to define a group of related "things" with one docblock comment would be sweeet.

@mattdrose

This comment has been minimized.

Show comment
Hide comment
@mattdrose

mattdrose Aug 27, 2014

I think it's pretty common practice for devs to organize their tools/modules into separate files. It'd be cool if the documentation had an option to automatically group things based on the file. You could use something similar to JSDoc's @file tag.

Furthermore, it'd be even better if you could have a higher level of grouping based on directories. You could use a navigation in the header to jump between these directories.

I've been using DSS to achieve exactly this at work. It gives you the ability to document large frameworks in an organized manner.

Here's a screenshot of what I setup—notice the menu:
screen shot 2014-08-27 at 5 16 05 pm

I think it's pretty common practice for devs to organize their tools/modules into separate files. It'd be cool if the documentation had an option to automatically group things based on the file. You could use something similar to JSDoc's @file tag.

Furthermore, it'd be even better if you could have a higher level of grouping based on directories. You could use a navigation in the header to jump between these directories.

I've been using DSS to achieve exactly this at work. It gives you the ability to document large frameworks in an organized manner.

Here's a screenshot of what I setup—notice the menu:
screen shot 2014-08-27 at 5 16 05 pm

@HugoGiraudel HugoGiraudel added this to the 2.0 milestone Aug 27, 2014

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Aug 27, 2014

Member

2.0 or before if we can make it, but no rush on this one.

Member

HugoGiraudel commented Aug 27, 2014

2.0 or before if we can make it, but no rush on this one.

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Aug 29, 2014

Member

+1

I think this should reside in the parser since it's all about parsing another comment type... sassdoc-filter shouldn't have to open the file again to make its own parsing for the global comment.

Maybe package this as an extension to the parser? I don't know enough scss-comment-parser to know what's the best solution for this. But if we can do like JSDoc with @file, it would be great.

Member

valeriangalliat commented Aug 29, 2014

+1

I think this should reside in the parser since it's all about parsing another comment type... sassdoc-filter shouldn't have to open the file again to make its own parsing for the global comment.

Maybe package this as an extension to the parser? I don't know enough scss-comment-parser to know what's the best solution for this. But if we can do like JSDoc with @file, it would be great.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Aug 29, 2014

Member

I like the @file annotation as well.

scss-comment-parser should obviously parse it, but should not be responsible for the later data massaging.

sassdoc-filter should of course not open any file, but I though it would need to make the distinction between @file and @group items.

Let's try to define the behavior clearly:

  • If @file is present at the top of a file, all items in that file will inherit this file/group.
  • If an item in such a file has an @group annotation, it will override the @file for that item.
  • We might need to introduce a new property in the view.json .sassdocrc for @file friendly names ?

I'm wondering how this could go along nicely with nested groups #135 and BEM #18
If an item override @file with an @group should this group be considered a top level one, or a sub group of @file ...

Member

pascalduez commented Aug 29, 2014

I like the @file annotation as well.

scss-comment-parser should obviously parse it, but should not be responsible for the later data massaging.

sassdoc-filter should of course not open any file, but I though it would need to make the distinction between @file and @group items.

Let's try to define the behavior clearly:

  • If @file is present at the top of a file, all items in that file will inherit this file/group.
  • If an item in such a file has an @group annotation, it will override the @file for that item.
  • We might need to introduce a new property in the view.json .sassdocrc for @file friendly names ?

I'm wondering how this could go along nicely with nested groups #135 and BEM #18
If an item override @file with an @group should this group be considered a top level one, or a sub group of @file ...

@mattdrose

This comment has been minimized.

Show comment
Hide comment
@mattdrose

mattdrose Aug 29, 2014

I like that idea of having @file and @group as two separate things, rather than having it override. 1) I think it's confusing because it's not intuitive that these two things are related and 2) separating them gives more flexibility as to what you can do with the extracted data.

Maybe in the @file comment block, you could set things like @author, @copyright, @group, etc. which would act as global settings to the file. Then you could override them if you specify them later in the file for individual items.

Also, as per grouping, we might be able to learn from the different tags JSDoc uses for grouping: @namespace and @module. They use some interesting techniques for nesting (@namespace NamespaceName.ChildItemName). Also, it's interesting that when you set @module, it assumes everything in that file is a part of that module unless otherwise specified.

I like that idea of having @file and @group as two separate things, rather than having it override. 1) I think it's confusing because it's not intuitive that these two things are related and 2) separating them gives more flexibility as to what you can do with the extracted data.

Maybe in the @file comment block, you could set things like @author, @copyright, @group, etc. which would act as global settings to the file. Then you could override them if you specify them later in the file for individual items.

Also, as per grouping, we might be able to learn from the different tags JSDoc uses for grouping: @namespace and @module. They use some interesting techniques for nesting (@namespace NamespaceName.ChildItemName). Also, it's interesting that when you set @module, it assumes everything in that file is a part of that module unless otherwise specified.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Aug 31, 2014

Member

I would like this feature to be one of the first we work on for v2 (or even before if we can implement it without breaking anything). I feel like having a way to set file-specific annotations is a real plus for architecture that has a huge number of files.

On the top of my head, following annotations might be useful at a file level:

  • @access
  • @author
  • @deprecated
  • @group
  • @since
  • @todo

Name and description might also come in handy at some point.

On the other hand, some annotations make absolutely no sense at a file level (e.g. @param, @return...). We need to find how to handle those.

Member

HugoGiraudel commented Aug 31, 2014

I would like this feature to be one of the first we work on for v2 (or even before if we can implement it without breaking anything). I feel like having a way to set file-specific annotations is a real plus for architecture that has a huge number of files.

On the top of my head, following annotations might be useful at a file level:

  • @access
  • @author
  • @deprecated
  • @group
  • @since
  • @todo

Name and description might also come in handy at some point.

On the other hand, some annotations make absolutely no sense at a file level (e.g. @param, @return...). We need to find how to handle those.

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Aug 31, 2014

Member

To make it simple, i'd say: every file annotation is affected to all items of the file, unless an item annotation overrides it, or unless there's specific code about some annotations (I think of @group in particular, that would be appended instead).

Member

valeriangalliat commented Aug 31, 2014

To make it simple, i'd say: every file annotation is affected to all items of the file, unless an item annotation overrides it, or unless there's specific code about some annotations (I think of @group in particular, that would be appended instead).

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Aug 31, 2014

Member

every file annotation is affected to all items of the file, unless an item annotation overrides it

Thanks for summarizing it in a one liner :)

Member

pascalduez commented Aug 31, 2014

every file annotation is affected to all items of the file, unless an item annotation overrides it

Thanks for summarizing it in a one liner :)

@HugoGiraudel HugoGiraudel changed the title from Global @group override in a file to File-level annotations Sep 7, 2014

@HugoGiraudel HugoGiraudel modified the milestones: 2.0, 1.7 Sep 7, 2014

@HugoGiraudel HugoGiraudel removed the Maybe label Sep 7, 2014

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 7, 2014

Member

We need @FWeinb on this one, but I don't see any reason not to keep it under 1.x, so we'll draw a 1.7 for this.

Member

HugoGiraudel commented Sep 7, 2014

We need @FWeinb on this one, but I don't see any reason not to keep it under 1.x, so we'll draw a 1.7 for this.

@HugoGiraudel HugoGiraudel changed the title from File-level annotations to Add ability to define file-level annotations Sep 8, 2014

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Sep 9, 2014

Member

This can become kinda hard to implement. I am searching for the context of each comment ignoring the whitespace in scss-comment-parser so i can't really detect a comment without a context.

Member

FWeinb commented Sep 9, 2014

This can become kinda hard to implement. I am searching for the context of each comment ignoring the whitespace in scss-comment-parser so i can't really detect a comment without a context.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 9, 2014

Member

Mpf. @valeriangalliat was suggesting this: when opening a file, check if there is a comment that's not directly followed by an item. If there is, it's a file-level annotation. Could it be possible?

Member

HugoGiraudel commented Sep 9, 2014

Mpf. @valeriangalliat was suggesting this: when opening a file, check if there is a comment that's not directly followed by an item. If there is, it's a file-level annotation. Could it be possible?

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Sep 19, 2014

Member

How would you define directly? Currently I am just searching for a hit without considering the size of the gap between the comment and actual code.

Member

FWeinb commented Sep 19, 2014

How would you define directly? Currently I am just searching for a hit without considering the size of the gap between the comment and actual code.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 19, 2014

Member

Would it be possible do do something like: if there is an @file in this block comment, then don't bother about context at all ?

Member

pascalduez commented Sep 19, 2014

Would it be possible do do something like: if there is an @file in this block comment, then don't bother about context at all ?

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 19, 2014

Member

Actually @FWeinb just found a way, he'll post about it shortly.

Member

HugoGiraudel commented Sep 19, 2014

Actually @FWeinb just found a way, he'll post about it shortly.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 19, 2014

Member

One thing I was thinking about the other night: item-specific annotations should always override the file-level annotations, not pile up. For instance, a @link file-level annotation would cascade to all items from file, except those with a @link annotation defined.

Member

HugoGiraudel commented Sep 19, 2014

One thing I was thinking about the other night: item-specific annotations should always override the file-level annotations, not pile up. For instance, a @link file-level annotation would cascade to all items from file, except those with a @link annotation defined.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Sep 19, 2014

Member

Edit from @HugoGiraudel: the idea is to give users a way to define annotations that are being applied at a file level. For instance, you could apply @group once in a file so all items from this file are considered as having this very @group annotation. We need some help to find the best solution about this.

I think we could do it as follows:

First define a "poster" comment block like:

/**
 * Poster Comment 
 **/ 

You see there are at least two * at the end.

Than we need to think about how we want these comments to work.
I can think of two basic ways:

  1. Just define one per file and apply every annotation to all other items. Item-specific annotations are always preferred.
  2. Use them as "Headlines" to group specify annotations at the beginning of a section. So a poster comment would effect all following comments until a new poster comment is found. So the rules that are used are always the latest post comment ones.

I would go for 1 because you should group your sass code by file rather than by specify section in one file, but that is just me.

I hope you get the idea here.

Member

FWeinb commented Sep 19, 2014

Edit from @HugoGiraudel: the idea is to give users a way to define annotations that are being applied at a file level. For instance, you could apply @group once in a file so all items from this file are considered as having this very @group annotation. We need some help to find the best solution about this.

I think we could do it as follows:

First define a "poster" comment block like:

/**
 * Poster Comment 
 **/ 

You see there are at least two * at the end.

Than we need to think about how we want these comments to work.
I can think of two basic ways:

  1. Just define one per file and apply every annotation to all other items. Item-specific annotations are always preferred.
  2. Use them as "Headlines" to group specify annotations at the beginning of a section. So a poster comment would effect all following comments until a new poster comment is found. So the rules that are used are always the latest post comment ones.

I would go for 1 because you should group your sass code by file rather than by specify section in one file, but that is just me.

I hope you get the idea here.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 19, 2014

Member

How complicated would the second option be?

Member

HugoGiraudel commented Sep 19, 2014

How complicated would the second option be?

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Sep 19, 2014

Member

The parser would need to keep state between each "poster" comment.

Member

FWeinb commented Sep 19, 2014

The parser would need to keep state between each "poster" comment.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 19, 2014

Member

I like the second option, but I wonder if that wouldn't be a bit overkill. Let me build a quick poll.

Member

HugoGiraudel commented Sep 19, 2014

I like the second option, but I wonder if that wouldn't be a bit overkill. Let me build a quick poll.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 19, 2014

Member

Option 2 is seducing, but I guess it would introduce complexity and possible confusion.
Agree with @FWeinb that enforcing clear files separation is preferred.
So my vote for 1.

Member

pascalduez commented Sep 19, 2014

Option 2 is seducing, but I guess it would introduce complexity and possible confusion.
Agree with @FWeinb that enforcing clear files separation is preferred.
So my vote for 1.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 19, 2014

Member

One feature with @file I would also fancy (could be added later), would be to specify per file short introduction text or description.

So that if I decide to keep the file organization into my theme, I can provide a description per section.

Member

pascalduez commented Sep 19, 2014

One feature with @file I would also fancy (could be added later), would be to specify per file short introduction text or description.

So that if I decide to keep the file organization into my theme, I can provide a description per section.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 19, 2014

Member

I've settled. First option. KISS. Let's go.

Member

HugoGiraudel commented Sep 19, 2014

I've settled. First option. KISS. Let's go.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Sep 20, 2014

Member

I am on implementing it. I decided to have this order of specificity:
Item-specify -> Poster Comment -> default value

So to give some examples:

  • an item annotation will never be overwritten by an annotation in a poster comment
  • a default value of a annotation can be overwritten by a annotation in a poster comment
  • all annotations defined in the poster comment will be overwrite to all items after that followed that poster comment
  • You can only have one poster comment per file.

While implementing it solution 2 would not be to hard to implement.

Member

FWeinb commented Sep 20, 2014

I am on implementing it. I decided to have this order of specificity:
Item-specify -> Poster Comment -> default value

So to give some examples:

  • an item annotation will never be overwritten by an annotation in a poster comment
  • a default value of a annotation can be overwritten by a annotation in a poster comment
  • all annotations defined in the poster comment will be overwrite to all items after that followed that poster comment
  • You can only have one poster comment per file.

While implementing it solution 2 would not be to hard to implement.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 20, 2014

Member

👍

Member

pascalduez commented Sep 20, 2014

👍

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 20, 2014

Member

all annotations defined in the poster comment will be merged to all items after that followed that poster comment

No. Not merged. Overridden.

Member

HugoGiraudel commented Sep 20, 2014

all annotations defined in the poster comment will be merged to all items after that followed that poster comment

No. Not merged. Overridden.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Sep 20, 2014

Member

I meant that. Fixed it.

Member

FWeinb commented Sep 20, 2014

I meant that. Fixed it.

@FWeinb FWeinb self-assigned this Sep 20, 2014

@FWeinb FWeinb closed this Sep 20, 2014

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 20, 2014

Member

I just added a data unit test for it, this feature totally rocks the place !

Member

pascalduez commented Sep 20, 2014

I just added a data unit test for it, this feature totally rocks the place !

@kaelig

This comment has been minimized.

Show comment
Hide comment
@kaelig

kaelig Sep 22, 2014

Thanks @pascalduez. That link to the test case seems broken.

kaelig commented Sep 22, 2014

Thanks @pascalduez. That link to the test case seems broken.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Sep 22, 2014

Member

@kaelig I think @pascalduez wanted to link to this.

Member

FWeinb commented Sep 22, 2014

@kaelig I think @pascalduez wanted to link to this.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 22, 2014

Member

@kaelig I think @pascalduez wanted to link to this.

That's the one, thanks @FWeinb :)

Member

pascalduez commented Sep 22, 2014

@kaelig I think @pascalduez wanted to link to this.

That's the one, thanks @FWeinb :)

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