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 vscode extensions update step #650

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

samhanic
Copy link
Contributor

Standards checklist:

  • The PR title is descriptive.
  • I have read CONTRIBUTING.md
  • The code compiles (cargo build)
  • The code passes rustfmt (cargo fmt)
  • The code passes clippy (cargo clippy)
  • The code passes tests (cargo test)
  • Optional: I have tested the code myself

For new steps

  • Optional: Topgrade skips this step where needed
  • Optional: The --dry-run option works with this step
  • Optional: The --yes option works with this step if it is supported by
    the underlying command

If you developed a feature or a bug fix for someone else and you do not have the
means to test it, please tag this person here.

Fixes #589
Fixes #593
Fixes #594
Fixes #609

This PR fixes the vscode extensions update in quite a few tricky situations by using the now upstreamed code --update-extensions command in the text editor ( vscode/pull#199893 ). This mean the step was completely rewritten.

Note that the command will only be released with January 2024 release (which should be published by early February) so this means I have NOT been able to properly test the code. This is why I create this as a draft until I can properly test it on the release.

Note also that is_wls is now a global function defined and compiled even if the target is not linux.
This is because I needed is_wsl to be global function to be able to properly deactivate the unusual behavior of WSL launching vscode server.

Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

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

Much appreciated!

@@ -349,7 +349,7 @@ fn run() -> Result<()> {
runner.execute(Step::Vcpkg, "vcpkg", || generic::run_vcpkg_update(&ctx))?;
runner.execute(Step::Pipx, "pipx", || generic::run_pipx_update(&ctx))?;
runner.execute(Step::Vscode, "Visual Studio Code extensions", || {
generic::run_vscode_extensions_upgrade(&ctx)
generic::run_vscode_extensions_update(&ctx)
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for changing it! Nice!

@SteveLauC
Copy link
Member

so this means I have NOT been able to properly test the code. This is why I create this as a draft until I can properly test it on the release.

Changes LGTM, do you want to merge this PR now or after that release?

@samhanic samhanic marked this pull request as ready for review January 23, 2024 20:01
@samhanic
Copy link
Contributor Author

As you want! Knowing that the next release of it is almost ready (at "endgame" week: microsoft/vscode#201255) and will be published within few days, I think we can wait for the release before testing.
But if you are confident enough to merge right away, I also don't see any problem to do so. I've just changed the status as ready if you decide so ;)

@SteveLauC
Copy link
Member

Let's merge this! Changes are good and it won't harm anything since we have checked the version!

@SteveLauC SteveLauC merged commit 10e1e17 into topgrade-rs:main Jan 24, 2024
7 checks passed
@pep-sanwer
Copy link

FYI looks like this change broke the VSCode extensions update step:
(up is an alias for topgrade -c )

image

code --version fails within topgrade but runs just fine in the same session.

Appreciate all the work on this wonderful tool!

@SteveLauC
Copy link
Member

code --version fails within topgrade but runs just fine in the same session.

So weird🥲

@SteveLauC
Copy link
Member

Hi, @pep-sanwer, this issue has been fixed, see #720 and #750:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants