FIX: SASS integration is broken - removed old parameter "--cache-location"

[maxcuttins]

fix the bug https://netbeans.org/bugzilla/show_bug.cgi?id=271126

[peedeeboy]

I did some quick testing today. Using the latest version of dart-sass, if "Generate sourcemaps" option is selected in the NB CSS Preprocessor options, an error still occurs because --debug-info is appended to the executable.

I suspect that this is because the version check method isn't working correctly either - because this class was designed for Ruby SASS, not libsass implementations.

I did some digging in the source - this SassExecutable.java class was for the original Ruby implementation of SASS. All modern implementations of SASS appear based on libsass. There is already a class for consuming libsass based implementations in Netbeans. LibSassExecutable.java

As an alternative, I would propose:

1. Leave SassExecutable.java as is for backwards compatibility with Ruby SASS
2. Add the missing functionality to LibSassExecutable.java (checking the Options for generating sourcemaps, check the project options for additional parameters etc.)
3. Amend SassCssPreprocessor.java to make LibSassExecutable the default implementation OR bonus points for adding the ability to select which implementation to use and amending SassCssPreprocessor.java to pick up this user choice

Hope that helps!

[maxcuttins]

As far as I know remove those paramters won't hurt new Ruby SASS implementation.
While leaving it in place just cut off all non Ruby implementations.
I guess it's better to promote NetBean10 with latest SASS version instead of support very very old version.
I found requrest to fix this issue since NetBean8.1.

PS: I see that you right, we should also remove --debug-info, which I miss

[matthiasblaesing]

@peedeeboy does the updated changeset make sense to you? If it improves the situation and does not introduce regressions, we should IMHO merge it.

[peedeeboy]

@maxcuttins Thank you so much for working on this, I've seen this brought up a couple of times so I'm sure quite a few people will be happy to see SASS support working again :) Apologies I haven't got around to testing until now!

I've tested on Windows with the dart-sass batch file, and with the .js implementation running through node. Both worked ok including adding additional parameters via the Project properties.

@matthiasblaesing Yes - this definitely makes things better and I think you are right to merge. I was a bit apprehensive as I saw in the old bugzilla that these parameters had already been removed in the past, then re-added later (probably for the reasons I mention - there is an incomplete class specifically for libsass implementations). See here: https://netbeans.org/bugzilla/show_bug.cgi?id=271126

There is still a bit of work to do in the long run, as the SassExecutable class was designed for Ruby SASS, some of the other methods such as checking version won't get the results they expect.

The only downside to this that I found in testing is that the Generate Sourcemaps flag in NB Options won't have any effect (due to the logic in the version check above not working with as expected with libsass implementations).

However, this can be worked around by adding the --no-source-map parameter in the Project Properties, so I think it is probably more important to get @maxcuttins fix in for NB11.1 and we can open a new Jira ticket for the tidy up work?

[matthiasblaesing]

I wanted to merge, but while testing I observed two things:

1. @maxcuttins please ensure that your author information in git is valid. Your author email ends with "@users.noreply.github.com"
2. This changeset introduces a regression for ruby sass users. Without the "--cache-location" parameter, the cache is placed into the public_html directory. That way it pollutes the work space of the user.

What do you think about this idea: use the "--help" call to see which options are supported. If an option is listed, it can be used (this assumes, that they are compatible).

[peedeeboy]

Hi @maxcuttins ! Have you had chance to look at Matthias' feedback? Would you like any help? And if you are too busy to look at this further, would you mind if I did? Cheers!

[maxcuttins]

I wanted to merge, but while testing I observed two things:

1. @maxcuttins please ensure that your author information in git is valid. Your author email ends with "@users.noreply.github.com"

I guess it's overwritten by GitHub.
Any Idea how to avoid this behavior?
(Maybe this was due that I just fix the code directly from the Web GUI instead of clone the entire repo). If somebody can help me avoid this I'll place my email without problems.

