-
Notifications
You must be signed in to change notification settings - Fork 146
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
feat(vscode): updated extensions for a given profile #1022
feat(vscode): updated extensions for a given profile #1022
Conversation
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.
Sorry for the late reply, been overwhelmed by notifications so I missed this one🤦♂️
Left some suggested changes, which would make the code more rusty
src/config.rs
Outdated
pub fn vscode_profile(&self) -> &str { | ||
self.config_file | ||
.vscode | ||
.as_ref() | ||
.and_then(|vscode| vscode.profile.as_deref()) | ||
.unwrap_or("") | ||
} |
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.
pub fn vscode_profile(&self) -> &str { | |
self.config_file | |
.vscode | |
.as_ref() | |
.and_then(|vscode| vscode.profile.as_deref()) | |
.unwrap_or("") | |
} | |
pub fn vscode_profile(&self) -> Option<&str> { | |
let vscode_cfg = self.config_file.vscode?; | |
let profile = vscode_cfg.profile?; | |
// Treat empty profile as None | |
if profile.is_empty() { | |
None | |
} else { | |
profile.as_str() | |
} | |
} | |
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.
Had to make some changes to apply this suggestion. The borrow checker had some complaints.
f0d055c
src/steps/generic.rs
Outdated
if !ctx.config().vscode_profile().is_empty() { | ||
ctx.run_type() | ||
.execute(vscode) | ||
.arg("--profile") | ||
.arg(ctx.config().vscode_profile()) | ||
.arg("--update-extensions") | ||
.status_checked() | ||
} else { | ||
ctx.run_type() | ||
.execute(vscode) | ||
.arg("--update-extensions") | ||
.status_checked() | ||
} |
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.
if !ctx.config().vscode_profile().is_empty() { | |
ctx.run_type() | |
.execute(vscode) | |
.arg("--profile") | |
.arg(ctx.config().vscode_profile()) | |
.arg("--update-extensions") | |
.status_checked() | |
} else { | |
ctx.run_type() | |
.execute(vscode) | |
.arg("--update-extensions") | |
.status_checked() | |
} | |
if let Some(profile) = ctx.config().vscode_profile() { | |
ctx.run_type() | |
.execute(vscode) | |
.arg("--profile") | |
.arg(profile) | |
.arg("--update-extensions") | |
.status_checked() | |
} else { | |
ctx.run_type() | |
.execute(vscode) | |
.arg("--update-extensions") | |
.status_checked() | |
} |
src/steps/generic.rs
Outdated
@@ -9,6 +10,7 @@ use std::{fs, io::Write}; | |||
use color_eyre::eyre::eyre; | |||
use color_eyre::eyre::Context; | |||
use color_eyre::eyre::Result; | |||
use color_eyre::owo_colors::OwoColorize; |
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.
Same here
Looks like this was imported accidentally?
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.
Removed e31ea1c
src/steps/generic.rs
Outdated
@@ -1,6 +1,7 @@ | |||
#![allow(unused_imports)] | |||
|
|||
use std::ffi::OsStr; | |||
use std::ops::Deref; |
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 like this was imported accidentally?
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.
Yes, must have been imported automatically by my editor.
Removed e31ea1c
config.example.toml
Outdated
# Specify the profile the extensions should be updated for. | ||
# If no profile is selected, the default one will be used. | ||
# (default: "") |
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.
# Specify the profile the extensions should be updated for. | |
# If no profile is selected, the default one will be used. | |
# (default: "") | |
# If this is set and is a non-empty string, it specify the profile the extensions | |
# should be updated for. | |
# | |
# (default: this won't be set by default) |
@SteveLauC thanks for the suggestions! They're appreciated, especially since I'm not a Rust dev. |
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.
Thanks!
What does this PR do
This PR adds a configuration extension to allow selecting a Vscode profile for which the extensions should be updated for. If no profile is stated in the config file, the default profile is used.
Standards checklist
CONTRIBUTING.md
For new steps
--dry-run
option works with this step--yes
option works with this step if it is supported bythe 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.