Skip to content

WIP:EIM-277 rename activation script in case there is already one present #225

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP:EIM-277 rename activation script in case there is already one present #225

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src-tauri/src/lib/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,24 @@ pub fn create_activation_shell_script(
ensure_path(file_path).map_err(|e| e.to_string())?;
let mut filename = PathBuf::from(file_path);
filename.push(format!("activate_idf_{}.sh", idf_version));
filename = match filename.try_exists() {
Ok(true) => {

let mut new_filename = filename.clone();
new_filename.set_file_name(format!("activate_idf_{}_{}.sh", idf_version, uuid::Uuid::new_v4()));
info!(
"Activation script already exists at {}, changing name to {}.",
filename.display(),
new_filename.display()
);
new_filename
}
Ok(false) => { filename}
Err(e) => {
error!("Failed to check if file exists: {}", e);
filename
}
};
Comment on lines +142 to +159
Copy link

@gemini-code-assist gemini-code-assist bot Oct 6, 2025

Choose a reason for hiding this comment

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

high

This block has two issues:

  1. Code Duplication: The logic for handling existing files is duplicated in create_powershell_profile on lines 356-373. This should be extracted into a helper function to improve maintainability.
  2. Incorrect Error Handling: In case of an error when checking if the file exists, the code falls back to using the original filename. This could lead to overwriting an existing file, which contradicts the goal of the PR ("we will now not rewrite it"). It would be safer to propagate the error.

I suggest creating a helper function that addresses both points. You could add this function within src-tauri/src/lib/mod.rs:

fn ensure_unique_filename(path: PathBuf, file_type: &str) -> Result<PathBuf, std::io::Error> {
    match path.try_exists() {
        Ok(true) => {
            let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or_default();
            let extension = path.extension().and_then(|s| s.to_str()).unwrap_or_default();

            let mut new_path = path.clone();
            let new_name = if extension.is_empty() {
                format!("{}_{}", stem, uuid::Uuid::new_v4())
            } else {
                format!("{}_{}.{}", stem, uuid::Uuid::new_v4(), extension)
            };
            new_path.set_file_name(new_name);

            info!(
                "{} already exists at {}, changing name to {}.",
                file_type,
                path.display(),
                new_path.display()
            );
            Ok(new_path)
        }
        Ok(false) => Ok(path),
        Err(e) => {
            error!("Failed to check if file exists: {}. Aborting.", e);
            Err(e)
        }
    }
}

With this helper function, you can replace this entire match block with:
filename = ensure_unique_filename(filename, "Activation script").map_err(|e| e.to_string())?;

let template = include_str!("../../bash_scripts/activate_idf_template.sh");
let mut tera = Tera::default();
if let Err(e) = tera.add_raw_template("activate_idf_template", template) {
Expand Down Expand Up @@ -335,6 +353,24 @@ fn create_powershell_profile(
}
let mut filename = PathBuf::from(profile_path);
filename.push(format!("Microsoft.{}.PowerShell_profile.ps1", idf_version));
filename = match filename.try_exists() {
Ok(true) => {

let mut new_filename = filename.clone();
new_filename.set_file_name(format!("Microsoft.{}.PowerShell_profile_{}.ps1", idf_version, uuid::Uuid::new_v4()));
info!(
"Profile already exists at {}, changing name to {}.",
filename.display(),
new_filename.display()
);
new_filename
}
Ok(false) => { filename}
Err(e) => {
error!("Failed to check if file exists: {}", e);
filename
}
};
Comment on lines +356 to +373
Copy link

@gemini-code-assist gemini-code-assist bot Oct 6, 2025

Choose a reason for hiding this comment

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

high

This block has the same issues as the one in create_activation_shell_script (lines 142-159): code duplication and incorrect error handling.

As suggested in the other comment, this logic should be extracted to a shared helper function. You can then replace this block with a call to the suggested ensure_unique_filename function:

filename = ensure_unique_filename(filename, "Profile")?;

fs::write(&filename, rendered).expect("Unable to write file");
Ok(filename.display().to_string())
}
Expand Down
Loading