Be nice with @requires variable #74

Closed
HugoGiraudel opened this Issue Jul 8, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@HugoGiraudel
Member

HugoGiraudel commented Jul 8, 2014

I spent a couple of minutes earlier trying to figure what was going on.

Here is the code I was running:

/**
 * @requires {variable} $my-variable
 */

SassDoc kept telling me my-variable didn't exist while it was obviously wrong. This was because I left the $ sign. It would be nice if we could leave it. Most people will write the name with the dollar sign anyway.

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

@HugoGiraudel HugoGiraudel added the 1.0 label Jul 8, 2014

@HugoGiraudel HugoGiraudel modified the milestones: 1.0, 1.1 Jul 8, 2014

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 9, 2014

Member

Well if variables in Sass always take a $, I think this should be required in SassDoc too.

Member

valeriangalliat commented Jul 9, 2014

Well if variables in Sass always take a $, I think this should be required in SassDoc too.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 9, 2014

Member

Well, that is kind of inconsistent with functions and mixins in some way. Having both syntaxes would be best if you ask me.

Member

HugoGiraudel commented Jul 9, 2014

Well, that is kind of inconsistent with functions and mixins in some way. Having both syntaxes would be best if you ask me.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 9, 2014

Member

Technical, if the name is prefixed with a dollar sign, there is no need for {variable}.

Ideally:

/**
 * @requires {variable} $my-variable
 * @requires {variable} my-variable
 * @requires $my-variable
 */

... would be all valid.

What do you say?

Member

HugoGiraudel commented Jul 9, 2014

Technical, if the name is prefixed with a dollar sign, there is no need for {variable}.

Ideally:

/**
 * @requires {variable} $my-variable
 * @requires {variable} my-variable
 * @requires $my-variable
 */

... would be all valid.

What do you say?

@valeriangalliat

This comment has been minimized.

Show comment
Hide comment
@valeriangalliat

valeriangalliat Jul 9, 2014

Member

Well the $ can help to identify the type of the dependency too.

I'd prefer @require $my-variable which is obviously (even for the parser) a variable, than @require {variable} my-variable that requires me to give the type to avoid potential duplicates with a my-variable function or mixin.

Okay you just posted the same idea a few seconds before me :3, well I agree.

Member

valeriangalliat commented Jul 9, 2014

Well the $ can help to identify the type of the dependency too.

I'd prefer @require $my-variable which is obviously (even for the parser) a variable, than @require {variable} my-variable that requires me to give the type to avoid potential duplicates with a my-variable function or mixin.

Okay you just posted the same idea a few seconds before me :3, well I agree.

@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 9, 2014

Member

I will implement this.

Member

FWeinb commented Jul 9, 2014

I will implement this.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 9, 2014

Member

Thanks @FWeinb!

We'll ship this in rc-13 which will be, hopefully, the last release candidate.

Member

HugoGiraudel commented Jul 9, 2014

Thanks @FWeinb!

We'll ship this in rc-13 which will be, hopefully, the last release candidate.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Jul 9, 2014

Member

Recap:

  • In case of {variable}, $ is optional
  • If there is $, {variable} is optional
Member

HugoGiraudel commented Jul 9, 2014

Recap:

  • In case of {variable}, $ is optional
  • If there is $, {variable} is optional
@FWeinb

This comment has been minimized.

Show comment
Hide comment
@FWeinb

FWeinb Jul 9, 2014

Member

Recap:

  • Always omit the $ in the parsed data so { type : 'variable', name 'name' }
Member

FWeinb commented Jul 9, 2014

Recap:

  • Always omit the $ in the parsed data so { type : 'variable', name 'name' }

@FWeinb FWeinb closed this in fbc2ce5 Jul 9, 2014

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

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