-
Notifications
You must be signed in to change notification settings - Fork 407
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: added flushFilePath() #4240
fix: added flushFilePath() #4240
Conversation
// console.log('what are the args', { | ||
// args0: sendExceptionStub.getCalls()[0].args[0], | ||
// args1: sendExceptionStub.getCalls()[0].args[1] | ||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about either adding a comment here as to why this is no longer applicable or removing entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove them. This was there and was adding noise to the output. salesforcedx-vscode-core has a lot of noise in it when the untegration tests run. When the tests run (either locally, or in CI) there are lots of errors reported, and it's at the point where it's hard to tell when a failure is a failure or not. This is just a "drop in the ocean" and I hope to focus on this at some point and clean up the build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In QA so far, I can confirm that renaming by changing the casing only and then deploying now works correctly. Nice!
Looking at the changes in the PR, are there delete or retrieve actions that should also be tested here?
Yes, this fix was also added to forceSourceDelete.ts and LibraryPathsGatherer.ts And LibraryPathsGatherer is used in fourceSourceDeploySourcePath and fourceSourceRerrieveSourcePath. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks @jeffb-sfdc !
- SFDX: Deploy Source to Org works after renaming locally by only making changes to the casing in the file/class name
- SFDX: Deploy Source to Org works when deploying multiple files after renaming one locally by only making changes to the casing in the file/class name
- SFDX: Retrieve Source from Org works as before
- Note: If you rename locally, you still need to deploy the class first so that it is renamed on the server before executing a “Retrieve” operation
- SFDX: Delete from Project and Org works as before
- Note: Similarly, if you rename the component/class locally, you need to deploy it so that the name is aligned on the server before running a delete operation
Thanks @klewis-sfdc . Just a heads up - I updated the existing tests and they pass. I'm now adding tests for |
Also tested "SFDX: Launch Apex Replay Debugger with Current File" - working as before - looks good! ✅ |
* fix: added flushFilePath() to fix the issue where VS Code passes us stale and out of date URIs
This reverts commit 6b563ec.
What does this PR do?
There's a bug in VS Code, which Microsoft refuses to fix (see microsoft/vscode#152993). After a file has been renamed, if only the casing has changes (eg no characters added or removed) VS Code passes the stale path to commands. This PR adds
flushFilePath()
andflushFilePaths()
to fix this issue.What issues does this PR fix or reference?
##3575, @W-10273785@
Functionality Before
Functionality After
1-3: the same
4. This deployment succeeds