Skip to content

Conversation

@davedash
Copy link
Contributor

@davedash davedash commented Aug 8, 2023

… arguments

In some cases in VSCode + fish you can run a command like:

 echo 'hi     
       -- hi'

It's perfectly legitimate and will run in a normal terminal just fine.

However VSCode's shell extensions will raise an error:

string join: -- hi': unknown option

/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data/fish/vendor_conf.d/shellIntegration.fish (line 1): 
string join ";" $argv
^
in command substitution
        called on line 65 of file /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data/fish/vendor_conf.d/shellIntegration.fish
in function '__vsc_esc' with arguments 'E echo\ \'hi "" --\ hi\''
        called on line 72 of file /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data/fish/vendor_conf.d/shellIntegration.fish
in function '__vsc_cmd_executed' with arguments 'echo\ \'hi\n\n--\ hi\''
in event handler: handler for generic event 'fish_preexec'

(Type 'help string' for related documentation)

The problem is string interprets the --hi as a switch. By adding -- we can negate that behavior and let --hi be treated as a positional argument.

See the string documentation

Arguments beginning with - are normally interpreted as switches; -- causes the following arguments not to be treated as switches even if they begin with -. Switches and required arguments are recognized only on the command line.

See: #187248

This is my first pull request to this repository, and I am happy to amend, update, as needed. To the best of my knowledge I've satisfied the guidelines.

… arguments

In some cases in VSCode + fish you can run a command like:

```
 echo 'hi                                                                                                               
                -- hi'
```

It's perfectly legitimate and will run in a normal terminal just fine.

However VSCode's shell extensions will raise an error:

```
string join: -- hi': unknown option

/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data/fish/vendor_conf.d/shellIntegration.fish (line 1): 
string join ";" $argv
^
in command substitution
        called on line 65 of file /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data/fish/vendor_conf.d/shellIntegration.fish
in function '__vsc_esc' with arguments 'E echo\ \'hi "" --\ hi\''
        called on line 72 of file /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data/fish/vendor_conf.d/shellIntegration.fish
in function '__vsc_cmd_executed' with arguments 'echo\ \'hi\n\n--\ hi\''
in event handler: handler for generic event 'fish_preexec'

(Type 'help string' for related documentation)
```

The problem is `string` interprets the `--hi` as a switch.  By adding `--` we can negate that behavior and let `--hi` be treated as a positional argument.

See the [string documentation][s]

> Arguments beginning with - are normally interpreted as switches; -- causes the following arguments not to be treated as switches even if they begin with -. Switches and required arguments are recognized only on the command line.

[s]: https://fishshell.com/docs/current/cmds/string.html
@sbatten sbatten assigned Tyriar and unassigned sbatten Aug 9, 2023
@Tyriar Tyriar added this to the August 2023 milestone Aug 9, 2023
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Looks good to me provided it doesn't cause an error for single/multi-line commands. @meganrogge could you test in fish on your mac to make sure it works?

@Tyriar Tyriar requested a review from meganrogge August 9, 2023 19:43
@meganrogge
Copy link
Contributor

works 👍🏼

Screenshot 2023-08-09 at 2 02 36 PM

Copy link
Contributor

@meganrogge meganrogge left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

@meganrogge meganrogge enabled auto-merge August 9, 2023 21:03
@meganrogge meganrogge merged commit 36fa6d4 into microsoft:main Aug 9, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants