vitaly-t / decomment

:hammer: Removes comments from JSON, JavaScript, CSS, HTML, etc.
75 stars 21 forks source link

Support custom safe reg exp #6

Closed itslenny closed 8 years ago

itslenny commented 8 years ago

Add ability to use custom patterns for the safe comment.

One example for when this might be useful (my use case) would be to preserve only JSDoc style comments. It could also be useful for anyone that wants to use something other than /*! to preserve comments.

This is a non-breaking change as safe still has the same default value and can support a boolean, but if you specify a regex or string it will use the provided value as a matcher for preserved comments.

Usage Example:

decomment('/** JSDoc style comment */ here is some stuff', {safe: /^\/\*\* /});
vitaly-t commented 8 years ago

There are some problems with this...

First of all, rule /^\/\*\* / doesn't quite define a jsDoc.

Many times people write comments like this:

/*********************************/
Next paragraph

or similar. That would be found as a jsDoc, even though it is not. Also, some known transformation engines use more advanced logic for keeping jsDoc comments, based on certain attributes set within the jsDoc block. There is no common ground here, it seems, unlike /*! - the only well known keep-comment block.

Another issue - the origin of the parameter. Parameter safe means safe comment removal, while preserving what is a known keep-comment block. Keep-comment opening /*! isn't something invented by this library, it is a standard de facto. Its name and meaning were borrowed from library strip-comments, and it is supported by many environments and utilities.

In case you wonder why rewriting the library in the first place. I found too many examples where strip-comments doesn't work, plus terrible performance. Something is wrong with its implementation, and fixing it seemed more difficult than writing one of my own. Even its gulp version gulp-strip-comments was later changed to use my library instead, for the same reasons.

With all above said, I really want to keep the protocol clean, with the needed minimum.

itslenny commented 8 years ago

First, your module is by far the best solution for comment removal so thanks for making it.

JsDoc is specifically /\ not /*\ (or more).

See here: http://usejsdoc.org/about-getting-started.html

However, just adding JSDocs support doesn't seem flexible enough to me (especially considering differing opinions on what constitutes JSDocs).

So, what would you prefer for a means to preserve other types of comments?

vitaly-t commented 8 years ago

So, what would you prefer for a means to preserve other types of comments?

I wonder how really this is necessary. The way the library is now, it has taken far away from other implementations already. I will think about it.

itslenny commented 8 years ago

The way the library is now, it has taken far away from other implementations already.

I'm not sure why this would necessarily be a bad thing. I'm not saying it's good either just that it shouldn't really matter what other implementations are doing especially since this change doesn't break anything and just adds a little more flexibility.

If you'd like I could add a new option preserve (or another name if you prefer) that would be separate from safe.

However, I prefer the way I implemented it because safe still works as-is, but you can optionally change the definition of what constitutes a "safe comment" and the code to handle the safe comment is basically the same as you had it before (just using a RegExp instead of indexOf).

vitaly-t commented 8 years ago

I haven't forgotten about this, still thinking. Actually, I was quite busy with a related issue...

I needed to minify PostgreSQL scripts, and tried to add support for that into the decomment. After some trials I realized it's all turning into a mess, and rolled it all back.

Instead, I wrote pg-minify, and glad that I made it all seperate, because for SQL it is all so much simpler and cleaner :)

vitaly-t commented 8 years ago

Doesn't look like it is going to happen. Instead, there is now option ignore that lets you implement the same kind of behavior, if you can write a regEx that identifies jsDoc blocks.

vitaly-t commented 8 years ago

@itslenny Further improvements have been made in version 0.8.4

Now you can also find in the option's documentation how to isolate jsDoc comments, which is precisely what you wanted:

{ignore: /\/\*\*([^\*]*(\*[^\/])?)*\*\//g}

see option ignore.

vitaly-t commented 8 years ago

@itslenny I have updated the documentation for option ignore: the regex for detecting jsDoc blocks now includes the rule that says /** must be followed by one or more line break.

I believe this is now more accurate than before ;)