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 for fish shell #59

Closed
frewsxcv opened this issue Feb 25, 2014 · 7 comments
Closed

Support for fish shell #59

frewsxcv opened this issue Feb 25, 2014 · 7 comments

Comments

@frewsxcv
Copy link

No description provided.

@jmsbtlr111
Copy link

+1

1 similar comment
@atourkow
Copy link

+1

@dasJ
Copy link
Contributor

dasJ commented Aug 5, 2016

Fish doesn't really care about POSIX compatiblity. This is going to be hard....

@dasJ dasJ added this to the 0.3.0 milestone Aug 6, 2016
@dasJ dasJ added the new-shell label Aug 6, 2016
@Miguel-Alonso
Copy link

Miguel-Alonso commented Aug 16, 2016

Hey, I actually started working on this (ahem, a month ago, wanted to create a pull request but got distracted). Now, I'm not a fish nor bash expert, but the tests pass and the only thing I was having trouble with was exposing AUTOENV_CUR_FILE and AUTOENV_CUR_DIR.

I have my code at https://github.com/loopbit/autoenv, all my changes are in two new files, activate.fish and tests/simple_comment_test.fish
Let me have a look to see if I'm missing something else and I'll do a PR.

@dasJ dasJ modified the milestones: 0.4.0, 0.3.0 Aug 26, 2016
@dasJ
Copy link
Contributor

dasJ commented Sep 7, 2016

@Miguel-Alonso Thank you very much for the contribution!

I knew fish syntax was different but I thought it was at least mostly compatible with bash/sh. The work of your PR shows me otherwise: Instead, they tried to reinvent the shell.
I don't want to start an argument if this is a good idea or a dumb idea, but I don't think I can ever merge this PR.
Please don't get me wrong, your work seems to be just what it takes to bring the autoenv magic to fish. But the fish part is completely separated from the rest of autoenv so it's basically two projects in one repository and I don't really want to maintain both (testing when bugs are reported, implementing new features twice, ...).

So instead, it would be much wiser to leave the fish part out of autoenv. You could create a new repository from scratch without the bash/dash code and we could link to each other in the readmes.
As you can see with #129 and #130 I plan to change a lot of things in autoenv and it would be great if we could keep compatibility. Please let me know if you are interested in this kind of collaboration or if you have a better idea 👍

@dasJ dasJ removed this from the 0.4.0 milestone Sep 7, 2016
@dasJ dasJ added the waiting label Sep 7, 2016
@Miguel-Alonso
Copy link

No problem at all, what you say makes total sense (and yeah, the decision to rewrite fish from scratch is weird, to say the least, but it is a nice shell).

I've just created a new repository at loopbit/autoenv_fish to keep the fish stuff. I'll make it clear in the README that it is just a translation of this project and, probably, a few versions behind the original 😄.

As for moving forward, what I'll probably do is to keep an eye on your tests and base my work on that. Does that sound good to you?

@dasJ dasJ closed this as completed in 981772f Sep 7, 2016
@dasJ dasJ removed the waiting label Sep 7, 2016
@dasJ
Copy link
Contributor

dasJ commented Sep 7, 2016

@Miguel-Alonso As you can tell from my commit: it sounds perfect to me. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants