Using any `autofill` options in `.sassdocrc` won't document items #399

Closed
indrekpaas opened this Issue Apr 17, 2015 · 10 comments

Comments

Projects
None yet
5 participants
@indrekpaas

Using

autofill: []

or any combination of

autofill:
  - throw
  - content
  - require

will produce an "empty" output file, as if there was no items to document, and fails silently.

@indrekpaas indrekpaas changed the title from Using any `autofill` options in `.sassdocrc` won't generate documented output to Using any `autofill` options in `.sassdocrc` won't document items Apr 17, 2015

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Apr 24, 2015

Member

Hey! Sorry for the late reply. You need to autofill access, otherwise all items are marked @access auto, and the theme don't know how to display this.

Member

valeriangalliat commented Apr 24, 2015

Hey! Sorry for the late reply. You need to autofill access, otherwise all items are marked @access auto, and the theme don't know how to display this.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Apr 24, 2015

Member

You need to autofill access, otherwise all items are marked @access auto, and the theme don't know how to display this.

Really ? Is that the intended behavior ? ¯\_(ツ)_/¯

Member

pascalduez commented Apr 24, 2015

You need to autofill access, otherwise all items are marked @access auto, and the theme don't know how to display this.

Really ? Is that the intended behavior ? ¯\_(ツ)_/¯

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Apr 24, 2015

Member

Well, @access was never thought to not be autofilled I guess. This is why I didn't close the issue, there's maybe stuff to change at this level.

Member

valeriangalliat commented Apr 24, 2015

Well, @access was never thought to not be autofilled I guess. This is why I didn't close the issue, there's maybe stuff to change at this level.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jun 1, 2015

Member

Ran through this issue today. We need to fix it, it's definitely correct. :x

Member

HugoGiraudel commented Jun 1, 2015

Ran through this issue today. We need to fix it, it's definitely correct. :x

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jun 4, 2015

Member

@access could be public by default so it still works when not autofilled, but if we do this we won't be able to distinguish between an explicitely public item and an item with the default value.

So when autofilled, the following code:

/// @access public
@function _test() {}

would and end up to be private.

Or the theme should handle auto access value, but I don't like this.

Or we can return something like Object.assign('public', { auto: true }) from the default function, and this way we'll be able to test from the autofill function if it's the default value or an explicit public annotation, but it's a hack.

Member

valeriangalliat commented Jun 4, 2015

@access could be public by default so it still works when not autofilled, but if we do this we won't be able to distinguish between an explicitely public item and an item with the default value.

So when autofilled, the following code:

/// @access public
@function _test() {}

would and end up to be private.

Or the theme should handle auto access value, but I don't like this.

Or we can return something like Object.assign('public', { auto: true }) from the default function, and this way we'll be able to test from the autofill function if it's the default value or an explicit public annotation, but it's a hack.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jun 4, 2015

Member

What a clusterfuck.

How about to force autofill to contains access ?
Not particularly nice but at least there shouldn't be side effects, nor breakages ?

Member

pascalduez commented Jun 4, 2015

What a clusterfuck.

How about to force autofill to contains access ?
Not particularly nice but at least there shouldn't be side effects, nor breakages ?

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jun 4, 2015

Member

Well for sure that's better than what I was proposing!

We could hook something like this in ensureEnvironment:

if (env.autofill && env.autofill.indexOf('access') === -1) {
  env.autofill.push('access');
}
Member

valeriangalliat commented Jun 4, 2015

Well for sure that's better than what I was proposing!

We could hook something like this in ensureEnvironment:

if (env.autofill && env.autofill.indexOf('access') === -1) {
  env.autofill.push('access');
}
@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jun 5, 2015

Member

I like this 👍

Member

FWeinb commented Jun 5, 2015

I like this 👍

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jun 5, 2015

Member

In fact it should more be in Environment.postProcess. Anyway, I'm not sure about this. I recall that we have a resolve function at annotation level, that could convert any auto to public when not autofilled. It's the cleanest solution I have in mind. I'm implementing this.

Member

valeriangalliat commented Jun 5, 2015

In fact it should more be in Environment.postProcess. Anyway, I'm not sure about this. I recall that we have a resolve function at annotation level, that could convert any auto to public when not autofilled. It's the cleanest solution I have in mind. I'm implementing this.

@valeriangalliat valeriangalliat self-assigned this Jun 5, 2015

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jun 5, 2015

Member

Fixed in 2.1.14.

Member

valeriangalliat commented Jun 5, 2015

Fixed in 2.1.14.

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