Skip to content

[scripts] lint-whitespace: use perl instead of grep -P #12097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 6, 2018
Merged

[scripts] lint-whitespace: use perl instead of grep -P #12097

merged 1 commit into from
Mar 6, 2018

Conversation

Sjors
Copy link
Member

@Sjors Sjors commented Jan 5, 2018 •
edited
Loading

MacOS does not support grep -P out of the box. This change makes
it easier for developers to check for whitespace problems locally.

Based on this and this Stack Exchange answer.

Tested with:

export TRAVIS_COMMIT_RANGE='fe78c9a...62e0453'
contrib/devtools/lint-whitespace.sh 
This diff appears to have added new lines with tab characters instead of spaces.
The following changes were suspected:

diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
@@ -0,0 +1,110 @@
+	num.setint64(n);

@Sjors
[scripts] lint-whitespace: use perl instead of grep -P
40b17f5 
MacOS does not support 'grep -P' out of the box. This change makes
it easier for developers to check for whitespace problems locally.
jonasschnelli
jonasschnelli approved these changes Jan 5, 2018
View reviewed changes
Copy link
Contributor

@jonasschnelli jonasschnelli left a comment

Choose a reason for hiding this comment

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

Tested (macOS) ACK 40b17f5

@sipa
Copy link
Member

sipa commented Jan 6, 2018

Concept ACK

@randolf
Copy link
Contributor

randolf commented Jan 9, 2018

On NetBSD 7.0, "grep -P" is also not supported, so this change can be helpful there too (and maybe other platforms as well?).

@practicalswift
Copy link
Contributor

Concept ACK

@laanwj
Copy link
Member

laanwj commented Feb 17, 2018 •
edited
Loading

Please, next time someone writes a script like this, use Python. Shell script is terrible for making portable tools. Every time someone introduces a shell script we go through countless iterations to make it compatible with every shell and OS tool environment since the 80's.

(nevertheless, concept ACK)

@laanwj laanwj merged commit 40b17f5 into bitcoin:master Mar 6, 2018
laanwj added a commit that referenced this pull request Mar 6, 2018
@laanwj
Merge #12097: [scripts] lint-whitespace: use perl instead of grep -P
14475e2 
40b17f5 [scripts] lint-whitespace: use perl instead of grep -P (Sjors Provoost)

