@group #29

Closed
pascalduez opened this Issue Jul 1, 2014 · 58 comments

Comments

Projects
None yet
7 participants
@pascalduez
Member

pascalduez commented Jul 1, 2014

A nice improvement would be to add group creation and ordering.
Right now the codebase is split in functions, mixins, variables. In that order.

With groups, one could create an helpers, API, output, whatever group.
Group order or weights, would allow to organize groups by importance. Moving the public API on top, or the like.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 1, 2014

Member

I like that.

Member

HugoGiraudel commented Jul 1, 2014

I like that.

@alademann

This comment has been minimized.

Show comment
Hide comment
@alademann

alademann Jul 3, 2014

Contributor

+1

Contributor

alademann commented Jul 3, 2014

+1

@varemenos

This comment has been minimized.

Show comment
Hide comment
@varemenos

varemenos Jul 3, 2014

+1, I'm working on a SASS library and I need to separate my partials into groups/categories

+1, I'm working on a SASS library and I need to separate my partials into groups/categories

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 3, 2014

Member

What should happen with items without the @group annotation? Shall we have an unclassified key or something containing everything which is not grouped?

Should items be divided into functions, mixins and variables under each group? So you'd have something like data.{{ group }}.functions.{{ function }} or something?

Member

HugoGiraudel commented Jul 3, 2014

What should happen with items without the @group annotation? Shall we have an unclassified key or something containing everything which is not grouped?

Should items be divided into functions, mixins and variables under each group? So you'd have something like data.{{ group }}.functions.{{ function }} or something?

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jul 3, 2014

Member

What should happen with items without the @group annotation? Shall we have an unclassified key or something containing everything which is not grouped?

If the visual separation of groups is clear enough, maybe the ungrouped stuff could just come next (bottom).

Should items be divided into functions, mixins and variables under each group? So you'd have something like data.{{ group }}.functions.{{ function }} or something?

Yeah, that would be great I guess.
Even better would be a way to manually order items...
Right now what's implying the order ? Looks like it's alphabetical.

Member

pascalduez commented Jul 3, 2014

What should happen with items without the @group annotation? Shall we have an unclassified key or something containing everything which is not grouped?

If the visual separation of groups is clear enough, maybe the ungrouped stuff could just come next (bottom).

Should items be divided into functions, mixins and variables under each group? So you'd have something like data.{{ group }}.functions.{{ function }} or something?

Yeah, that would be great I guess.
Even better would be a way to manually order items...
Right now what's implying the order ? Looks like it's alphabetical.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 3, 2014

Member

Groups (functions, mixins and variables), then alphabetical.

Member

HugoGiraudel commented Jul 3, 2014

Groups (functions, mixins and variables), then alphabetical.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 3, 2014

Member

@valeriangalliat and I have started talking about it. More about the topic tomorrow.

Member

HugoGiraudel commented Jul 3, 2014

@valeriangalliat and I have started talking about it. More about the topic tomorrow.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 5, 2014

Member

Once we get 1.0.0 out of the box, this will be the next thing on the list. I need to talk with @valeriangalliat and @FWeinb about the best way to tackle this issue. I am not quite sure at the moment.

Member

HugoGiraudel commented Jul 5, 2014

Once we get 1.0.0 out of the box, this will be the next thing on the list. I need to talk with @valeriangalliat and @FWeinb about the best way to tackle this issue. I am not quite sure at the moment.

@HugoGiraudel HugoGiraudel added this to the 1.1 milestone Jul 5, 2014

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 5, 2014

Member

I like this: data.{{ group }}.functions.{{ function }}.

So we would get:

A
 |---> variable
   |---> a
   |---> b
 |---> function
 |---> mixin
B
 |---> variable
   |---> a
   |---> b
 |---> function
 |---> mixin
Member

FWeinb commented Jul 5, 2014

I like this: data.{{ group }}.functions.{{ function }}.

So we would get:

A
 |---> variable
   |---> a
   |---> b
 |---> function
 |---> mixin
B
 |---> variable
   |---> a
   |---> b
 |---> function
 |---> mixin
@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 5, 2014

Member

I think it makes sense.

Member

