Add ability to use Sass indented syntax #183

Closed
HugoGiraudel opened this Issue Aug 24, 2014 · 34 comments

Comments

Projects
None yet
4 participants
@HugoGiraudel
Member

HugoGiraudel commented Aug 24, 2014

Try the following:

  • Make some .sass files with SassDoc documentation
  • Convert .sass into .scss
  • Run SassDoc
  • Test if it works

@HugoGiraudel HugoGiraudel changed the title from A workaround for .sass to A workaround for Sass indented syntax Aug 24, 2014

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Aug 25, 2014

Member

First tests:

sass-convert is not reproducing/closing the Cdoc style comments properly, resulting in no docs.

See https://github.com/pascalduez/sassdoc-convert

The command used is npm test as declared here

Member

pascalduez commented Aug 25, 2014

First tests:

sass-convert is not reproducing/closing the Cdoc style comments properly, resulting in no docs.

See https://github.com/pascalduez/sassdoc-convert

The command used is npm test as declared here

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Aug 25, 2014

Member

Might be an issue with Sass. Maybe @nex3 could give us her opinion.

Member

HugoGiraudel commented Aug 25, 2014

Might be an issue with Sass. Maybe @nex3 could give us her opinion.

@nex3

This comment has been minimized.

Show comment
Hide comment
@nex3

nex3 Aug 25, 2014

Can you narrow this down to a single repro case and open an issue on sass/sass?

nex3 commented Aug 25, 2014

Can you narrow this down to a single repro case and open an issue on sass/sass?

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
Member

HugoGiraudel commented Aug 25, 2014

@nex3 There you go: sass/sass#1398.

@stevenleeg

This comment has been minimized.

Show comment
Hide comment
@stevenleeg

stevenleeg Aug 27, 2014

🍻

Super excited for .sass support. Let me know if there's any way I can help out via testing or whatnot.

🍻

Super excited for .sass support. Let me know if there's any way I can help out via testing or whatnot.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Aug 27, 2014

Member

Well, as long as there is this bug in Sass core, we are not going to move any forward.

Member

HugoGiraudel commented Aug 27, 2014

Well, as long as there is this bug in Sass core, we are not going to move any forward.

@stevenleeg

This comment has been minimized.

Show comment
Hide comment
@stevenleeg

stevenleeg Aug 28, 2014

just showed my support over on that ticket as well, hopefully that'll provide a small bit of help.

just showed my support over on that ticket as well, hopefully that'll provide a small bit of help.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Aug 28, 2014

Member

It looks like @nex3 had no issue with:

$ ./bin/sass-convert --version
Sass 3.4.1.09c8af1 (Selective Steve)
$ ./bin/sass-convert --from sass --to scss <<SASS
/*
 * Comment
 */
SASS

/* Comment
 */

Could you please give it a try @pascalduez?

Member

HugoGiraudel commented Aug 28, 2014

It looks like @nex3 had no issue with:

$ ./bin/sass-convert --version
Sass 3.4.1.09c8af1 (Selective Steve)
$ ./bin/sass-convert --from sass --to scss <<SASS
/*
 * Comment
 */
SASS

/* Comment
 */

Could you please give it a try @pascalduez?

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Aug 28, 2014

Member

Actually, Natalie seems to have found a way to reproduce it, so let's wait a bit.

Member

HugoGiraudel commented Aug 28, 2014

Actually, Natalie seems to have found a way to reproduce it, so let's wait a bit.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 7, 2014

Member

@nex3 Natalie, out of curiosity, do you have any idea when this slight issue is going to be fixed?

I bet it's not a priority since it's mostly about comments at this point, but knowing an approximate date would help us schedule the next releases. :)

Member

HugoGiraudel commented Sep 7, 2014

@nex3 Natalie, out of curiosity, do you have any idea when this slight issue is going to be fixed?

I bet it's not a priority since it's mostly about comments at this point, but knowing an approximate date would help us schedule the next releases. :)

@HugoGiraudel HugoGiraudel changed the title from A workaround for Sass indented syntax to Add ability to use Sass indented syntax Sep 8, 2014

@nex3

This comment has been minimized.

Show comment
Hide comment
@nex3

nex3 Sep 12, 2014

@HugoGiraudel It made it into 3.4.2, I just forgot to close the issue :p.

nex3 commented Sep 12, 2014

@HugoGiraudel It made it into 3.4.2, I just forgot to close the issue :p.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 12, 2014

Member

@nex3 Thanks a lot !

The comments are now properly closed in the resulting .scss.

