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

fix: update package.json files to comply with new parser #4749

Merged
merged 12 commits into from
Apr 18, 2023

Conversation

CristiCanizales
Copy link
Contributor

@CristiCanizales CristiCanizales commented Mar 22, 2023

What does this PR do?

  • Updates package.json files across the Salesforce Extensions to comply with new parser

What issues does this PR fix or reference?

@W-12645770@

Functionality Before

  • squigglies in package.json's

Functionality After

  • no squigglies in package.json's

@CristiCanizales CristiCanizales requested a review from a team as a code owner March 22, 2023 19:41
@CristiCanizales CristiCanizales self-assigned this Mar 22, 2023
Copy link
Contributor

@klewis-sfdc klewis-sfdc left a comment

Choose a reason for hiding this comment

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

Thanks for this work, Cristi! Looks great. And thanks for sending me the link to the PR that explains the new parser changes! I've got one question about clearing out one more set of squigglies but otherwise this is looking great.

@@ -153,7 +153,7 @@
},
{
"command": "sfdx.force.test.view.runSingleTest",
"when": "view == sfdx.force.test.view && viewItem =~ /(apexTest)(_.*|\\b)/",
"when": "view == sfdx.force.test.view && viewItem =~ /(apexTest)(_.*||\\b)/",
Copy link
Contributor

Choose a reason for hiding this comment

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

dumb reg ex q. Seems like they moved to js style regular expressions. So did it just change for a|b -> a or b to a||b -> a or b?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems so! According to this issue and this page

Copy link
Contributor

Choose a reason for hiding this comment

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

hum...interestingly the matches example on that page uses the single |
Screenshot 2023-03-24 at 12 01 35 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right... mm then the || would be for other cases?
image

Choose a reason for hiding this comment

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

dumb reg ex q. Seems like they moved to js style regular expressions. So did it just change for a|b -> a or b to a||b -> a or b?
it seems so! According to microsoft/vscode#175540 and this page

Hmm, would you mind pointing which wording made you think so? Because we didn't change how we interpret regex patterns, it's just I probably had worded myself poorly.

Also, I think you don't need to change a when clause if there are no errors underlined.

@randi274
Copy link
Contributor

Remaining QE, given this is a dev-only impact:

  • Build + Spot-check with VSIXes in 1.77+
  • Build + Spot-check with VSIXes in 1.77-

@randi274 randi274 self-assigned this Apr 17, 2023
Copy link
Contributor

@randi274 randi274 left a comment

Choose a reason for hiding this comment

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

QA:
✅ - Builds locally, and also fixes the issue with the extra commands no longer appearing!
✅ - Extensions look good with VSIXes in 1.77.3
✅ - start function, lightning rename and lwc preview commands appear in proper context
✅ - Extensions look good with VSIXes in 1.75.1 (January build)
✅ - start function, lightning rename and lwc preview commands appear in proper context

@CristiCanizales CristiCanizales merged commit e83f740 into develop Apr 18, 2023
12 checks passed
@CristiCanizales CristiCanizales deleted the cristi/new-parser branch April 18, 2023 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants