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

Integrated Terminal - Added support Fish Shell #1908

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

nis-ship-it
Copy link

@nis-ship-it nis-ship-it commented Oct 17, 2024

Description

Add the fish shell to the enums and reading the config.fish only at present.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

image

Add the fish shell to the enums and reading the config.fish only at
present.
Thought installing fish through homebrew would install under /bin/ path
but that's not the case. Add few UI changes to select and test out the
shell though not working at the moment.
Added the shell script to locate the fish config and use it. Had forgot
about it intially, on how we can locate the original fish shell config.
@nis-ship-it nis-ship-it marked this pull request as ready for review October 18, 2024 04:29

var url: String {
switch self {
case .bash:
"/bin/bash"
case .zsh:
"/bin/zsh"
case .fish:
"/opt/homebrew/bin/fish"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nis-ship-it Is this safe to assume that the fish binary will always be installed by homebrew?

Copy link
Author

Choose a reason for hiding this comment

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

Yes it might be different so I will try to figure on how to obtain path

Copy link
Collaborator

Choose a reason for hiding this comment

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

We might look at how other editors handle this

Copy link
Collaborator

@austincondiff austincondiff left a comment

Choose a reason for hiding this comment

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

Leaving a change request review here while you look at the path issues mentioned earlier.

At this stage, we're basically running a process that calls zsh shell to
run which fish and get the path.
@tom-ludwig
Copy link
Member

It would be nice if it defaulted to zsh/bash when Fish isn’t available. A small red message in the terminal like Fish shell not found. Defaulting to bash/zsh would be helpful too, but the priority is making sure it defaults to another shell for now.

@nis-ship-it
Copy link
Author

@tom-ludwig I was thinking also the same switching to zsh since it's by default

@tom-ludwig tom-ludwig marked this pull request as draft October 31, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants