Skip to content
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.

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

Unify node.gyp code styling #41072

Open
asamuzaK opened this issue Dec 3, 2021 · 1 comment
Open

Unify node.gyp code styling #41072

asamuzaK opened this issue Dec 3, 2021 · 1 comment
Labels
build Issues and PRs related to build files or the CI. gyp Issues and PRs related to the GYP tool and .gyp build files

Comments

@asamuzaK
Copy link

asamuzaK commented Dec 3, 2021

Version

16.13.1, maybe also 17.2.0 (not tested myself)

Platform

Microsoft Windows NT 10.0.19043.0 x64

Subsystem

node.gyp

What steps will reproduce the bug?

In #40481, it combines multiple lines of strings with \ before a line break.
See node/node.gyp at master · nodejs/node

This \ causes an error in gyp-parser addaleax/gyp-parser: GYP file format parser in JS.

I looked at the .gyp docs GYP - Input Format Reference, I wasn't sure if it allowed to combine multiple lines of strings with \.

But any way, it doesn't match code styling in the same file.
In other places, multiple lines are combined like

['"foo" and '
 '"bar"', {
}]

#L79
#L849

I think #L1479 should follow the same code styling.

How often does it reproduce? Is there a required condition?

Always since 16.13.1

What is the expected behavior?

Do not cause errors in other modules.

What do you see instead?

Parse error in gyp-parser.

Additional information

No response

@RaisinTen
Copy link
Contributor

\ is used in a couple of other places too. You can run git grep --line-number '\\$' "*.gyp*" to get a full list if you would like to fix them all.

asamuzaK added a commit to asamuzaK/gyp-parser that referenced this issue Dec 4, 2021
Regarding [Unify node.gyp code styling · Issue #41072 · nodejs/node](nodejs/node#41072), in `node.gyp`, multi-line character strings are combined by inserting `\` before a line break.
That causes 'Unknown escape character' error in `gyp-parser`.

This PR is a bug fix on the `gyp-parser` side.
addaleax pushed a commit to addaleax/gyp-parser that referenced this issue Dec 6, 2021
Regarding [Unify node.gyp code styling · Issue #41072 · nodejs/node](nodejs/node#41072), in `node.gyp`, multi-line character strings are combined by inserting `\` before a line break.
That causes 'Unknown escape character' error in `gyp-parser`.

This PR is a bug fix on the `gyp-parser` side.
@RedYetiDev RedYetiDev added build Issues and PRs related to build files or the CI. gyp Issues and PRs related to the GYP tool and .gyp build files labels Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. gyp Issues and PRs related to the GYP tool and .gyp build files
Projects
None yet
Development

No branches or pull requests

3 participants