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

Fix annotation parsing in nested comment blocks #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davidknezic
Copy link
Contributor

I need to be able to document classes that are nested. Like in this example:

@each $breakpoint in map-keys($grid-breakpoints) {
  @include media-breakpoint-up($breakpoint) {

    /// text
    ///
    /// @example
    ///   <div class="fade-in-xs-up">
    ///     <span>Test</span>
    ///   </div>
    .fade-in-#{$breakpoint}-up {
      // ...
    }
  }
}

This works, except for all the annotation handling. Annotations don't get recognized and all land in the description. The reason for that is how indented comments get returned from CDocParser.CommentExtractor. The snippet above would result in this output:

{
  "lines": [
    "text",
    "   ",
    "    @example",
    "      <div class=\"fade-in-xs-up\">",
    "        <span>Test</span>",
    "      </div>"
  ],
  "type": "line",
  "commentRange": {
    "start": 43,
    "end": 48
  },
  "context": {}
}

The filterAndGroup function then filters and groups the lines and uses the following to detect annotations:

lines.forEach(function (line){
  var isAnnotation = line.indexOf('@') === 0;

This of course doesn't work, because some of the lines are indented.

This fix trims the comment lines to detect annotations, uses the trimmed lines when pushing into a new group and then keeps using the untrimmed line so intentional indentation doesn't get lost (ex. in @example source code).

@pascalduez
Copy link
Member

Hi,

thanks for the PR!
The changes looks good to me. Although I sorta remember we stated long time ago that nested code documentation was not possible/desired in the current state of things... Most probably inside functions, mixins declarations. Well, if it works without breaking things that's a pretty nifty patch :)

Could you please write a few unit tests so that the feature is clearly defined?

Also running the unit tests from the main sassdoc lib with that patched dependency would be quite reassuring.

@deap82
Copy link

deap82 commented Feb 27, 2019

What's the latest on this?

I'm writing my scss with bem conventions like this:

/// This works good
.some-block {
    /.../

    /// This also works
    &--small {
        /.../
    }

    /// This example annotation does not work
    /// @example html
    /// <div class="some-block some-block--large">My large block</div>
    &--large {
        /.../
    }
}

I'm using the Herman theme (https://github.com/oddbird/sassdoc-theme-herman) and use a @group for each block level class, so documenting "&--small" works really well as I get a page for each block level, and therefore it's clear that the &--large is a documentation of .some-block--large.

But I'd really need the @example annotation to work even if it is indented...

Update:
Well, I'm using gulp, so installed gulp-replace and did this in my sassdoc gulp task:

	return gulp.src(scssPaths)
		.pipe(replace('\t\t\t/// @', '/// @'))
		.pipe(replace('\t\t/// @', '/// @'))
		.pipe(replace('\t/// @', '/// @'))
		.pipe(sassdoc(options));

@pascalduez
Copy link
Member

What's the latest on this?

Needs unit tests.

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

Successfully merging this pull request may close these issues.

None yet

3 participants