However, the opening comment is not kept: /** is transformed in /* .
Which in the case of SassDoc will be a problem.

Is it intended ?

Member

pascalduez commented Sep 12, 2014

@nex3 Thanks a lot !

The comments are now properly closed in the resulting .scss.

However, the opening comment is not kept: /** is transformed in /* .
Which in the case of SassDoc will be a problem.

Is it intended ?

@nex3

This comment has been minimized.

Show comment
Hide comment
@nex3

nex3 Sep 12, 2014

File a separate bug for that. I'll see if I can make it work.

nex3 commented Sep 12, 2014

File a separate bug for that. I'll see if I can make it work.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
Member

HugoGiraudel commented Sep 12, 2014

@nex3 There you go: sass/sass#1432.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 13, 2014

Member

It works ! Starting Sass 3.4.4

.sass successfuly converted to .scss and SassDoc documentation compiled.
https://github.com/pascalduez/sassdoc-convert

Thanks @nex3 for the quick responses.

Member

pascalduez commented Sep 13, 2014

It works ! Starting Sass 3.4.4

.sass successfuly converted to .scss and SassDoc documentation compiled.
https://github.com/pascalduez/sassdoc-convert

Thanks @nex3 for the quick responses.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 13, 2014

Member

So the idea is to:

  1. Add an option to the API (--sass-syntax, --sass, --convert...);
  2. If option is being passed, convert the <src> folder to a temporary location (__sassdoc_tmp or anything);
  3. Run SassDoc on the temporary folder;
  4. Delete temporary folder.

@SassDoc/owners Does it seem okay to y'all?

I suppose we'll something like shelljs to run sass-convert.

Member

HugoGiraudel commented Sep 13, 2014

So the idea is to:

  1. Add an option to the API (--sass-syntax, --sass, --convert...);
  2. If option is being passed, convert the <src> folder to a temporary location (__sassdoc_tmp or anything);
  3. Run SassDoc on the temporary folder;
  4. Delete temporary folder.

@SassDoc/owners Does it seem okay to y'all?

I suppose we'll something like shelljs to run sass-convert.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 13, 2014

Member

I'm currently trying this.

Member

pascalduez commented Sep 13, 2014

I'm currently trying this.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 13, 2014

Member

I asked @hcatlin to know if sass-convert exists in LibSass. I'm not sure.

Member

HugoGiraudel commented Sep 13, 2014

I asked @hcatlin to know if sass-convert exists in LibSass. I'm not sure.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 13, 2014

Member

I'm having an issue trying to use the --recursive option of sass-convert.

tmp is a folder with some .sass files.

$ sass-convert -F sass -T scss -R tmp test

directory test
    convert tmp/indented.sass
     create test/indented.scss
Errno::EISDIR: Is a directory - tmp
  Use --trace for backtrace.

$ sass-convert -F sass -T scss -R -i tmp

convert tmp/indented.sass
  overwrite tmp/indented.sass
Errno::EISDIR: Is a directory - tmp
  Use --trace for backtrace.

No .scss file created.

Member

pascalduez commented Sep 13, 2014

I'm having an issue trying to use the --recursive option of sass-convert.

tmp is a folder with some .sass files.

$ sass-convert -F sass -T scss -R tmp test

directory test
    convert tmp/indented.sass
     create test/indented.scss
Errno::EISDIR: Is a directory - tmp
  Use --trace for backtrace.

$ sass-convert -F sass -T scss -R -i tmp

convert tmp/indented.sass
  overwrite tmp/indented.sass
Errno::EISDIR: Is a directory - tmp
  Use --trace for backtrace.

No .scss file created.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 13, 2014

Member
$ sass-convert -R -F scss -T sass node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils/ .tmp
  directory .tmp
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_functions.scss
     create .tmp/_functions.sass
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_helpers.scss
     create .tmp/_helpers.sass
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_mixins.scss
     create .tmp/_mixins.sass
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_variables.scss
     create .tmp/_variables.sass
$ sass-convert -R -F sass -T scss .tmp .tmp2
  directory .tmp2
    convert .tmp/_functions.sass
     create .tmp2/_functions.scss
    convert .tmp/_helpers.sass
     create .tmp2/_helpers.scss
    convert .tmp/_mixins.sass
     create .tmp2/_mixins.scss
    convert .tmp/_variables.sass
     create .tmp2/_variables.scss

Everything works here.

Member

HugoGiraudel commented Sep 13, 2014

$ sass-convert -R -F scss -T sass node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils/ .tmp
  directory .tmp
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_functions.scss
     create .tmp/_functions.sass
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_helpers.scss
     create .tmp/_helpers.sass
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_mixins.scss
     create .tmp/_mixins.sass
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_variables.scss
     create .tmp/_variables.sass
$ sass-convert -R -F sass -T scss .tmp .tmp2
  directory .tmp2
    convert .tmp/_functions.sass
     create .tmp2/_functions.scss
    convert .tmp/_helpers.sass
     create .tmp2/_helpers.scss
    convert .tmp/_mixins.sass
     create .tmp2/_mixins.scss
    convert .tmp/_variables.sass
     create .tmp2/_variables.scss

Everything works here.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 13, 2014

Member

Running that same command from sassdoc repo I get the error >_<

sass-convert -R -F scss -T sass node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils/ .tmp

directory .tmp
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_functions.scss
     create .tmp/_functions.sass
Errno::EISDIR: Is a directory - node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils/
  Use --trace for backtrace.

$ sass -v

Sass 3.4.4 (Selective Steve)

$ ruby -v

ruby 2.0.0p451 (2014-02-24 revision 45167) [universal.x86_64-darwin13]
Member

pascalduez commented Sep 13, 2014

Running that same command from sassdoc repo I get the error >_<

sass-convert -R -F scss -T sass node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils/ .tmp

directory .tmp
    convert node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils//_functions.scss
     create .tmp/_functions.sass
Errno::EISDIR: Is a directory - node_modules/sassdoc-theme-default/node_modules/sassdoc-theme-light/scss/utils/
  Use --trace for backtrace.

$ sass -v

Sass 3.4.4 (Selective Steve)

$ ruby -v

ruby 2.0.0p451 (2014-02-24 revision 45167) [universal.x86_64-darwin13]
@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 13, 2014

Member

The error happens starting with Sass 3.4.2 if I run 3.4.1 it's fine. cc @nex3

@HugoGiraudel Which version of Sass are you using ?

Member

pascalduez commented Sep 13, 2014

The error happens starting with Sass 3.4.2 if I run 3.4.1 it's fine. cc @nex3

@HugoGiraudel Which version of Sass are you using ?

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 13, 2014

Member

The first draft is here: https://github.com/pascalduez/sassdoc-convert/blob/master/index.js

Sadly it only works with Sass 3.4.1 which does not have the fix for comments converting...
So to test:
$ bundle install --path .
$ npm install
$ node index.js

To test the EISDIR error, change gem 'sass', '3.4.1' into gem 'sass', '3.4.2' in the Gemfile :
$ bundle install --path .
$ bundle exec sass-convert -R -F sass -T scss sass .tmp

Member

pascalduez commented Sep 13, 2014

The first draft is here: https://github.com/pascalduez/sassdoc-convert/blob/master/index.js

Sadly it only works with Sass 3.4.1 which does not have the fix for comments converting...
So to test:
$ bundle install --path .
$ npm install
$ node index.js

To test the EISDIR error, change gem 'sass', '3.4.1' into gem 'sass', '3.4.2' in the Gemfile :
$ bundle install --path .
$ bundle exec sass-convert -R -F sass -T scss sass .tmp

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 14, 2014

Member

@HugoGiraudel Which version of Sass are you using ?

3.3.4.

Member

HugoGiraudel commented Sep 14, 2014

@HugoGiraudel Which version of Sass are you using ?

3.3.4.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 14, 2014

Member

Did you ask @nex3 about the issue perhaps?

Member

HugoGiraudel commented Sep 14, 2014

Did you ask @nex3 about the issue perhaps?

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 14, 2014

Member

I can confirm there is a regression with sass-convert in Sass 3.4.4.

Member

HugoGiraudel commented Sep 14, 2014

I can confirm there is a regression with sass-convert in Sass 3.4.4.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 14, 2014

Member

An issue was created at sass/sass#1435

Member

pascalduez commented Sep 14, 2014

An issue was created at sass/sass#1435

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 14, 2014

Member

Perfect.

Member

HugoGiraudel commented Sep 14, 2014

Perfect.

@HugoGiraudel HugoGiraudel modified the milestone: 1.7 Sep 14, 2014

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 19, 2014

Member

Depending on how soon @nex3 is willing to fix this, we might fall back on converting all files individually with Node.

Member

HugoGiraudel commented Sep 19, 2014

Depending on how soon @nex3 is willing to fix this, we might fall back on converting all files individually with Node.

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 19, 2014

Member

Issue seems to have been fixed in 3.4.5: sass/sass@ff9fd27. Gem is not pushed yet, but it will probably soon be.

Once it's done, let's retry the whole thing and merge the PR.

Member

HugoGiraudel commented Sep 19, 2014

Issue seems to have been fixed in 3.4.5: sass/sass@ff9fd27. Gem is not pushed yet, but it will probably soon be.

Once it's done, let's retry the whole thing and merge the PR.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 19, 2014

Member

Will monitor this.

Member

pascalduez commented Sep 19, 2014

Will monitor this.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 20, 2014

Member

Sass 3.4.5 is published, thanks a lot to @nex3 for the quick fix an release :)

Should we make a check in the convert script on the version number and warn/abort if it's lower than 3.4.5 ?

Member

pascalduez commented Sep 20, 2014

Sass 3.4.5 is published, thanks a lot to @nex3 for the quick fix an release :)

Should we make a check in the convert script on the version number and warn/abort if it's lower than 3.4.5 ?

@HugoGiraudel

This comment has been minimized.

Show comment
Hide comment
@HugoGiraudel

HugoGiraudel Sep 20, 2014

Member

Yeah that sounds good.

Member

HugoGiraudel commented Sep 20, 2014

Yeah that sounds good.

@pascalduez

This comment has been minimized.

Show comment
Hide comment
@pascalduez

pascalduez Sep 20, 2014

Member

This will be available in SassDoc 1.7.0
See #205 for details.

Member

pascalduez commented Sep 20, 2014

This will be available in SassDoc 1.7.0
See #205 for details.

@pascalduez pascalduez closed this Sep 20, 2014

@pascalduez pascalduez referenced this issue in SassDoc/sass-convert Nov 13, 2015

Open

LibSass support #14

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