2. This changeset introduces a regression for ruby sass users. Without the "--cache-location" parameter, the cache is placed into the `public_html` directory. That way it pollutes the work space of the user.

This is right but should be left as it is. 1) the Ruby version is supported but deprecated and should not be used anymore. 2) if you still want to use the Ruby version, you can always add all the arguments that you need on the command line in the Project Properties in order to get the behaviour you want.

Everybody use the command line option to add --style=compressed.
If Ruby folks have to add also --cache-location on old version of SASS I think is quite acceptable and not so a bad deal instead to have a broken compilar for anybody else.

I think we should avoid to hard-code old/deprecated/distribution-specific arguments that will break any other version of SASS.

What do you think about this idea: use the "--help" call to see which options are supported. If an option is listed, it can be used (this assumes, that they are compatible).

Are you talking about cast everytime --help to SASS, parse the output, get the options, strip the descriptions, check the arguments and create a whitelist of arguments to use? If I get this right (maybe not) this is tons of hours of work in order to obtain a fragile parser which is subject to brake up everytime the format of the help change. As far as I can see SASS changes a lot its formats, it come from snake_style arguments to camelStyle arguments. So I'll not going to put any effort to try to parse something that is written from a team of people that changes their mind so rapidly and make so much code break changes. Of course it's just my opinion

[matthiasblaesing]

Technical issue

I had another look in the code and @peedeeboy already gave the right hint. NetBeans allows switching from the original sass implementation to libsass based implementations via a system property nb.sass.libsass. See here:

https://github.com/apache/netbeans/blob/master/ide/css.prep/src/org/netbeans/modules/css/prep/sass/SassCli.java#L52-L56

https://github.com/apache/netbeans/blob/master/ide/css.prep/src/org/netbeans/modules/css/prep/sass/LibSassExecutable.java

Testing can be done:

• checkout the master branch
• build it with ant
• run ant tryme -Dtryme.arg.1="-J-Dnb.sass.libsass=true"

Or with any recent netbeans installation:

• run <PATH_TO_NETBEANS>/bin/netbeans -J-Dnb.sass.libsass=true

With that, sassc is correctly found as the sass compiler and running it works as expected.

Having said this, why not just follow @peedeeboy suggestion (third bullet point):

private static final boolean USE_LIBSASS = Boolean.parseBoolean(System.getProperty("nb.sass.libsass", "true")); // NOI18N

The above code updated in SassCli, line 56 will switch the default from ruby sass to libsass. Anyone requiring ruby sass, can run with nb.sass.libsass=false. This needs to be tested of course.

Username issue

1. @maxcuttins please ensure that your author information in git is valid. Your author email ends with "@users.noreply.github.com"

I guess it's overwritten by GitHub.
Any Idea how to avoid this behavior?
(Maybe this was due that I just fix the code directly from the Web GUI instead of clone the entire repo). If somebody can help me avoid this I'll place my email without problems.

For your concrete problem: You most probably set the option "Keep my email addresses private` in you Settings (Personal settings -> Emails).

IMHO this is not a valid way to contribute. It can be argued, that the change is small enough to be doable from a WebGUI, you obvisously did not run UnitTests for the changeset.

[maxcuttins]

Technical issue

I had another look in the code and @peedeeboy already gave the right hint. NetBeans allows switching from the original sass implementation to libsass based implementations via a system property nb.sass.libsass. with any recent netbeans installation:

* run `<PATH_TO_NETBEANS>/bin/netbeans -J-Dnb.sass.libsass=true`

Maybei'm in wrong (probably) but I don't think that this is can be set by GUI interface.

private static final boolean USE_LIBSASS = Boolean.parseBoolean(System.getProperty("nb.sass.libsass", "true")); // NOI18N

I didn't know that there was already a property available in order to switch the type of SASS.

The above code updated in SassCli, line 56 will switch the default from ruby sass to libsass. Anyone requiring ruby sass, can run with nb.sass.libsass=false. This needs to be tested of course.

