Skip to content

fix: source not respecting arguments of sourced commands - #604

Merged
caarlos0 merged 1 commit into
mainfrom
source-set-fix
Mar 27, 2025
Merged

caarlos0 merged 1 commit into
mainfrom
source-set-fix

Conversation

@caarlos0

Copy link
Copy Markdown
Contributor

refs #551
refs #567

it was setting the an empty string as argument to all sourced commands.

@caarlos0 caarlos0 added the bug Something isn't working label Mar 27, 2025
@caarlos0
caarlos0 requested a review from Copilot March 27, 2025 12:34
@caarlos0 caarlos0 self-assigned this Mar 27, 2025
@caarlos0
caarlos0 requested a review from a team as a code owner March 27, 2025 12:34
@caarlos0
caarlos0 requested review from bashbunni and removed request for a team March 27, 2025 12:34

Copilot AI left a comment

Copy link
Copy Markdown

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 addresses an issue where sourced commands were receiving an empty string for their arguments. The fix removes assignments that were mistakenly propagating an empty argument.

  • Removed the assignment of cmd.Args from p.peek.Literal.
  • Removed the assignment of srcCmd.Source to cmd.Args to prevent unintended argument propagation.
Comments suppressed due to low confidence (2)

parser/parser.go:728

  • The removal of this assignment fixes the propagation of an empty argument to sourced commands. Please verify that the intended arguments are being set elsewhere to maintain correct behavior.
cmd.Args = p.peek.Literal

parser/parser.go:735

  • Eliminating this assignment prevents the unintended overwriting of the source value for sourced commands. Confirm that sourced commands receive the correct source information from another source in the code.
srcCmd.Source = cmd.Args

This was referenced Mar 27, 2025
@abbra

abbra commented Mar 27, 2025

Copy link
Copy Markdown

LGTM! I tested it with my scenario and it works!

@caarlos0
caarlos0 merged commit 25d888f into main Mar 27, 2025
@caarlos0
caarlos0 deleted the source-set-fix branch March 27, 2025 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

⌂ 换网址