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

Allow switch with zero arguments #5677

Closed
floam opened this issue Feb 19, 2019 · 8 comments
Closed

Allow switch with zero arguments #5677

floam opened this issue Feb 19, 2019 · 8 comments

Comments

@floam
Copy link
Member

floam commented Feb 19, 2019

I think one should be able to switch on a variable that may or may not be set:

switch $LANG
    case 'en_US*'
...
end

If $LANG is unset:

fish: switch: Expected exactly one argument, got 0

(Same with command substitutions which may sometimes print nothing.)

I think it should just silently short circuit and do nothing.

@floam
Copy link
Member Author

floam commented Feb 19, 2019

It will work if the string is quoted. The command substitution situation might be a better example.

@floam
Copy link
Member Author

floam commented Feb 19, 2019

Here's an example of a user being bitten:

from __fish_complete_mount_opts.fish:

    switch (string replace -r '=.*' '=' -- $last_arg)
        case uid=
            set -a fish_mount_opts uid=(__fish_print_user_ids)
        case gid=
            set -a fish_mount_opts gid=(__fish_print_group_ids)
        case setuid=
            set -a fish_mount_opts setuid=(__fish_print_user_ids)
        case setgid=
            set -a fish_mount_opts setgid=(__fish_print_group_ids)
    end

When there are no replacements done, this will cause an unexpected error. (mount -o <TAB> does it here.)

@faho
Copy link
Member

faho commented Feb 20, 2019

When there are no replacements done, this will cause an unexpected error.

Not the one you're thinking of.

Because this uses string replace -r without -f/--filter. So if no replacement is done to a given argument, it is printed as-is.

That means if $last_arg has one element, it'll check it, and because it has no "=" (because otherwise it would have done a replacement) it won't match any case and nothing will happen. Which might or might not be what you want.

If $last_arg is empty, it's equivalent to switch $last_arg, and if it has multiple args #3042.


Anyway, I agree that switch with something that happens to expand to zero arguments shouldn't error. The question is what case it should match. Since an empty quoted list expands to the empty string, it would logically match case '*' or case ''. But that might make the user think that it's the empty string.

E.g.

switch $empty
    case ''
       # oh cool, it's the empty string!
       echo banana$empty # prints nothing!

An alternative is to not let it match anything. That would be the most consistent, and would allow you to quote it to let it match something.

Then there's the question of what happens to zero arguments - should just

switch
    case '*'

continue to error? Probably yes, but if that prevents us from allowing empty expansions it seems worth it to allow it as well.


I actually think whatever we do here isn't backwards-incompatible - we're removing an error, so any code that used this was broken.

@floam
Copy link
Member Author

floam commented Feb 20, 2019

I was thinking maybe, a case with zero arguments?

case and case (false) does not error.

@floam
Copy link
Member Author

floam commented Feb 20, 2019

Or, if that's just too weird, I think it's also not terrible for no case to be able to match at all, the switch does nothing. Maybe you could pick up on that with switch ... end; or ...?

@ridiculousfish
Copy link
Member

My first reaction is that case '*' is sufficient. You wouldn't normally write a switch statement that deliberately matches zero arguments, so it's sufficient to group this into an error handling catch-all glob. If you expect to sometimes get zero arguments it's easy to handle this outside of the switch or inside of the case.

@mqudsi
Copy link
Contributor

mqudsi commented Apr 1, 2019

I'm ok with what @ridiculousfish said so long as switch with zero literal arguments remains a syntax error.

@faho
Copy link
Member

faho commented Jul 31, 2019

Yeah, I allowed things that expand to nothing, while keeping switch as a syntax error.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants