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

set -- mistakenly outputs shell variables and shell functions #339

Closed
ko1nksm opened this issue Jan 21, 2025 · 4 comments · Fixed by #354 or #356
Closed

set -- mistakenly outputs shell variables and shell functions #339

ko1nksm opened this issue Jan 21, 2025 · 4 comments · Fixed by #354 or #356
Assignees
Labels
bug Something isn't working

Comments

@ko1nksm
Copy link

ko1nksm commented Jan 21, 2025

set -- must clear positional parameters.

$ brush --version
brush 0.2.14 (git:dd1d39c)

$ brush -c 'set --'
BASH=brush
BASH_VERSINFO=([0]="5" [1]="2" [2]="15" [3]="1" [4]="release" [5]="unknown")
BASH_VERSION=5.2.15(1)-release
COMP_WORDBREAKS='


$ bash -c 'set --'
(nothing)
@reubeno
Copy link
Owner

reubeno commented Jan 22, 2025

Thanks for filing this issue. This one may be tricky, as we're relying on the clap-rs command-line parser to parse arguments for built-in commands; unfortunately, I don't think there's any way for our code to distinguish between set and set --.

We'll need to think on this for a bit.

(For posterity, note that these two command-lines also have different side effects; set does not reset $1, etc. but set -- does.)

@reubeno reubeno added the bug Something isn't working label Jan 22, 2025
@reubeno reubeno self-assigned this Jan 22, 2025
@reubeno
Copy link
Owner

reubeno commented Jan 22, 2025

Thanks for reporting this issue!

After some thought (and investigation), it looks like we can use the same workaround that had been done for echo in #147. I've submitted a PR with a proposed fix.

@ko1nksm
Copy link
Author

ko1nksm commented Jan 23, 2025

Apparently, a small issue remains.

$ brush --version
brush 0.2.14 (git:cf45a9f)

$  brush -c 'set -- a ${1:-+}; echo "$@"'
a --+

$ bash -c 'set -- a ${1:-+}; echo "$@"'
a +

FYI. This bug was accidentally discovered because the 63rd customizable character in base64 is +.
https://github.com/ko1nksm-shlab/sh-base64/blob/17cece824649d60962bed3bea44203d266d85ebd/base64.sh#L10

@reubeno
Copy link
Owner

reubeno commented Jan 23, 2025

You're right! We have some logic to work around clap-rs's limitations in supporting + options. It sounds like that workaround is incorrectly getting applied to the positional arguments.

I'll reactivate this issue and look at addressing this. Thanks again for the clear details.

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
2 participants