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

Support fish shell integration automatic injection #139400

Closed
Tyriar opened this issue Dec 17, 2021 · 62 comments · Fixed by #168211, #183005 or #185355
Closed

Support fish shell integration automatic injection #139400

Tyriar opened this issue Dec 17, 2021 · 62 comments · Fixed by #168211, #183005 or #185355
Assignees
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan terminal-shell-fish An issue in the terminal specific to fish, including shell integration terminal-shell-integration Shell integration infrastructure, command decorations, etc.
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Dec 17, 2021

Parent issue #133084


Update: Activate the fish shell integration by following these instructions: https://code.visualstudio.com/docs/terminal/shell-integration#_manual-installation

We're keeping this open until automatic injection is looked into.

@Tyriar Tyriar added plan-item VS Code - planned item for upcoming terminal-shell-integration Shell integration infrastructure, command decorations, etc. labels Dec 17, 2021
@Tyriar Tyriar added this to the January 2022 milestone Dec 17, 2021
@Tyriar Tyriar modified the milestones: January 2022, February 2022 Jan 26, 2022
@Tyriar Tyriar added feature-request Request for new features or functionality and removed plan-item VS Code - planned item for upcoming labels Jan 26, 2022
@Tyriar Tyriar modified the milestones: February 2022, March 2022 Feb 24, 2022
@Tyriar Tyriar modified the milestones: March 2022, Backlog Mar 7, 2022
@Tyriar
Copy link
Member Author

Tyriar commented Mar 7, 2022

Deferring to focus on polishing how shell integration works in general

@meganrogge meganrogge added the help wanted Issues identified as good community contribution opportunities label Mar 8, 2022
@Tyriar Tyriar added the terminal-shell-fish An issue in the terminal specific to fish, including shell integration label Jun 3, 2022
@zgracem
Copy link
Contributor

zgracem commented Aug 5, 2022

Final implementation details aside, fish's event handing will make at least some of this dead easy:

# ~/.config/fish/conf.d/vscode-shellintegration.fish

function __vsc_esc # just a little helper
    builtin printf "\e]633;%s\a" (string join ";" $argv)
end

function __vsc_cmd_before --on-event fish_preexec
    __vsc_esc C
    __vsc_esc E "$argv"
end

function __vsc_cmd_after --on-event fish_postexec
    __vsc_esc D $status
end

function __vsc_update_cwd --on-event fish_prompt
    __vsc_esc P "Cwd=$PWD"
end

Some will be more difficult, at least to deploy auto-magically, because fish's prompts are the output of shell functions, not the contents of variables:

Fish does not use the $PS1, $PS2 and so on variables. Instead the prompt is the output of the fish_prompt function... and the fish_right_prompt function for the right prompt.

So to transparently preserve the user's original prompt, like the bash script does, would require modifying a shell function on-the-fly, not just a variable. Yikes.

That said, I put the above code, plus the following, in my own config—

# ~/.config/fish/conf.d/vscode-shellintegration.fish (cont'd.)

function __vsc_fish_prompt_before; __vsc_esc A; end
function __vsc_fish_prompt_after; __vsc_esc B; end
function __vsc_fish_rprompt_before; __vsc_esc H; end
function __vsc_fish_rprompt_after; __vsc_esc I; end

# ~/.config/fish/functions/fish_prompt.fish

function fish_prompt
    __vsc_fish_prompt_before
    # ...my existing code...
    __vsc_fish_prompt_after
end

# ~/.config/fish/functions/fish_right_prompt.fish

function fish_right_prompt
    __vsc_fish_rprompt_before
    # ...my existing code...
    __vsc_fish_rprompt_after
end

—and I had all of VS Code's documented shell integration features working perfectly with VS Code v1.70.0 + fish 3.5.1.

It's only a proof of concept, but hopefully this saves somebody a few web searches down the line, at least. :)

@Tyriar
Copy link
Member Author

Tyriar commented Aug 5, 2022

@zgracem very handy info thanks! The bash script handles both PS1 and PRONPT_COMMAND, might be able to wrap the function automatically in a similar way?

@zgracem
Copy link
Contributor

zgracem commented Aug 5, 2022

@Tyriar Well, fish doesn't have PROMPT_COMMAND either: if you want something to run right before the prompt displays, you put it in a function with --on-event fish_prompt.

There is, alas, no event handler that runs after displaying the prompt but before accepting input (which is where you'd want CommandStart). There are also no events specific to the right-side prompt only.

FWIW, there's also no PS2 equivalent in fish—"instead it leaves the lines indented to show that the commandline isn’t complete yet"—but the lack of ContinuationStart and ContinuationEnd sequences in the above didn't seem to have any noticeable impact on VS Code's ability to parse the terminal in my testing.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 5, 2022

@zgracem ah I see, I think this stuff should still work though. We actually don't currently use some of these positions and the 633 E sequence kind of removes the need for them all together. So if there are limitations like this we can just take note for the future, like when finalizing the sequences this month.

The limitations imposed on us by the Windows backend are much more frustrating to deal with than this sounds 🥲

If you want to put a PR out with just the script as a starting point (just the script) we could then give it a try and see if we can get it in as an experimental thing. I guess just manually installing until it's considered stable, plus the automatic injection was a handful for the other shells. You could also move the 633 A to just after 633 D, I'd expect that to work fine.

@kidonng
Copy link
Contributor

kidonng commented Aug 5, 2022

I've been wanting to do this since last month, and @zgracem's comment finally motivated me to put up a fish plugin here: https://github.com/kidonng/vscode.fish

So to transparently preserve the user's original prompt, like the bash script does, would require modifying a shell function on-the-fly, not just a variable. Yikes.

The trick I have seen in kitty's automatic shell integration is that they wait for a fish_prompt event and then initialize the code, at which time the prompt function should be ready however they are defined.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 5, 2022

@kidonng FYI those sequences may change as they are yet to be finalized (#155639).

@zgracem
Copy link
Contributor

zgracem commented Aug 5, 2022

@Tyriar Now that @Kiddong has reminded me fish can do e.g. functions --copy fish_prompt _original_fish_prompt (which I just... totally blanked on yesterday 🤦🏼‍♀️), I think I could put together an experimental PR for this—one that could be easily tweaked if/when the escape sequences are finalized.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 5, 2022

Also thanks to a PR from @babakks, the fish history file is now loaded so run recent command pulls from it when fish is active 🎉 #156058

image

@Tyriar
Copy link
Member Author

Tyriar commented Aug 5, 2022

@zgracem that would be fantastic 👍

@meganrogge
Copy link
Contributor

meganrogge commented Jun 1, 2023

IE my expectation was after undoing manual injection, it would work automatically. That wasn't the case.

@meganrogge
Copy link
Contributor

I had misunderstood how this injection works because it is different from that of bash and zsh. Deleting my $XDG_DATA_DIRS in the comment above was the reason I saw it not working. I now see it working.

Screenshot 2023-06-01 at 10 21 23 AM

@meganrogge meganrogge added verified Verification succeeded and removed verification-found Issue verification failed labels Jun 1, 2023
@Tyriar
Copy link
Member Author

Tyriar commented Jun 1, 2023

@roblourens since it's working for @meganrogge and I we'll close this and include it in the release notes. It would be good to get to the bottom of your issue, we can track that in another issue? We can have a call to investigate when you're free if that works.

@roblourens
Copy link
Member

Sure, call any time

@mkhl
Copy link
Contributor

mkhl commented Jun 1, 2023

"XDG_DATA_DIRS":"/home/roblou/snap/code-insiders/1310/.local/share:/home/roblou/snap/code-insiders/1310:/snap/code-insiders/1310/usr/share:/usr/share/xfce4:/usr/local/share:/usr/share:/var/lib/snapd/desktop:/usr/share:/snap/code-insiders/1310/usr/share/code-insiders/resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data"

@mkhl any idea what could be going wrong here wrt the XDG_DATA_DIR auto activation?

Not really, sorry. Maybe the snap packaging interferes somehow?

@roblourens From your shell session, are /snap/code-insiders/1310/usr/share/code-insiders/resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data and /snap/code-insiders/1310/usr/share/code-insiders/resources/app/out/vs/workbench/contrib/terminal/browser/media/fish_xdg_data/fish/vendor_conf.d/vscode-shell-integration.fish present?

@Tyriar
Copy link
Member Author

Tyriar commented Jun 1, 2023

We investigated, it ended up being the fish version being too old. After the update it worked great! Thanks for all the help @mkhl

@mkhl
Copy link
Contributor

mkhl commented Jun 8, 2023

This isn't working for me in 1.79.0 (non-insider), as far as I can see the fish config is there but XDG_DATA_DIRS isn't getting set.

And looking at the code I found #184364 -- what happened?

@Tyriar
Copy link
Member Author

Tyriar commented Jun 9, 2023

@mkhl we had to disable it because it was printing an error when starting up for someone on the team: #184191

For some reason fish_prompt was undefined at the time, it got set later, after our XDG_DATA_DIRS applied.

@Tyriar Tyriar reopened this Jun 9, 2023
@Tyriar Tyriar modified the milestones: May 2023, June 2023 Jun 9, 2023
@Tyriar Tyriar removed verified Verification succeeded help wanted Issues identified as good community contribution opportunities labels Jun 9, 2023
@mkhl
Copy link
Contributor

mkhl commented Jun 15, 2023

Looking at the configuration files order documentation again I believe this would happen when something we need, like fish_prompt, is configured in either config.fish or in a ~/.config/fish/conf.d/*.fish file with a basename that's sorted before the one we expose.

The config.fish file is intentionally read last so people can override vendor configuration, so since we need to run after user configuration maybe providing vendor configuration is just not the way to go here… sorry 😔

Maybe instead we could invoke fish with --init-command="source vscode fish integration here"? That would ensure the integration happens after any configuration but before interactive input.

If I have the spoons and noone beats me to it I'll try to whip up a PR for that next week.

@zgracem
Copy link
Contributor

zgracem commented Jun 15, 2023

One way to make sure something runs after the user's config.fish but before interactive input is to put the necessary code in a self-erasing function tied to the fish_prompt event, e.g.:

function init_vscode_shell_integration --on-event fish_prompt
    # Run only once
    functions --erase init_vscode_shell_integration

    # Do the magic here...
end

This can be placed in the appropriate vendor-config directory as simply (e.g.) init_vscode_shell_integration.fish with no need to worry about sorting order. The function will be defined during startup, executed just before printing the first prompt, then immediately undefined so it doesn't run twice.

@meganrogge
Copy link
Contributor

@zgracem I tried something like that thanks to your suggestion. It's not working atm and not sure why... lmk if you see what could be wrong here

https://github.com/microsoft/vscode/compare/merogge/fish-s?expand=1

@zgracem
Copy link
Contributor

zgracem commented Jun 16, 2023

@meganrogge I think you also need to add --on-event fish_prompt to this function:

function init_vscode_shell_integration
functions --copy fish_prompt __vsc_fish_prompt
functions --erase init_vscode_shell_integration
end

@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 20, 2023
@alexr00
Copy link
Member

alexr00 commented Jun 28, 2023

Removing verification-needed as this was covered by a test plan item.

@alexr00 alexr00 removed the verification-needed Verification of issue is requested label Jun 28, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan terminal-shell-fish An issue in the terminal specific to fish, including shell integration terminal-shell-integration Shell integration infrastructure, command decorations, etc.
Projects
None yet