Pull request description:

  MacOS does not support `grep -P` out of the box. This change makes
  it easier for developers to check for whitespace problems locally.

  Based on [this](https://stackoverflow.com/a/16658690) and [this](https://serverfault.com/a/504387) Stack Exchange answer.

  Tested with:
  ```sh
  export TRAVIS_COMMIT_RANGE='fe78c9a...62e0453'
  contrib/devtools/lint-whitespace.sh
  This diff appears to have added new lines with tab characters instead of spaces.
  The following changes were suspected:

  diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
  @@ -0,0 +1,110 @@
  +	num.setint64(n);
  ```

Tree-SHA512: 37c342a0ca2580289cf326a278a051a7c21ba918d6b2143fd9987f159fab85f1de3d770fcf532a642cd5d1957afc8595678128196e102dc473924758f133db7f
@Sjors Sjors deleted the lint-whitespace-no-grep-P branch March 6, 2018 22:17
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12097: [scripts] lint-whitespace: use perl instead of g…
6f3f0af 
…rep -P

40b17f5 [scripts] lint-whitespace: use perl instead of grep -P (Sjors Provoost)

Pull request description:

  MacOS does not support `grep -P` out of the box. This change makes
  it easier for developers to check for whitespace problems locally.

  Based on [this](https://stackoverflow.com/a/16658690) and [this](https://serverfault.com/a/504387) Stack Exchange answer.

  Tested with:
  ```sh
  export TRAVIS_COMMIT_RANGE='fe78c9a...62e0453'
  contrib/devtools/lint-whitespace.sh
  This diff appears to have added new lines with tab characters instead of spaces.
  The following changes were suspected:

  diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
  @@ -0,0 +1,110 @@
  +	num.setint64(n);
  ```

Tree-SHA512: 37c342a0ca2580289cf326a278a051a7c21ba918d6b2143fd9987f159fab85f1de3d770fcf532a642cd5d1957afc8595678128196e102dc473924758f133db7f
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12097: [scripts] lint-whitespace: use perl instead of g…
e1a9bce 
…rep -P

40b17f5 [scripts] lint-whitespace: use perl instead of grep -P (Sjors Provoost)

Pull request description:

  MacOS does not support `grep -P` out of the box. This change makes
  it easier for developers to check for whitespace problems locally.

  Based on [this](https://stackoverflow.com/a/16658690) and [this](https://serverfault.com/a/504387) Stack Exchange answer.

  Tested with:
  ```sh
  export TRAVIS_COMMIT_RANGE='fe78c9a...62e0453'
  contrib/devtools/lint-whitespace.sh
  This diff appears to have added new lines with tab characters instead of spaces.
  The following changes were suspected:

  diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
  @@ -0,0 +1,110 @@
  +	num.setint64(n);
  ```

Tree-SHA512: 37c342a0ca2580289cf326a278a051a7c21ba918d6b2143fd9987f159fab85f1de3d770fcf532a642cd5d1957afc8595678128196e102dc473924758f133db7f
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 17, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12097: [scripts] lint-whitespace: use perl instead of g…
aa70077 
…rep -P

40b17f5 [scripts] lint-whitespace: use perl instead of grep -P (Sjors Provoost)

Pull request description:

  MacOS does not support `grep -P` out of the box. This change makes
  it easier for developers to check for whitespace problems locally.

  Based on [this](https://stackoverflow.com/a/16658690) and [this](https://serverfault.com/a/504387) Stack Exchange answer.

  Tested with:
  ```sh
  export TRAVIS_COMMIT_RANGE='fe78c9a...62e0453'
  contrib/devtools/lint-whitespace.sh
  This diff appears to have added new lines with tab characters instead of spaces.
  The following changes were suspected:

  diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
  @@ -0,0 +1,110 @@
  +	num.setint64(n);
  ```

Tree-SHA512: 37c342a0ca2580289cf326a278a051a7c21ba918d6b2143fd9987f159fab85f1de3d770fcf532a642cd5d1957afc8595678128196e102dc473924758f133db7f
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 18, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12097: [scripts] lint-whitespace: use perl instead of g…
a88ad93 
…rep -P

40b17f5 [scripts] lint-whitespace: use perl instead of grep -P (Sjors Provoost)

Pull request description:

  MacOS does not support `grep -P` out of the box. This change makes
  it easier for developers to check for whitespace problems locally.

  Based on [this](https://stackoverflow.com/a/16658690) and [this](https://serverfault.com/a/504387) Stack Exchange answer.

  Tested with:
  ```sh
  export TRAVIS_COMMIT_RANGE='fe78c9a...62e0453'
  contrib/devtools/lint-whitespace.sh
  This diff appears to have added new lines with tab characters instead of spaces.
  The following changes were suspected:

  diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
  @@ -0,0 +1,110 @@
  +	num.setint64(n);
  ```

Tree-SHA512: 37c342a0ca2580289cf326a278a051a7c21ba918d6b2143fd9987f159fab85f1de3d770fcf532a642cd5d1957afc8595678128196e102dc473924758f133db7f
@str4d str4d mentioned this pull request Oct 27, 2020
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Oct 28, 2020
@zkbot
Auto merge of #4829 - str4d:backport-lints, r=<try>
43ac206 
Backport useful lints from upstream

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6892
- bitcoin/bitcoin#11151
- bitcoin/bitcoin#11300
- bitcoin/bitcoin@96d91b7
- bitcoin/bitcoin#12097
- bitcoin/bitcoin#12098
- bitcoin/bitcoin#12442
- bitcoin/bitcoin#12572
- bitcoin/bitcoin#12757
- bitcoin/bitcoin#11878
- bitcoin/bitcoin#12933
- bitcoin/bitcoin#12871
- bitcoin/bitcoin#12972
- bitcoin/bitcoin#13281
- bitcoin/bitcoin#13385
- bitcoin/bitcoin#13041
- bitcoin/bitcoin#13454
- bitcoin/bitcoin#13448
- bitcoin/bitcoin#13510
- bitcoin/bitcoin#13851
- bitcoin/bitcoin#13863
- bitcoin/bitcoin#14115
- bitcoin/bitcoin#14831
- bitcoin/bitcoin#15164
- bitcoin/bitcoin#15170
- bitcoin/bitcoin#15166
- bitcoin/bitcoin#16036
- bitcoin/bitcoin#16768

Several of the lints fail for our current codebase; these will be addressed in a
subsequent PR.
zkbot added a commit to zcash/zcash that referenced this pull request Nov 9, 2020
@zkbot
Auto merge of #4829 - str4d:backport-lints, r=str4d
84a5830 
Backport useful lints from upstream

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6892
- bitcoin/bitcoin#11151
- bitcoin/bitcoin#11300
- bitcoin/bitcoin@96d91b7
- bitcoin/bitcoin#12097
- bitcoin/bitcoin#12098
- bitcoin/bitcoin#12442
- bitcoin/bitcoin#12572
- bitcoin/bitcoin#12757
- bitcoin/bitcoin#11878
- bitcoin/bitcoin#12933
- bitcoin/bitcoin#12871
- bitcoin/bitcoin#12972
- bitcoin/bitcoin#13281
- bitcoin/bitcoin#13385
- bitcoin/bitcoin#13041
- bitcoin/bitcoin#13454
- bitcoin/bitcoin#13448
- bitcoin/bitcoin#13510
- bitcoin/bitcoin#13851
- bitcoin/bitcoin#13863
- bitcoin/bitcoin#14115
- bitcoin/bitcoin#14831
- bitcoin/bitcoin#15164
- bitcoin/bitcoin#15170
- bitcoin/bitcoin#15166
- bitcoin/bitcoin#16036
- bitcoin/bitcoin#16768
- bitcoin/bitcoin#13494

Several of the lints fail for our current codebase; these will be addressed in a
subsequent PR.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@jonasschnelli jonasschnelli jonasschnelli approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Scripts and tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Sjors @sipa @randolf @practicalswift @laanwj @jonasschnelli @fanquake