HugoGiraudel commented Jul 5, 2014

I think it makes sense.

@HugoGiraudel HugoGiraudel modified the milestones: 1.2, 1.1 Jul 7, 2014

@HugoGiraudel HugoGiraudel added 1.2 and removed 1.1 labels Jul 7, 2014

@HugoGiraudel HugoGiraudel referenced this issue Jul 9, 2014

Closed

Order items #71

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 9, 2014

Member

Related: define how we should order each level (groups, types, items).

Member

HugoGiraudel commented Jul 9, 2014

Related: define how we should order each level (groups, types, items).

@alademann

This comment has been minimized.

Show comment
Hide comment
@alademann

alademann Jul 10, 2014

Contributor

Instead of @group - should we use the @module annotation that JSDoc uses already?

This would work well with the documenting of the relationships between components in a BEM CSS implementation (e.g. #18)

Contributor

alademann commented Jul 10, 2014

Instead of @group - should we use the @module annotation that JSDoc uses already?

This would work well with the documenting of the relationships between components in a BEM CSS implementation (e.g. #18)

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 10, 2014

Member

On the other side, this might conflict with the BEM implementation.

Member

HugoGiraudel commented Jul 10, 2014

On the other side, this might conflict with the BEM implementation.

@HugoGiraudel HugoGiraudel added 1.3 and removed 1.2 labels Jul 19, 2014

@HugoGiraudel HugoGiraudel modified the milestones: 1.3, 1.2 Jul 19, 2014

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 19, 2014

Member

Moving this to 1.3.

Member

HugoGiraudel commented Jul 19, 2014

Moving this to 1.3.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 19, 2014

Member

@SassDoc/owners

Let's sum up:

  • We allow a @group annotation
  • The annotation should allow a string as only parameter
  • Items are grouped according to their @group (as a safe measure, we could lower-case and trim all spaces from group name to make sure they match; that would avoid issues where Helpers and helpers are different groups)
  • Inside their group, they are grouped according to their type (function/mixin/variable)
  • Inside their type, they are ordered according to their file order

Remaining question is: what happens to items that do not have a @group annotation? Should we create a leftover (others? unclassified? anything?) group for this?

Member

HugoGiraudel commented Jul 19, 2014

@SassDoc/owners

Let's sum up:

  • We allow a @group annotation
  • The annotation should allow a string as only parameter
  • Items are grouped according to their @group (as a safe measure, we could lower-case and trim all spaces from group name to make sure they match; that would avoid issues where Helpers and helpers are different groups)
  • Inside their group, they are grouped according to their type (function/mixin/variable)
  • Inside their type, they are ordered according to their file order

Remaining question is: what happens to items that do not have a @group annotation? Should we create a leftover (others? unclassified? anything?) group for this?

@alademann

This comment has been minimized.

Show comment
Hide comment
@alademann

alademann Jul 20, 2014

Contributor

The only thing I would add is - what about parsing the group string for / and . characters to specify more than one level of grouping? I'm not sure if one level would be enough in a large scale project.

As far as the "leftover" items, I would say group them as "ungrouped", and make them appear last in the sort order after all other groups? 

Maybe provide a configuration option so that an alias could be provided for the "ungrouped" group - e.g. If someone wanted their ungrouped items to appear under the heading "Miscellaneous" instead of the default "Ungrouped"

Contributor

alademann commented Jul 20, 2014

The only thing I would add is - what about parsing the group string for / and . characters to specify more than one level of grouping? I'm not sure if one level would be enough in a large scale project.

As far as the "leftover" items, I would say group them as "ungrouped", and make them appear last in the sort order after all other groups? 

Maybe provide a configuration option so that an alias could be provided for the "ungrouped" group - e.g. If someone wanted their ungrouped items to appear under the heading "Miscellaneous" instead of the default "Ungrouped"

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 20, 2014

Member

+1 for nested groups.

The separator should be the same as for #61, so maybe add an helper somewhere splitNamespace that splits on ., :: and /; the src/annotations/annotation/require.js code would do obj.external = splitNamespace(obj.name).length > 1, so we keep the separator consistant and in one only place.

Member

valeriangalliat commented Jul 20, 2014

+1 for nested groups.

The separator should be the same as for #61, so maybe add an helper somewhere splitNamespace that splits on ., :: and /; the src/annotations/annotation/require.js code would do obj.external = splitNamespace(obj.name).length > 1, so we keep the separator consistant and in one only place.

@HugoGiraudel HugoGiraudel modified the milestones: 1.2, 1.3 Jul 20, 2014

@HugoGiraudel HugoGiraudel added 1.2 and removed 1.3 labels Jul 20, 2014

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 20, 2014

Member

We need to move this back to 1.2 because of #120. Basically, once we'll give the ability to people to build their own templates, we won't be able to change the way data is structured (unless we release a major version).

So, we need to take care of this in the same minor version as the templating engine.

As far as the "leftover" items, I would say group them as "ungrouped", and make them appear last in the sort order after all other groups?
Maybe provide a configuration option so that an alias could be provided for the "ungrouped" group - e.g. If someone wanted their ungrouped items to appear under the heading "Miscellaneous" instead of the default "Ungrouped"

I like that. I want this to happen.

The separator should be the same as for #61, so maybe add an helper somewhere splitNamespace that splits on ., :: and /; the src/annotations/annotation/require.js code would do obj.external = splitNamespace(obj.name).length > 1, so we keep the separator consistant and in one only place.

Done.

Member

HugoGiraudel commented Jul 20, 2014

We need to move this back to 1.2 because of #120. Basically, once we'll give the ability to people to build their own templates, we won't be able to change the way data is structured (unless we release a major version).

So, we need to take care of this in the same minor version as the templating engine.

As far as the "leftover" items, I would say group them as "ungrouped", and make them appear last in the sort order after all other groups?
Maybe provide a configuration option so that an alias could be provided for the "ungrouped" group - e.g. If someone wanted their ungrouped items to appear under the heading "Miscellaneous" instead of the default "Ungrouped"

I like that. I want this to happen.

The separator should be the same as for #61, so maybe add an helper somewhere splitNamespace that splits on ., :: and /; the src/annotations/annotation/require.js code would do obj.external = splitNamespace(obj.name).length > 1, so we keep the separator consistant and in one only place.

Done.

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 21, 2014

Member

Sub-groups is another question. If you can handle sub-groups within data, you can handle sub-groups in an index too.

How do you handle sub-groups? I thought of something like this:

var groups = ['a', 'a.b', 'a.b.c'];

var indexByGroupDeep = {
  a: {
    b: {
      c: [flat[x], flat[x]],
    }

    b: [flat[x], flat[x]],
  }

  a: [flat[x], flat[x]],
};

While this is not valid JavaScript, this could be achieved like this (but I don't really like the solution):

var indexByGroupsDeep = {};

indexByGroupsDeep.a = [flat[x], flat[x]];
indexByGroupsDeep.a.b = [flat[x], flat[x]];
indexByGroupsDeep.a.b.c = [flat[x], flat[x]];
Member

valeriangalliat commented Jul 21, 2014

Sub-groups is another question. If you can handle sub-groups within data, you can handle sub-groups in an index too.

How do you handle sub-groups? I thought of something like this:

var groups = ['a', 'a.b', 'a.b.c'];

var indexByGroupDeep = {
  a: {
    b: {
      c: [flat[x], flat[x]],
    }

    b: [flat[x], flat[x]],
  }

  a: [flat[x], flat[x]],
};

While this is not valid JavaScript, this could be achieved like this (but I don't really like the solution):

var indexByGroupsDeep = {};

indexByGroupsDeep.a = [flat[x], flat[x]];
indexByGroupsDeep.a.b = [flat[x], flat[x]];
indexByGroupsDeep.a.b.c = [flat[x], flat[x]];
@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 21, 2014

Member

I'd like @FWeinb's opinion on all this. And @pascalduez's too.

Member

HugoGiraudel commented Jul 21, 2014

I'd like @FWeinb's opinion on all this. And @pascalduez's too.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 21, 2014

Member

As far as I am concerned, I would like to avoid nested groups for now. Let's keep things simple with one level of groups only.

Nested groups will ship in 2.0.0.

Member

HugoGiraudel commented Jul 21, 2014

As far as I am concerned, I would like to avoid nested groups for now. Let's keep things simple with one level of groups only.

Nested groups will ship in 2.0.0.

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 21, 2014

Member

+1 @HugoGiraudel, we should keep things simple for now, so we don't block this 1.2, then we'll take time to think of a simple and clean solution for this nested groups question for 2.0.

Member

valeriangalliat commented Jul 21, 2014

+1 @HugoGiraudel, we should keep things simple for now, so we don't block this 1.2, then we'll take time to think of a simple and clean solution for this nested groups question for 2.0.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 21, 2014

Member

@valeriangalliat +1 would allow the view to mix and match elements and create combinations on there own. We could even change the way we handle @requires and let the view handle all that.

I would suggest that we have the data indexed by type and name like this:

{
 data : {
  'function' : {
   'name' : {
      'key' : 'value' 
    }
  }
 }
}

And a object called indexByGroup:

{
  'goupA' : {
     '[type]' : [
        'name'
     ]
  },
  'groupB' : {
     '[type]' : [
        'name'
     ]
  }
}

The good thing here is that we don't have duplicate items in the data. The view than just needs to map [type] and name to the appropriate object.

Member

FWeinb commented Jul 21, 2014

@valeriangalliat +1 would allow the view to mix and match elements and create combinations on there own. We could even change the way we handle @requires and let the view handle all that.

I would suggest that we have the data indexed by type and name like this:

{
 data : {
  'function' : {
   'name' : {
      'key' : 'value' 
    }
  }
 }
}

And a object called indexByGroup:

{
  'goupA' : {
     '[type]' : [
        'name'
     ]
  },
  'groupB' : {
     '[type]' : [
        'name'
     ]
  }
}

The good thing here is that we don't have duplicate items in the data. The view than just needs to map [type] and name to the appropriate object.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jul 21, 2014

Member

Let's go simple for a start. One level.

Nested groups might be an interesting addition but as far as I'm concerned, not required.
I understand there might be huge project in needs of more complex folder structure, but I would try to avoid it myself as far as I can.

As a side note nested @import can really slow down your Sass.

Member

pascalduez commented Jul 21, 2014

Let's go simple for a start. One level.

Nested groups might be an interesting addition but as far as I'm concerned, not required.
I understand there might be huge project in needs of more complex folder structure, but I would try to avoid it myself as far as I can.

As a side note nested @import can really slow down your Sass.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 21, 2014

Member

@valeriangalliat I implemented it like you suggested: https://github.com/SassDoc/sassdoc/tree/add-group-annotation

(You need to run npm install to get lodash)

We now have these "views" of the data:

{
        flat         
        indexByTypeName /* where each key is like [type]_[name] */
        indexByType 
        indexByGroup 
        groups
}

See:

sassdoc/src/file.js

Lines 310 to 316 in 0934903

return {
flat : flat,
indexByTypeName : indexByTypeName,
indexByType : indexByType,
indexByGroup : indexByGroup,
groups : groups
};

Member

FWeinb commented Jul 21, 2014

@valeriangalliat I implemented it like you suggested: https://github.com/SassDoc/sassdoc/tree/add-group-annotation

(You need to run npm install to get lodash)

We now have these "views" of the data:

{
        flat         
        indexByTypeName /* where each key is like [type]_[name] */
        indexByType 
        indexByGroup 
        groups
}

See:

sassdoc/src/file.js

Lines 310 to 316 in 0934903

return {
flat : flat,
indexByTypeName : indexByTypeName,
indexByType : indexByType,
indexByGroup : indexByGroup,
groups : groups
};

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 21, 2014

Member

Hey, it's looking pretty nice! Any chance we could have an option to configure the name of the ungrouped group? One might want to call it extras or miscellaneous or whatever.

Otherwise, we're getting somewhere. Do you plan on tweaking your code Fabrice or can we merge it into develop? Once it is, what's the next step? Tweaking the theme to reflect the change I suppose?

Extras notes:

  • What happens if there are 2 @group annotations on the same item? I'd say only the first gets parsed, others get ignored.
  • You left a console.log in the code (src/api.js:33).
  • In src/utils.js:17, you could use utils.isset() to check for obj. Same line 20.
  • I'm not sure of the point of the setValueAtPath function. Would you mind explaining?
Member

HugoGiraudel commented Jul 21, 2014

Hey, it's looking pretty nice! Any chance we could have an option to configure the name of the ungrouped group? One might want to call it extras or miscellaneous or whatever.

Otherwise, we're getting somewhere. Do you plan on tweaking your code Fabrice or can we merge it into develop? Once it is, what's the next step? Tweaking the theme to reflect the change I suppose?

Extras notes:

  • What happens if there are 2 @group annotations on the same item? I'd say only the first gets parsed, others get ignored.
  • You left a console.log in the code (src/api.js:33).
  • In src/utils.js:17, you could use utils.isset() to check for obj. Same line 20.
  • I'm not sure of the point of the setValueAtPath function. Would you mind explaining?
@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 21, 2014

Member

I will refactor some code. To create a better structure.

Am 21.07.2014 um 22:49 schrieb Hugo Giraudel notifications@github.com:

Hey, it's looking pretty nice! Any chance we could have an option to configure the name of the ungrouped group? One might want to call it extras or miscellaneous or whatever.

Otherwise, we're getting somewhere. Do you plan on tweaking your code Fabrice or can we merge it into develop? Once it is, what's the next step? Tweaking the theme to reflect the change I suppose?


Reply to this email directly or view it on GitHub.

Member

FWeinb commented Jul 21, 2014

I will refactor some code. To create a better structure.

Am 21.07.2014 um 22:49 schrieb Hugo Giraudel notifications@github.com:

Hey, it's looking pretty nice! Any chance we could have an option to configure the name of the ungrouped group? One might want to call it extras or miscellaneous or whatever.

Otherwise, we're getting somewhere. Do you plan on tweaking your code Fabrice or can we merge it into develop? Once it is, what's the next step? Tweaking the theme to reflect the change I suppose?


Reply to this email directly or view it on GitHub.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 22, 2014

Member

Regarding your notes:

  • There can only be one group level. I am just returning an array to be future prove (see file.js#L303)
  • I am aware of that, left them for testing
  • will be removed was part of another solution

How many views do we want to expose?
/cc @HugoGiraudel, @valeriangalliat @pascalduez

Member

FWeinb commented Jul 22, 2014

Regarding your notes:

  • There can only be one group level. I am just returning an array to be future prove (see file.js#L303)
  • I am aware of that, left them for testing
  • will be removed was part of another solution

How many views do we want to expose?
/cc @HugoGiraudel, @valeriangalliat @pascalduez

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Jul 22, 2014

Member

How many views do we want to expose?

What do you mean by views ?

Member

pascalduez commented Jul 22, 2014

How many views do we want to expose?

What do you mean by views ?

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 22, 2014

Member

The different ways we sort the data and export it to the template:

{
        flat         
        indexByTypeName /* where each key is like [type]_[name] */
        indexByType 
        indexByGroup 
        groups
}
Member

FWeinb commented Jul 22, 2014

The different ways we sort the data and export it to the template:

{
        flat         
        indexByTypeName /* where each key is like [type]_[name] */
        indexByType 
        indexByGroup 
        groups
}
@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 23, 2014

Member

I talked with @HugoGiraudel and we discussed the following structure:

{
  groups : ['group1', 'group2', 'group3'], // this is sorted
  byType : {
    'function' : {

    }
    ...
  },
  byGroupAndType : {
    'group2' : {
      Like `byType`
    },
    'group3' : {
      Like `byType`
    },
    'group1' : {
      Like `byType`
    }
  }
}

In a swig template this could be evaluated like this:

{% for group in data.groups %}
  {% set _group = data.byGroupAndType[group] %} 
  {% for type in _group %}
    {% for item in _group[type] %}
     ...
    {% endfor %}
  {% endfor %}
{% endfor %}
Member

FWeinb commented Jul 23, 2014

I talked with @HugoGiraudel and we discussed the following structure:

{
  groups : ['group1', 'group2', 'group3'], // this is sorted
  byType : {
    'function' : {

    }
    ...
  },
  byGroupAndType : {
    'group2' : {
      Like `byType`
    },
    'group3' : {
      Like `byType`
    },
    'group1' : {
      Like `byType`
    }
  }
}

In a swig template this could be evaluated like this:

{% for group in data.groups %}
  {% set _group = data.byGroupAndType[group] %} 
  {% for type in _group %}
    {% for item in _group[type] %}
     ...
    {% endfor %}
  {% endfor %}
{% endfor %}
@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 23, 2014

Member

I absolutely agree.

So we stil have a flat array, and byType.function[n] is the same refernce as matching byGroupAndType.group2.function[n] in the flat array?

Member

valeriangalliat commented Jul 23, 2014

I absolutely agree.

So we stil have a flat array, and byType.function[n] is the same refernce as matching byGroupAndType.group2.function[n] in the flat array?

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 23, 2014

Member

Right

Member

FWeinb commented Jul 23, 2014

Right

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 23, 2014

Member

I double absolutely agree then. 👍

Member

valeriangalliat commented Jul 23, 2014

I double absolutely agree then. 👍

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 24, 2014

Member

Something's not working correctly @FWeinb, hit me when you have a moment.

Member

HugoGiraudel commented Jul 24, 2014

Something's not working correctly @FWeinb, hit me when you have a moment.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 25, 2014

Member

@FWeinb

Current group key looks like this:

[ ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'] ]

... while it should look like this:

[ 'ungrouped' ]
Member

HugoGiraudel commented Jul 25, 2014

@FWeinb

Current group key looks like this:

[ ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'], ['ungrouped'] ]

... while it should look like this:

[ 'ungrouped' ]
@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
Member

HugoGiraudel commented Jul 25, 2014

I made a fix, but that's probably not ideal: https://github.com/SassDoc/sassdoc-theme-light/blob/master/index.js#L47.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 25, 2014

Member

One thing we should probably do for @group is storing them as objects with a raw key, and a safe key so that users can have weird group names.

/**
 * @group Weird Hacks and Tricks
 */

Would yield:

{
  raw: "Weird Hacks and Tricks",
  safe: "weirdhacksandtricks"
}

The reason behind this is that group names are used as anchors to sections in the view. Anchors cannot contain spaces for instance. Also, if a group name contains uppercase letters, we currently put it down to lowercase which is kind of annoying for the end user.

Thoughts?

Member

HugoGiraudel commented Jul 25, 2014

One thing we should probably do for @group is storing them as objects with a raw key, and a safe key so that users can have weird group names.

/**
 * @group Weird Hacks and Tricks
 */

Would yield:

{
  raw: "Weird Hacks and Tricks",
  safe: "weirdhacksandtricks"
}

The reason behind this is that group names are used as anchors to sections in the view. Anchors cannot contain spaces for instance. Also, if a group name contains uppercase letters, we currently put it down to lowercase which is kind of annoying for the end user.

Thoughts?

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 25, 2014

Member

For the group name, I think it's better to use only a simple slug in the annotation; it's repeated all accross the codebase, so we should avoid longer names like Weird Hacks and Tricks, because it means if we want to rename the title, we need to update all the annotations.

To have a friendly title, we could make this configurable, for example in view.json:

{
  ...

  "groups": {
    "hacks": "Weird Hacks and Tricks"
  }
}

And in the documentation, only @group hacks is needed.

Member

valeriangalliat commented Jul 25, 2014

For the group name, I think it's better to use only a simple slug in the annotation; it's repeated all accross the codebase, so we should avoid longer names like Weird Hacks and Tricks, because it means if we want to rename the title, we need to update all the annotations.

To have a friendly title, we could make this configurable, for example in view.json:

{
  ...

  "groups": {
    "hacks": "Weird Hacks and Tricks"
  }
}

And in the documentation, only @group hacks is needed.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 26, 2014

Member

I think that both @HugoGiraudel and @valeriangalliat issues are responsibility of the theme. In the view.json of the theme author can define an alias feature for groups. Making a string work in an anchor tag is the responsibility of a theme.
Regarding the group key issue i will fix it.

Member

FWeinb commented Jul 26, 2014

I think that both @HugoGiraudel and @valeriangalliat issues are responsibility of the theme. In the view.json of the theme author can define an alias feature for groups. Making a string work in an anchor tag is the responsibility of a theme.
Regarding the group key issue i will fix it.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 26, 2014

Member

In the view.json of the theme author can define an alias feature for groups.

Should we open an issue or something?

Member

HugoGiraudel commented Jul 26, 2014

In the view.json of the theme author can define an alias feature for groups.

Should we open an issue or something?

FWeinb added a commit that referenced this issue Jul 26, 2014

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 26, 2014

Member

Can we close this maybe?

Member

HugoGiraudel commented Jul 26, 2014

Can we close this maybe?

@jsit

This comment has been minimized.

Show comment
Hide comment
@jsit

jsit Aug 27, 2014

Does this mean that the display order for groups is customizable? What's the view.json syntax for that?

jsit commented Aug 27, 2014

Does this mean that the display order for groups is customizable? What's the view.json syntax for that?

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Aug 27, 2014

Member

@jsit Nope, it's only to add a "friendly" name to the group slugs used in the comment blocks.

Member

valeriangalliat commented Aug 27, 2014

@jsit Nope, it's only to add a "friendly" name to the group slugs used in the comment blocks.

@jsit

This comment has been minimized.

Show comment
Hide comment
@jsit

jsit Aug 27, 2014

Thanks @valeriangalliat; is there an issue dealing with the sorting of groups right now or should I open a feature request?

jsit commented Aug 27, 2014

Thanks @valeriangalliat; is there an issue dealing with the sorting of groups right now or should I open a feature request?

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Aug 27, 2014

Member

Well I don't remember anyone evoked such a feature before, and I don't see any technical issue with the groups sorting, so I'd say you can open a feature request. :)

Member

valeriangalliat commented Aug 27, 2014

Well I don't remember anyone evoked such a feature before, and I don't see any technical issue with the groups sorting, so I'd say you can open a feature request. :)

@pascalduez

This comment has been minimized.

Show comment
Hide comment
Member

pascalduez commented Aug 27, 2014

@jsit

This comment has been minimized.

Show comment
Hide comment
@jsit

jsit Aug 27, 2014

@pascalduez: Aren't those issues about nesting groups, not necessarily ordering them?

jsit commented Aug 27, 2014

@pascalduez: Aren't those issues about nesting groups, not necessarily ordering them?

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Aug 27, 2014

Member

@pascalduez Both issues are closed, but do we have anything allowing to define a custom order for groups?

Member

valeriangalliat commented Aug 27, 2014

@pascalduez Both issues are closed, but do we have anything allowing to define a custom order for groups?

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Aug 27, 2014

Member

#135 is still open and active. The ordering of groups is mentioned.

There's no order capabilities at the moment, although it's been evoked since the beginning.

Member

pascalduez commented Aug 27, 2014

#135 is still open and active. The ordering of groups is mentioned.

There's no order capabilities at the moment, although it's been evoked since the beginning.

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Aug 27, 2014

Member

You're right, I saw a lot of "closed" flags in the bottom of the page but these were not about the same issue.

Member

valeriangalliat commented Aug 27, 2014

You're right, I saw a lot of "closed" flags in the bottom of the page but these were not about the same issue.

@jsit

This comment has been minimized.

Show comment
Hide comment
@jsit

jsit Aug 27, 2014

@pascalduez Since #135 appears to be (at least from its title, despite any comments) exclusively about group nesting, and since group nesting and group sorting could be pursued independently, should group sorting be its own issue?

jsit commented Aug 27, 2014

@pascalduez Since #135 appears to be (at least from its title, despite any comments) exclusively about group nesting, and since group nesting and group sorting could be pursued independently, should group sorting be its own issue?

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Aug 27, 2014

Member

should group sorting be its own issue

I guess having 2 issues is a bit dependent on how we might ends up implement this (or not).
Let's ping @HugoGiraudel here :)

Member

pascalduez commented Aug 27, 2014

should group sorting be its own issue

I guess having 2 issues is a bit dependent on how we might ends up implement this (or not).
Let's ping @HugoGiraudel here :)

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