style: add gulp task to only format changed files #24969

brandonroberts · 2018-07-18T18:30:59Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[x] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

This introduces a new gulp task to only format changed files (using git diff) that intersect the whitelist of necessary source files. The diff is compared against the provided branch (default: master). It significantly reduces the time to run formatting when working on docs and source code.

gkalpak

LGTM 👍
Unfortunately, git diff will not list untracked files, but I don't think there is a non-intrusive way to fix this.

tools/gulp-tasks/format.js

brandonroberts · 2018-09-06T21:42:13Z

@gkalpak yes, I'm going implement that idea.

gkalpak · 2018-09-07T08:33:04Z

You might want to hurry. You have competition now: #25847 😛

brandonroberts · 2018-09-07T15:57:29Z

@gkalpak @petebacondarwin this one is ready for another review

gkalpak · 2018-09-10T17:02:36Z

tools/gulp-tasks/format.js

+    let currentMatch;
+
+    while ((currentMatch = RE_STATUS.exec(data)) !== null) {
+      // This is necessary to avoid infinite loops with zero-width matches


I don't think there can be zero-width matches with the current RegExp 😕
Can you give an example?

I can't think of an example since the output from git is standardized. I generated this when I was testing the Regex. I'll remove that line.

This is more about the RegExp (the input it runs on does not matter). This problem is true for RegExps that can have a zero match (i.e. do match with the input). For example: /.*/ (since * can match 0 or more character).

It is one of the weird issue of JS RegExps. If you don't know about this exec() limitation, it must be quite frustrating figuring it out 😁

gkalpak · 2018-09-10T17:11:39Z

gulpfile.js

@@ -29,6 +29,9 @@ function loadTask(fileName, taskName) {

 gulp.task('format:enforce', loadTask('format', 'enforce'));
 gulp.task('format', loadTask('format', 'format'));
+gulp.task('format:changes', loadTask('format', 'format-changes'));
+gulp.task('format:diff', loadTask('format', 'format-diff'));
+gulp.task('format:changed', ['format:changes', 'format:diff']);


AFAICT, format:changes and format:diff have a good chance of formatting the same files (i.e. formatting a file twice).

Not a big deal, of course, but if you wanted to avoid that, you could change format:changes to only format untracked files (and rename to format:untracked 😛).

Then, the combination of format:diff and format:untracked would format all files without formatting a file twice \o/

Closes angular#24904

Closes angular#24904 PR Close angular#24969

…ngular#24969) PR Close angular#24969

…angular#24969) PR Close angular#24969

angular-automatic-lock-bot · 2019-09-14T02:22:11Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Jul 18, 2018

brandonroberts added target: major This PR is targeted for the next major release comp: misc labels Jul 18, 2018

brandonroberts requested review from IgorMinar, gkalpak and petebacondarwin July 18, 2018 18:31

gkalpak approved these changes Jul 18, 2018

View reviewed changes

petebacondarwin reviewed Jul 21, 2018

View reviewed changes

tools/gulp-tasks/format.js Show resolved Hide resolved

brandonroberts added the state: WIP label Jul 23, 2018

gkalpak mentioned this pull request Sep 6, 2018

build: add a script that only formats affected files #25847

Closed

brandonroberts force-pushed the format-changed branch from e9a42f3 to 44d795f Compare September 7, 2018 15:50

brandonroberts added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Sep 10, 2018

gkalpak approved these changes Sep 10, 2018

View reviewed changes

brandonroberts force-pushed the format-changed branch 2 times, most recently from 600c5b4 to f6e9528 Compare September 17, 2018 18:25

brandonroberts added 2 commits September 27, 2018 12:40

style: add gulp task to only format changed files

c89c6e9

Closes angular#24904

style: add combined task to format from git diff and status commands

1c82003

brandonroberts force-pushed the format-changed branch from f6e9528 to 0e287cb Compare September 27, 2018 17:41

style: update gulp task to format untracked and diff files separately

7388f87

brandonroberts force-pushed the format-changed branch from 0e287cb to 7388f87 Compare September 27, 2018 17:55

IgorMinar approved these changes Sep 27, 2018

View reviewed changes

IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 27, 2018

alxhub closed this in ac3252a Sep 27, 2018

brandonroberts deleted the format-changed branch September 27, 2018 19:15

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

style: add gulp task to only format changed files (angular#24969)

1a9aec1

Closes angular#24904 PR Close angular#24969

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

style: add combined task to format from git diff and status commands (a…

7cf8b3e

…ngular#24969) PR Close angular#24969

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

style: update gulp task to format untracked and diff files separately (…

e2112a6

…angular#24969) PR Close angular#24969

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

style: add gulp task to only format changed files #24969

style: add gulp task to only format changed files #24969

brandonroberts commented Jul 18, 2018

gkalpak left a comment

brandonroberts commented Sep 6, 2018

gkalpak commented Sep 7, 2018 •

edited

brandonroberts commented Sep 7, 2018

gkalpak Sep 10, 2018

brandonroberts Sep 10, 2018

gkalpak Sep 10, 2018

gkalpak Sep 10, 2018 •

edited

brandonroberts Sep 10, 2018

angular-automatic-lock-bot bot commented Sep 14, 2019

style: add gulp task to only format changed files #24969

style: add gulp task to only format changed files #24969

Conversation

brandonroberts commented Jul 18, 2018

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

gkalpak left a comment

Choose a reason for hiding this comment

brandonroberts commented Sep 6, 2018

gkalpak commented Sep 7, 2018 • edited

brandonroberts commented Sep 7, 2018

gkalpak Sep 10, 2018

Choose a reason for hiding this comment

brandonroberts Sep 10, 2018

Choose a reason for hiding this comment

gkalpak Sep 10, 2018

Choose a reason for hiding this comment

gkalpak Sep 10, 2018 • edited

Choose a reason for hiding this comment

brandonroberts Sep 10, 2018

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 14, 2019

gkalpak commented Sep 7, 2018 •

edited

gkalpak Sep 10, 2018 •

edited