mootools / mootools-core

MooTools Core Repository
https://mootools.net
2.65k stars 507 forks source link

DEV Comments in Source Files #2742

Closed boppy closed 8 years ago

boppy commented 8 years ago

I recently changed a RegExp in more/URI by PR and read twice that the RegExp is not "that easy". It isn't but it's neither THAT hard. So I want to describe the RegExp to ensure that upcoming changes are not put on hold due to missing understanding of the regex.

After a short discussion with Sérgio I finaly think that it would be possible to include dev-comments just like compat-code but to strip it out at "go-live" by adding another skip to grunt-options (@mootools-more:/gruntfile.js:L52, @mootools-core:/Tests/gruntfile-options.js:L25). Might be added to other builds also.

I would add a PR to more and core if the is any interest in DEV-Comments just like

var regex = /\d+\s\d*/;
/*<devComment>
Definition of RegExp:
\d+    At least one Number [0-9]
\s     A single Space-Char
\d*    Any number of Numbers (also zero) [0-9]
</devComment>*/
SergioCrisostomo commented 8 years ago

I am ok with the "devComment" idea, in cases where complexity of code might need some explanation. Can be positive and pedagogic. These possible addictions to the repository would still be analysed one by one in pull requests.

I suppose the syntax the packager needs should be:

/*<devComment>*/ - to indicate start of block /* ... */ - for the comment itself, before the line it refers to /*</devComment>*/ - after the block of comment, before the actual code line

timwienk commented 8 years ago

There is no reason to explain how regular expressions work in general, I'm sure that anyone who wants to work or read regular expressions either knows what things like \d mean or can find this information elsewhere, it's super basic information.

However, more content related comments regarding regular expressions are very useful. What we've done in the past is add a comment explaining how to build the specific regular expression just above the "compiled" version of it, so it's easier to recreate and/or update and enhance.

Example: https://github.com/mootools/mootools-more/blob/1.5.2/Source/Forms/Form.Validator.js#L473-L486

/*
var chars = "[a-z0-9!#$%&'*+/=?^_`{|}~-]",
    local = '(?:' + chars + '\\.?){0,63}' + chars,

    label = '[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?',
    hostname = '(?:' + label + '\\.)*' + label;

    octet = '(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)',
    ipv4 = '\\[(?:' + octet + '\\.){3}' + octet + '\\]',

    domain = '(?:' + hostname + '|' + ipv4 + ')';

var regex = new RegExp('^' + local + '@' + domain + '$', 'i');
*/
var regex = /^(?:[a-z0-9!#$%&'*+\/=?^_`{|}~-]\.?){0,63}[a-z0-9!#$%&'*+\/=?^_`{|}~-]@(?:(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)*[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\])$/i;

I would not like to add "<devComment>" around it, there is no reason to. They're comments in the source code, so they're "dev comments" by default. Minified versions will strip them out anyway.

SergioCrisostomo commented 8 years ago

Closing this now. If I can resume the ideas here:

@boppy thank you for opening this and raising the question.

Cheers