Skip to content

Conversation

@Trott
Copy link
Member

@TrottTrott commented May 3, 2017

Relax the rule for maximum line length in JS files if the line contains
a regular expression literal. This will avoid the need to convert a
regular expression literal into a RegExp constructor call broken across
multiple lines in order to satisfy the maximum line length rule. That
practice hampers readability.

In lib and test (but mostly test), replace RegExp constructors with regular expression literals where
possible.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools lib test

Trott added 2 commits May 2, 2017 23:11
Relax the rule for maximum line length in JS files if the line contains a regular expression literal. This will avoid the need to convert a regular expression literal into a RegExp constructor call broken across multiple lines in order to satisfy the maximum line length rule. That practice hampers readability.
Replace RegExp constructors with regular expression literals where possible.
@TrottTrott added lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels May 3, 2017
@nodejs-github-botnodejs-github-bot added http Issues or PRs related to the http subsystem. tools Issues and PRs related to the tools directory. labels May 3, 2017
@Trott
Copy link
MemberAuthor

Trott commented May 3, 2017

Copy link
Contributor

@refackrefack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 With both hands

@refack
Copy link
Contributor

When will JS get RegEx.VERBOSE 💚😞

@ChALkeR
Copy link
Member

ChALkeR commented May 4, 2017

This is great! The main purpose behind the max-len rule is to make code more readable (by forcing splitting the code into smaller logical chunks), but in the case of regexps it was doing the opposite. It's nice to see that fixed =).

@addaleax
Copy link
Member

Landed in f1d593c...6bcf65d

@addaleaxaddaleax closed this May 5, 2017
addaleax pushed a commit that referenced this pull request May 5, 2017
Relax the rule for maximum line length in JS files if the line contains a regular expression literal. This will avoid the need to convert a regular expression literal into a RegExp constructor call broken across multiple lines in order to satisfy the maximum line length rule. That practice hampers readability. PR-URL: #12807 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
addaleax pushed a commit that referenced this pull request May 5, 2017
Replace RegExp constructors with regular expression literals where possible. PR-URL: #12807 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
anchnk pushed a commit to anchnk/node that referenced this pull request May 6, 2017
Relax the rule for maximum line length in JS files if the line contains a regular expression literal. This will avoid the need to convert a regular expression literal into a RegExp constructor call broken across multiple lines in order to satisfy the maximum line length rule. That practice hampers readability. PR-URL: nodejs#12807 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
anchnk pushed a commit to anchnk/node that referenced this pull request May 6, 2017
Replace RegExp constructors with regular expression literals where possible. PR-URL: nodejs#12807 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@jasnelljasnell mentioned this pull request May 11, 2017
@gibfahngibfahn mentioned this pull request Jun 15, 2017
3 tasks
@gibfahn
Copy link
Member

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

Trott added a commit to Trott/io.js that referenced this pull request Jun 19, 2017
Relax the rule for maximum line length in JS files if the line contains a regular expression literal. This will avoid the need to convert a regular expression literal into a RegExp constructor call broken across multiple lines in order to satisfy the maximum line length rule. That practice hampers readability. PR-URL: nodejs#12807 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Trott added a commit to Trott/io.js that referenced this pull request Jun 19, 2017
Replace RegExp constructors with regular expression literals where possible. PR-URL: nodejs#12807 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@Trott
Copy link
MemberAuthor

@gibfahn Backport in #13776

gibfahn pushed a commit that referenced this pull request Jun 20, 2017
Relax the rule for maximum line length in JS files if the line contains a regular expression literal. This will avoid the need to convert a regular expression literal into a RegExp constructor call broken across multiple lines in order to satisfy the maximum line length rule. That practice hampers readability. PR-URL: #12807 Backport-PR-URL: #13776 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
gibfahn pushed a commit that referenced this pull request Jun 20, 2017
Replace RegExp constructors with regular expression literals where possible. PR-URL: #12807 Backport-PR-URL: #13776 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@TrottTrott mentioned this pull request Jul 5, 2017
2 tasks
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
Relax the rule for maximum line length in JS files if the line contains a regular expression literal. This will avoid the need to convert a regular expression literal into a RegExp constructor call broken across multiple lines in order to satisfy the maximum line length rule. That practice hampers readability. PR-URL: #12807 Backport-PR-URL: #13776 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
Replace RegExp constructors with regular expression literals where possible. PR-URL: #12807 Backport-PR-URL: #13776 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 18, 2017
@TrottTrott deleted the no-break-regexp branch April 11, 2021 22:47
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

httpIssues or PRs related to the http subsystem.lib / srcIssues and PRs related to general changes in the lib or src directory.testIssues and PRs related to the tests.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@Trott@refack@ChALkeR@addaleax@gibfahn@jasnell@lpinca@hiroppy@vsemozhetbyt@nodejs-github-bot