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

[bugfix] Fix errors for scripts with spaces #6027

Merged
merged 5 commits into from
Mar 29, 2025
Merged

Conversation

haiiliin
Copy link
Owner

Description

Fixes #6026

@haiiliin haiiliin added the backport-to-all Backport to all branches label Mar 29, 2025
@haiiliin haiiliin requested a review from Copilot March 29, 2025 02:28
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aims to fix errors when script paths containing spaces are used by ensuring they are properly quoted.

  • Update the quote wrapping logic in the cae and python functions.
  • Introduce f-string formatting in some parts while some instances still use incorrect string interpolation.
Comments suppressed due to low confidence (2)

src/abqpy/cli.py:113

  • Consider using f-string formatting (e.g., f'"{script}"') instead of a literal string to ensure the script variable is correctly interpolated.
script = '"{script}"' if " " in script else script

src/abqpy/cli.py:170

  • Consider using f-string formatting (e.g., f'"{script}"') instead of a literal string to correctly embed the script variable when its value contains spaces.
script = '"{script}"' if " " in script else script

@haiiliin haiiliin requested a review from Copilot March 29, 2025 02:30
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes errors when scripts with spaces are passed by ensuring that such paths are properly quoted. The changes add inline logic to wrap script paths in quotes in different functions to achieve this fix.

  • Update quoting logic in the cae function
  • Update quoting logic in the pde and python functions
Comments suppressed due to low confidence (1)

src/abqpy/cli.py:170

  • [nitpick] Similar to the logic in the cae function, this logic might double quote scripts that have already been quoted. A check to strip or detect existing quotes can help prevent this issue.
script = f'"{script}"' if " " in script else script

@haiiliin haiiliin force-pushed the fix-path-with-spaces branch from 15ce421 to 316fc80 Compare March 29, 2025 02:34
@haiiliin haiiliin requested a review from Copilot March 29, 2025 02:34
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request fixes the errors that occur when scripts with spaces in their paths are used. The changes adjust the way scripts are quoted across different functions to ensure paths containing spaces are handled correctly.

  • Update script quoting in the cae function.
  • Update script and multiple scripts quoting in the pde function.
  • Update script quoting in the python function.

@haiiliin haiiliin changed the title [bugfix] Fix errors for scripts with spaces in paths [bugfix] Fix errors for scripts with spaces Mar 29, 2025
@haiiliin haiiliin merged commit 91e28d1 into 2025 Mar 29, 2025
8 checks passed
@haiiliin haiiliin deleted the fix-path-with-spaces branch March 29, 2025 03:09
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot pushed a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
mergify bot added a commit that referenced this pull request Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-all Backport to all branches bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Path with spaces have error when trying to run the script
1 participant