By having already a property, this is defintly the best way to go.
However, we have to check if the property can be switched easily.

For your concrete problem: You most probably set the option "Keep my email addresses private` in you Settings (Personal settings -> Emails).

Thanks! Checked just yesterday after my post.
It was exactly how you write.

IMHO this is not a valid way to contribute. It can be argued, that the change is small enough to be doable from a WebGUI, you obvisously did not run UnitTests for the changeset.

I know and I agree with you, exactly as you wrote the argued is this fix was just about remove an argument --command from the CLI command of SASS. This was doable from any kind of editor and without tests.

[peedeeboy]

Hi @maxcuttins :)

Maybei'm in wrong (probably) but I don't think that this is can be set by GUI interface.

However, we have to check if the property can be switched easily.
It can be set by editing the /etc/netbeans.conf file in a Netbeans installation. There are already a few parameters that are set this way.

If I understand @matthiasblaesing correctly, he is suggesting that as a quick fix, we flip the default value so that libsass becomes the default implementation instead of RubySASS (as 99% of people will be using libsass based implementations of sass nowadays).

Any legacy RubySASS users can add a parameter to their netbeans.conf file, until an option is added to the Netbeans GUI CSS Preprocessor options.

To implement that, you will need to:

1. ide/css.prep/src/org/netbeans/modules/css/prep/sass/SassExecutable.java
Revert the changes you made, to restore functionality to RubySASS implementations.

2. ide/css.prep/src/org/netbeans/modules/css/prep/sass/SassCli.java
Make libsass the default implementation by making @matthiasblaesing recommended change, changing line 56 from:

private static final boolean USE_LIBSASS Boolean.getBoolean("nb.sass.libsass", ); // NOI18N`

to:

private static final boolean USE_LIBSASS = Boolean.parseBoolean(System.getProperty("nb.sass.libsass", "true")); // NOI18N

This will make libsass flag default to true unless Netbeans is started with the -J-Dnb.sass.libsass=true flag

3. ide/css.prep/src/org/netbeans/modules/css/prep/sass/LibSassExecutable.java
Make libsass work with the 'Generate Source Maps' option in the Netbeans CSS Preprocessor options by:
Adding the following two lines as member variables (around line 35):

private static final String SOURCEMAP_PARAM = "--source-map"; // NOI18N
private static final String NO_SOURCEMAP_PARAM = "--no-source-map"; // NOI18N

Add the following lines into the getParameters() method:

// sourcemaps
boolean debug = CssPrepOptions.getInstance().getSassDebug();
if(debug) {
    params.add(SOURCEMAP_PARAM);
}
else {
    params.add(NO_SOURCEMAP_PARAM);
}

Those lines should come after:

List<String> params = new ArrayList<>();

and before:

// compiler options
        params.addAll(compilerOptions);

Shout if you get stuck! 👍

[matthiasblaesing]

The situation is a bit more complex as each tool should have its own options (insert sarcastic comment here), sassc and node-sass do not share the same CLI options.

• sassc has: -m, --sourcemap[=TYPE] Emit source map (auto or inline).
• node-sass has: --source-map Emit source map

So while both are based on libsass both are incompatible. The libsass implementation worked around this by not handling the debug setting. So we would still need to check the binary to get the right option.

[peedeeboy]

Thanks @matthiasblaesing
Ok, @maxcuttins - disregard my point 3. apologies for causing confusion.

Perhaps the best thing to do for libsass implementations is rely on the command line options being set in the project properties.

Regarding the Netbeans CSS PreProcessor options, either:
a) Amend the "Generate Source Maps" checkbox label to "Generate Source Maps (legacy RubySass only)"
b) Conditionally hide the "Generate Source Maps" checkbox and label based on the value of nb.sass.libsass

The Full path of sass executable (usually x or y) in Netbeans options already changes value depending on the value of nb.sass.libsass, so B would be consistent with that?