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

Compatible with a fish plugin manager #116

Closed
5 of 10 tasks
folliehiyuki opened this issue Oct 9, 2020 · 21 comments
Closed
5 of 10 tasks

Compatible with a fish plugin manager #116

folliehiyuki opened this issue Oct 9, 2020 · 21 comments
Assignees
Labels

Comments

@folliehiyuki
Copy link
Contributor

Check list

  • I have read through the README
  • I have the latest version of forgit
  • I have searched through the existing issues

Environment info

  • OS
    • Linux
    • Mac OS X
    • Windows
    • Others:
  • Shell
    • bash
    • zsh
    • fish

Problem / Steps to reproduce

Hi. Thanks for a nice shell plugin.
I have some suggestions: maybe moving forgit.plugin.fish into a directory named conf.d so that it can be easily installed,updated and autoloaded with a plugin manager like fisher or oh my fish. And the file doesn't need to be executable.

@cjappl
Copy link
Collaborator

cjappl commented Oct 9, 2020

Hi @AoHiyuki. This is a great idea.

Unfortunately I don't have a lot of experience using or developing fish plugins for these systems. Do you have any experience? If so, please feel free to do a PR on this!

Otherwise it may be a little bit while I figure out how to make it work :)

@folliehiyuki
Copy link
Contributor Author

Hi @cjappl. I made it working with Fisher. Fairly simple as I described.
As I'm not familiar with Oh My Fish, it may take a while for me to figure out.
1 question though: do you mind if I separate the plugin into autoload and functions, since issue #110 ?

@wfxr
Copy link
Owner

wfxr commented Oct 10, 2020

@AoHiyuki The main reason I object to splitting forgit.plugin.zsh is that it makes it difficult for zsh and bash to share code, or you have to do a lot of dirty work. This however will not make the plugin easier to use or maintain.

But for the fish version, it is not compatible with zsh or bash from the beginning (As you can see, it is a translation of the bash/zsh version, not a symbol link). Therefore, I am not opposed to splitting it. @cjappl is responsible for maintaining the fish version. You can freely discuss solutions with him. @AoHiyuki

@wfxr
Copy link
Owner

wfxr commented Oct 13, 2020

fisher support has been implemented. I think we can close this issue now @cjappl

@cjappl
Copy link
Collaborator

cjappl commented Oct 13, 2020

Sounds good.

Thanks for the good work @wfxr and @AoHiyuki

@cjappl cjappl closed this as completed Oct 13, 2020
@folliehiyuki
Copy link
Contributor Author

I just caught a minor issue. Didn't check things out thoroughly :(
forgit.plugin.fish appears in 2 places (even if it is just a symlink). So when installed with Fisher it will be copied to both conf.d and functions (the file in conf.d to ~/.config/fish/conf.d and the main file to ~/.config/fish/functions)
It works with no problems, but fish now will register a function named forgit.plugin (that doesn't do anything) since the file is there in the functions directory.

Only 1 minor issue @cjappl , but it doesn't affect anything badly.

@cjappl
Copy link
Collaborator

cjappl commented Oct 13, 2020 via email

@folliehiyuki
Copy link
Contributor Author

folliehiyuki commented Oct 14, 2020

I would only keep the file inside conf.d directory and delete the other one @cjappl . In case you do that we should ask @wfxr to generate a new short link as well, since it still points at the file in root directory of the repo.

@wfxr
Copy link
Owner

wfxr commented Oct 14, 2020

@AoHiyuki I used symbol link to reuse the shorten url.

fisher's mechanism for loading plugins seems a bit special. I wonder if it support the plugin placed in a subdirectory? Like this layout:

.
├── fish
│  └── conf.d
│     └── forgit.plugin.fish
├── forgit.plugin.sh -> forgit.plugin.zsh
├── forgit.plugin.zsh
├── LICENSE
└── README.md

EDIT
I looked into it and it looks unlikely. Let's replace the symbol link and regenerate a shorten URL.

@wfxr
Copy link
Owner

wfxr commented Oct 14, 2020

@AoHiyuki I replaced the link with actual file. You can update and test if it works fine. If everything is ok I will create a new shorten url for conf.d/forgit.plugin.fish and update readme.

BTW now forgit's project layout seems already compatible with omf:

omf install https://github.com/wfxr/forgit

@cjappl Would you like to submit a PR to this repo so that users can install forgit by using name instead of full url?

@folliehiyuki
Copy link
Contributor Author

folliehiyuki commented Oct 14, 2020

@wfxr yeah it works fine. Thanks for the quick fix.

Both Fisher and OMF utilize the conf.d directory, so we can already install forgit with either one.

The plugin doesn't work when installed with omf yet, as I mentioned. The file is sourced, but not everything will be "transported" to the shell.

I've only moved things around. It works flawlessly with Fisher but has some problems with OhMyFish. Local variables like set forgit_pager "$FORGIT_PAGER" aren't sourced by OhMyFish.
We can work around this by making them global, or create a symlink in install hook. I will address this in OhMyFish, since I don't know whether it is a bug.

@wfxr
Copy link
Owner

wfxr commented Oct 14, 2020

Got it. @AoHiyuki

@wfxr
Copy link
Owner

wfxr commented Oct 14, 2020

@AoHiyuki Changed all variables which should be global to global. I think this is the rightway to solve the problem.

@folliehiyuki
Copy link
Contributor Author

folliehiyuki commented Oct 14, 2020

I just installed OMF to test it and can confirm that it works. You can update README for both plugin managers now @wfxr . I didn't make the change earlier since I wasn't sure enough whether making them global could break things.

wfxr added a commit that referenced this issue Oct 14, 2020
@wfxr
Copy link
Owner

wfxr commented Oct 14, 2020

Updated url from git.io/forgit.fish to git.io/forgit-fish because as of Sep 2020 we can no longer request git.io outdated or incorrect shortened URLs be released by GitHub Support.

https://github.blog/changelog/2020-09-23-git-io-urls-no-longer-released-by-github-support/

@cjappl
Copy link
Collaborator

cjappl commented Oct 14, 2020

Requested that we be added to the public repo:
oh-my-fish/packages-main#126

@cjappl
Copy link
Collaborator

cjappl commented Oct 14, 2020

Will close when we are in there.

@stale
Copy link

stale bot commented Mar 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 26, 2021
@wfxr wfxr removed the stale label Mar 26, 2021
@stale
Copy link

stale bot commented May 28, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 28, 2021
@cjappl cjappl removed the stale label Jun 2, 2021
@stale
Copy link

stale bot commented Aug 1, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 1, 2021
@cjappl
Copy link
Collaborator

cjappl commented Aug 1, 2021

I'm just going to close this. We have done what we can and now we are waiting for an (extremely slow) external dependency.

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

No branches or pull requests

3 participants