Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "debug" option in library #145

Closed
XhmikosR opened this issue Sep 7, 2013 · 14 comments
Closed

Add "debug" option in library #145

XhmikosR opened this issue Sep 7, 2013 · 14 comments
Assignees
Milestone

Comments

@XhmikosR
Copy link
Contributor

XhmikosR commented Sep 7, 2013

Example:

var packCss = cleanCSS.process(inCss, {
    removeEmpty: true,
    keepSpecialComments: 0,
    debug: true
});
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Nov 4, 2013

Thanks!

PS. Do you think the tests should run with debug enabled so that we see the stats and stuff?

@GoalSmashers
Copy link
Contributor

I don't think it's necessary as verifying minified output is what matters the most.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Nov 4, 2013

Ok.

I was trying the new option but I don't see anything on the cmd window

Using

echo("### Combining css files...");

// pack.css
var inCss = cat([
        "_static/css/bootstrap.css",
        "_static/css/font-awesome.css",
        "_static/css/jquery.fancybox.css",
        "_static/css/jquery.fancybox-thumbs.css",
        "_static/css/style.css"
    ]);

var minifiedCss = new CleanCSS({
        debug: true,
        keepSpecialComments: 0,
        selectorsMergeMode: "ie8"
    }).minify(inCss);

writeText(buildTarget + "_static/css/pack.css", minifiedCss);
### Combining css files...

### Combining js files...

### Build finished. The HTML pages are in C:\Users\xmr\Desktop\mpc-hc.org/build/
website/.

### Starting webserver...
Serving HTTP on 0.0.0.0 port 8000 ...

@GoalSmashers
Copy link
Contributor

That's because only CLI prints this output to stderr. It's up to you to decide what to do with it when using a library. You can access debug data from:

var minifier = new CleanCSS({
        debug: true,
        keepSpecialComments: 0,
        selectorsMergeMode: "ie8"
    });
var minifiedCss = minifier.minify(inCss);
// access minifier.stats to see the efficiency, etc

I think it should be up to one who uses the library to decide how / if to output that data.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Nov 4, 2013

I don't think that's right in this case. I mean I expected to see the stats by just using the debug option.

@GoalSmashers
Copy link
Contributor

What if someone wants to access minification stats but does not want to have it printed to stderr?

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Nov 4, 2013

I guess they can redirect stderr in that case. When one enables debug it's because they want to see the stats IMO.

Also,

### Combining css files...
{ originalSize: 105537,
  timeSpent: 39,
  efficiency: 0.22997621687180803,
  minifiedSize: 81266 }

This doesn't seem very user friendly. It has no units and those brackets.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Nov 4, 2013

Another thing, I think a warning or something should be shown when debug isn't true and one accesses stats. Or at least don't show the brackets at all in that case.

@GoalSmashers
Copy link
Contributor

I don't agree. After all it's a library access so people expect raw data and should know how to deal with it. It's the CLI where nice output matters.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Nov 4, 2013

Well how about the brackets at least? They show up even when debug is false and one accesses stats.

@GoalSmashers
Copy link
Contributor

Would you prefer an empty string?

@GoalSmashers
Copy link
Contributor

I meant when debug is false. When you print it out it will always be there because stats is a hash.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Nov 4, 2013

Not sure if an empty string is better. Maybe leave it like this and wait for feedback from other people too.

@GoalSmashers
Copy link
Contributor

Ok, let's wait and see!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants