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

JS Remove Comments setting is missing #189

Open
PrzemyslawKlys opened this issue Nov 9, 2020 · 11 comments · May be fixed by #228
Open

JS Remove Comments setting is missing #189

PrzemyslawKlys opened this issue Nov 9, 2020 · 11 comments · May be fixed by #228
Assignees

Comments

@PrzemyslawKlys
Copy link

There is a setting called RemoveComments for HTML, but for CSS/JS those are missing. Maybe it should be an option for those.

Although this brings special kinds of problems like it does for VSCode where it's not able to format this properly.

            h6 {
                font-family: 'Hammersmith One', sans-serif;
                /*
    font-family: 'Questrial', sans-serif;
    */
            }
@trullock
Copy link
Owner

trullock commented Nov 9, 2020

Because the current default functionality is to always remove them, right?

And you want the option to keep them?

@PrzemyslawKlys
Copy link
Author

PrzemyslawKlys commented Nov 9, 2020

Well, it's always removing it for CSS and JS, yet you give an option to keep HTML comments. Seems reasonable to me to have the option to keep them for JS and CSS as well. Not that I need them, just think for consistency's sake. Sometimes I do include some stuff that may have meaning to debug things, that may otherwise be removed.

@trullock
Copy link
Owner

trullock commented Nov 9, 2020

For CSS you can use CssSettings.CommentMode, but for JS theres only the option to keep important comments

@PrzemyslawKlys
Copy link
Author

I see, couldn't find that :-) What's an important comment?

@trullock
Copy link
Owner

trullock commented Nov 9, 2020

Comments that start with //! or /*!, you usually see them at the top of libs with the license info in it

Not an official syntax but commonly adopted

@trullock trullock changed the title JS/CSS Remove Comments setting is missing JS Remove Comments setting is missing Nov 11, 2020
@trullock
Copy link
Owner

This has uncovered a bug whereby prologue directives are broken by there being preceeding important comments in the block.

This needs fixing, might as well be along with this ticket as this ticket will otherwise only reintroduce the same issue with non-important comments

@trullock trullock added the bug label Jan 2, 2021
@trullock trullock self-assigned this Jan 2, 2021
trullock added a commit that referenced this issue Jan 4, 2021
trullock added a commit that referenced this issue Jan 4, 2021
@trullock trullock linked a pull request Jan 4, 2021 that will close this issue
@trullock
Copy link
Owner

trullock commented Jan 4, 2021

Added a PR for this, please have a play and see if this meets your needs

@PrzemyslawKlys
Copy link
Author

Just came back to this seeing as the PR hasn't been merged yet. Is there anything you want me to check, confirm?

@trullock
Copy link
Owner

Yeah, can you test the PR meets your needs?

@PrzemyslawKlys
Copy link
Author

This has uncovered a bug whereby prologue directives are broken by there being preceding important comments in the block.

This needs fixing, might as well be along with this ticket as this ticket will otherwise only reintroduce the same issue with non-important comments

I thought the PR is to fix unrelated issues no?

trullock added a commit that referenced this issue Mar 15, 2021
trullock added a commit that referenced this issue Mar 15, 2021
@trullock
Copy link
Owner

It fixes both, i hope!

trullock added a commit that referenced this issue Jan 14, 2022
trullock added a commit that referenced this issue Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants