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.

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

Fixes argument parsing for escaped newlines #1176

Merged
merged 4 commits into from
Dec 7, 2024
Merged

Fixes argument parsing for escaped newlines #1176

merged 4 commits into from
Dec 7, 2024

Conversation

aarondandy
Copy link
Contributor

@aarondandy aarondandy commented Dec 6, 2024 •
edited by ehmicky
Loading

Fixes #1175.

Trying to handle escaped line breaks in template strings.

@aarondandy aarondandy changed the title Adds test for escaped newline issue Fixes argument parsing for escaped newlines Dec 6, 2024
@aarondandy
Copy link
Contributor Author

I'm not sure if I have this fix right, even though the test passes.

@ehmicky
Copy link
Collaborator

ehmicky commented Dec 6, 2024

Hi @aarondandy,

Thanks, but this should not be needed due to #1175 (comment)

@ehmicky ehmicky closed this Dec 6, 2024
@aarondandy
Copy link
Contributor Author

aarondandy commented Dec 6, 2024 •
edited
Loading

Even though this is closed, I still think this indicates a bug in splitByWhitespaces. I think my current solution is insufficient though because it does not correctly treat these escaped newlines as token terminators which appears to be the intent of this method. An example that I think may highlight this would be a failure if the next line is unindented. I'm thinking that maybe the behavior that would better fit the intent of this method would be to delimit tokens on these escaped newlines while also correctly adjusting the index values.

Edit: I'm not longer confident in my unconfidence, see: #1175 (comment)

aarondandy
aarondandy commented Dec 6, 2024
View reviewed changes
lib/methods/template.js Outdated Show resolved Hide resolved
@ehmicky
Copy link
Collaborator

ehmicky commented Dec 6, 2024

Let's continue this discussion on #1175.

@ehmicky ehmicky reopened this Dec 6, 2024
@ehmicky ehmicky marked this pull request as ready for review December 6, 2024 23:10
@ehmicky
Copy link
Collaborator

ehmicky commented Dec 6, 2024 •
edited
Loading

This looks good (except for a few small things), thanks for the PR. 👍

@ehmicky ehmicky mentioned this pull request Dec 6, 2024
Closed
ehmicky
ehmicky reviewed Dec 7, 2024
View reviewed changes
test/methods/template.js Outdated Show resolved Hide resolved
@ehmicky
Copy link
Collaborator

ehmicky commented Dec 7, 2024

Looks good to me. Thanks again @aarondandy.

@sindresorhus Is this good to merge?

@ehmicky ehmicky merged commit 2aa3b0c into sindresorhus:main Dec 7, 2024
8 checks passed
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Dec 8, 2024
@renovate
chore(deps): update dependency execa to v9.5.2
f8f1b0c 
##### [v9.5.2](https://github.com/sindresorhus/execa/releases/tag/v9.5.2)

#### Bug fixes

-   Fix escaping newlines inside template strings. Thanks [@aarondandy](https://github.com/aarondandy)! ([#1176](sindresorhus/execa#1176))
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 8, 2024
@renovate
Update dependency execa to v9.5.2
1b20cc7 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | execa   | 9.5.1 | 9.5.2 |


## [v9.5.2](https://github.com/sindresorhus/execa/releases/tag/v9.5.2)

#### Bug fixes

-   Fix escaping newlines inside template strings. Thanks [@aarondandy](https://github.com/aarondandy)! ([#1176](sindresorhus/execa#1176))
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 10, 2024
@renovate
Update dependency execa to v9.5.2
53e0fdf 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | execa   | 9.5.1 | 9.5.2 |


## [v9.5.2](https://github.com/sindresorhus/execa/releases/tag/v9.5.2)

#### Bug fixes

-   Fix escaping newlines inside template strings. Thanks [@aarondandy](https://github.com/aarondandy)! ([#1176](sindresorhus/execa#1176))
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 11, 2024
@renovate
Update dependency execa to v9.5.2
b412c80 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | execa   | 9.5.1 | 9.5.2 |


## [v9.5.2](https://github.com/sindresorhus/execa/releases/tag/v9.5.2)

#### Bug fixes

-   Fix escaping newlines inside template strings. Thanks [@aarondandy](https://github.com/aarondandy)! ([#1176](sindresorhus/execa#1176))
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 12, 2024
@renovate
Update dependency execa to v9.5.2
43def1b 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | execa   | 9.5.1 | 9.5.2 |


## [v9.5.2](https://github.com/sindresorhus/execa/releases/tag/v9.5.2)

#### Bug fixes

-   Fix escaping newlines inside template strings. Thanks [@aarondandy](https://github.com/aarondandy)! ([#1176](sindresorhus/execa#1176))
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Dec 14, 2024
@renovate
Update dependency execa to v9.5.2
8d0030d 
| datasource | package | from  | to    |
| ---------- | ------- | ----- | ----- |
| npm        | execa   | 9.5.1 | 9.5.2 |


## [v9.5.2](https://github.com/sindresorhus/execa/releases/tag/v9.5.2)

#### Bug fixes

-   Fix escaping newlines inside template strings. Thanks [@aarondandy](https://github.com/aarondandy)! ([#1176](sindresorhus/execa#1176))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehmicky ehmicky ehmicky approved these changes

@sindresorhus sindresorhus sindresorhus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Escaped newlines break arguments in version 9
3 participants
@aarondandy @ehmicky @sindresorhus