/** vs /*! multiline comment style
revelt opened this issue ยท 14 comments
Very popular npm packages and projects use a different comment style, one which starts with /*!
, with exclamation mark.
I wonder, why are we using /**
instead of /*!
and should we switch to the latter?
As far as I see /*!
are used to prevent JS minifiers from stripping the block and many famous projects are using exactly this way. For example, Vue: https://unpkg.com/vue
Hi @revelt,
This plugin use /**
comment style but I am completely open to use /*!
(in fact, I asked myself this question when I wrote this plugin).
For information, you can already use /*!
if the template you are given starts with this, for example:
module.exports = {
plugins: [
license({
banner: `/*! Copyright <%= moment().format('YYYY') %> */`,
}),
],
}
Otherwise, it will add generate a comment block using /**
.
I'm open to any suggestion, do you have ideas to handle this use case? We can add an option to choose comment style, what do you think?
I had a think about this, I'd say option is the most flexible way.
Plus, if we implement a dedicated option, you could strip all existing comment blocks from the banner and this way convert the style if any was present. This would allow us to support cases:
- In messed up comment block banners (multiple openings, erroneous etc)
- In missing tails of the comment block
What if we hungrily chomped any kinds of opening comment blocks from the beginning of the banner (considering those might be stacked with whitespace in between), then, same way chomp tail of the banner. Then, wrap with preferred style /*!
or (default) /**
?
What do you think?
Thanks for looking into this.
PS. I raised an issue in 3rd-Eden/commenting#8 to allow /*!
blocks. If it was implemented there, we could simply pass the options flag there...
I think, this can already be done with current version of commenting
. We can probably just pass the desired comment style as an option directly in this plugin. Something like this:
const commenting = require('commenting');
const header = commenting(text.trim(), {
extension: '.js',
style: new commenting.Style(' *','/*!', ' */'),
});
The only question is: what do you suggest as option name (and I will be happy to implement that, or if you like, you can submit a PR)? :)
HM, that's a pickle. I did look around, I can't find a formal name how they're called. Maybe option could be something like ignoredCommentsStyle
or genericCommentsStyle
? Kangax mentioned that in http://perfectionkills.com/html-minifier-revisited/
Hi all, may I suggest that the user can define a commenting style in the banner object to extend the functionality of what we have now, or specify not using a style.
license({
banner: {
file: path.join(__dirname, 'HEAD'),
commentingStyle: ['body', 'start', 'end'], // user defined style
noCommenting: true, // no style used, prepend text as is
}),
Where commentingStyle
is an Array then create a custom new style as your example @mjeanroy
Where noCommenting === true
then omit styling and prepend the text as is.
Background: I prepend a Greasemonkey Meta Block. My use case works well with a banner file as a lodash template to insert version number script title, url links and so forth and already has //
style comments.
If you like my proposal I could do some tests and if I'm happy with the results submit a PR for discussion.
^ very logical
@revelt @subz390 I was not a big fan of having two options for more or less the same thing and I was not a big fan of giving an option to define the comment style like this. So, I added an option to specify the comment style using name linking to "pre-defined" style:
license({
banner: {
file: path.join(__dirname, 'HEAD'),
commentStyle: 'regular', // the default, other options may be: `ignored', `slash` or `none`
}),
What this option mean:
- When using
regular
comment style, the "classic" block is used (/** ... */
). - When using
ignored
comment style (use the term that you suggested @revelt), the license block will be used (/*! ... */
). - When using
slash
comment style, only slashed will be used (// ....
). - When
none
is used, the banner is not modified at all (so it is up to the plugin user to be sure to have a valid comment block).
What do you think? Is that ok for you? Would you like to suggest an improvement?
Thank you, I'm happy ๐
Your suggestion gets my +1 vote, thank you ๐
I'm quite excited, updated my rollup configs already, waiting for release...
@mjeanroy how do we customise the comment style if banner
is a string? For example, this does not work:
license({
banner: licenseStr,
commentStyle: "ignored"
})
I could stick the license string into a plain object and put commentStyle
in that object but under which key does the licenseStr
go then?
license({
banner: {
?: licenseStr,
commentStyle: "ignored"
},
})
thank you for help.
Just got around to updating my rollup.config.js
to incorporate the above changes. Would like to say thank you very much @mjeanroy for implementing the options as discussed. It's a small thing, but now my headers are perfect and that makes my OCD feel all warm and fuzzy.
Also thank you for adding a depreciation warning note relating to banner.content.file
mind blown - you'd think all developers could do something like that to help with smooth transitions and updates, instead of forcing your dependents to catch up only when things break.