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 whitespace option #14

Closed
andrey-semenoff opened this issue May 31, 2016 · 9 comments · Fixed by #21
Closed

Add whitespace option #14

andrey-semenoff opened this issue May 31, 2016 · 9 comments · Fixed by #21
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted

Comments

@andrey-semenoff
Copy link

andrey-semenoff commented May 31, 2016

Issuehunt badges

Hallo!
Your plugin works good, but there is one annoying fact - after removing comment block it left empty line, and it looks not very tidily.
I propose to add some options to plugin to have a choice (like in gulp-strip-comments):

  • options.space ⇒ Boolean
    false (default) - remove comment blocks entirely
    true - replace comment blocks with white spaces where needed, in order to preserve the original line + column position of every code element.
    NOTE: When this option is enabled, option trim is ignored.
  • options.trim ⇒ Boolean
    false (default) - do not trim comments
    true - remove empty lines that follow removed full-line comments
    NOTE: This option has no effect when option space is enabled.

or at least add only first one.

Thanks.


IssueHunt Summary

yaodingyd yaodingyd has been rewarded.

Backers (Total: $80.00)

Submitted pull Requests


Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@sindresorhus
Copy link
Owner

Yes, that sounds like a bug.

Why would you need to preserve the line/column in CSS? It makes sense in JS where you want stack traces to point to the correct source location, but you don't have that with CSS.

@andrey-semenoff
Copy link
Author

I don't need to preserve lines - it'll be enough if it just don't left empty lines in CSS.
Here is CSS when plugin is off:
strip-css-comments - off

And here plugin is on:
strip-css-comments - on

sindresorhus added a commit that referenced this issue May 31, 2016
@sindresorhus
Copy link
Owner

Alright. I added a failing test for this, but I don't have time to fix this right now, so PR welcome :)

@andrey-semenoff
Copy link
Author

Thank you. I'll try if I could :))

@lebbe
Copy link

lebbe commented Jun 29, 2016

"Why would you need to preserve the line/column in CSS? It makes sense in JS where you want stack traces to point to the correct source location, but you don't have that with CSS."

The developer tools in chrome use the sourcemap for pointing to file and line-number. This gets all wrong if comment-stripping remove lines. I tend to do css-editing in the browser devtools, and paste the result into the correct file, so this is actually a feature I would want.

@sindresorhus
Copy link
Owner

I think it would make sense to implement a whitespace option, like https://github.com/sindresorhus/strip-json-comments#whitespace

@sindresorhus
Copy link
Owner

sindresorhus commented Jun 29, 2019

For anyone that wants to work on this, see #18 for an initial attempt.


Note: This issue has a bounty, so it's expected that you are an experienced programmer and that you give it your best effort if you intend to tackle this. Don't forget, if applicable, to add tests, docs (double-check for typos), and update TypeScript definitions. And don't be sloppy. Review your own diff multiple times and try to find ways to improve and simplify your code. Instead of asking too many questions, present solutions. The point of an issue bounty is to reduce my workload, not give me more. Include a 🦄 in your PR description to indicate that you've read this. Thanks for helping out 🙌 - @sindresorhus

@sindresorhus sindresorhus changed the title Remove empty line Add whitespace option Jun 29, 2019
@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Jun 29, 2019
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jun 29, 2019

@issuehunt has funded $80.00 to this issue.


@issuehunt-oss issuehunt-oss bot removed the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Nov 16, 2019
@issuehunt-oss
Copy link

issuehunt-oss bot commented Nov 16, 2019

@sindresorhus has rewarded $72.00 to @yaodingyd. See it on IssueHunt

  • 💰 Total deposit: $80.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $8.00

@issuehunt-oss issuehunt-oss bot added the 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt label Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants