Closed goodevilgenius closed 2 years ago
@goodevilgenius Thanks for your suggestion! This would be a nice extension for forgit
IMO. Since I'm a bit busy lately, I would be glad to accept the pr if you are willing to contribute.
@wfxr if I can find the time to clean up my script and provide an easy way to install, I'll put together a PR.
Although if someone else comes along with more time they can feel free to use my script as a starting point.
@goodevilgenius Thank you.
Although if someone else comes along with more time they can feel free to use my script as a starting point.
I added a link to this issue in reademe to help others find this solution.
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.
Still valid. Not stale.
Oh, hi. I came here to file another issue, but I have a small wrapper script I wrote in my dotfiles that does exactly this. I'd be happy to clean it up and send over a PR if the OP isn't working on this anymore (I don't want to step on anyone's toes).
@wren I would say if you get to it first, it's all yours! We haven't heard from the OP in a few months
@wren I'm not. Feel free.
I never found the time to get it together.
Okay, cool. I'm busy the next two weeks, but I should have something for this by the end of the month.
Hi! I'm back now (a bit later than expected), and I just wanted to say that I haven't forgotten about this, and should be sending a PR over soon.
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.
@wren still working on this, or no?
@wren still working on this, or no?
Some personal stuff came up at the time, and I completely forgot about this. Sorry about that! I actually have some time tonight to work on this, so I'll post here about how it goes.
Ok, so I took a crack at it, and I need a decision from the project about the specific approach. The requirements as I see them are:
$PATH
The way I see it, there are a few different ways to accomplish this. They're all about the same amount of effort, but have some subtle differences. And it mostly seems to me to be a matter of preference, which is why I'm asking for guidance from the project. The options I see are:
bin
directory to the repo, and have forgit.plugin.zsh
add that directory to the path when the plugin is sourced (probably controlled by an environment variable, so it doesn't annoy users that don't want it)
forgit::install::bin
or whatever, or an environment variable)
$PATH
is requiredbin
directory, and then rely on the plugin manager to handle updating the path and/or the installation to a system bin directory
Anyway, I'm happy to implement any one of these (or some other option). Just let me know what you prefer, @wfxr, and I can finish this up and send over a PR.
@wren Sorry for the late reply. I'm too busy recently. And Thank you for your efforts. I just have some questions:
@wfxr No worries bout the time.
Have you considered the compatibility for bash, zsh and fish ? Yes, I mention above that option one "... is usually different for each shell, so extra handling is required." It wasn't super clear but the "each shell" part was referring to bash, zsh, and fish. Some of the options affect shells differently, and some don't.
What are the major flaws in the original solution contributed by @goodevilgenius ? I think it is elegant, involves fewer changes, and do not make users who don’t use this feature pay ?
The solution I have is essentially the same as the one by @goodevilgenius. The only issue is around this line:
. /path/to/forgit/forgit.plugin.zsh
Unless the path to forgit is hard-coded, then there will have to be some extra steps for the user to be able to use this new file. So, all of the options above are essentially different approaches to handle this one line (which is required for use in git). To be clear, though, none of the options above would have any additional performance cost for users that don't want this new feature (other than maybe the time it takes to check an environment variable, which is very minimal).
If you're looking for an option that has the absolute least amount of code changes, then having this new file read the forgit path from a manually set environment variable is an option. In this case, for users that want to have this, the installation would look like this:
$FORGIT_INSTALL_DIR
or whatever to that install directory, so that the . /path/to/forgit/forgit.plugin.zsh
line above can work$FORGIT_INSTALL_DIR/bin
to $PATH
So, if the manual way is what you prefer, I can certainly implement it that way. I was just thinking it would be useful if users didn't have to do the extra steps if possible.
How would you like me to proceed?
@wren Thank you for the detailed explanation.
Unless the path to forgit is hard-coded, then there will have to be some extra steps for the user to be able to use this new file. So, all of the options above are essentially different approaches to handle this one line (which is required for use in git).
What about adding the following line into forgit.plugin.zsh
?
export FORGIT_INSTALL_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
Then we should be able to access $FORGIT_INSTALL_DIR
in the git sub-commands script:
source "$FORGIT_INSTALL_DIR/forgit.plugin.zsh"
@wren Thank you for the detailed explanation.
Happy to help. :)
What about adding the following line into
forgit.plugin.zsh
?export FORGIT_INSTALL_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
Then we should be able to access
$FORGIT_INSTALL_DIR
in the git sub-commands script:source "$FORGIT_INSTALL_DIR/forgit.plugin.zsh"
Yeah, I think something like this would work. It's what I was trying to explain for Option 1 above (potentially with some extra handling because I don't know if $BASH_SOURCE
works on zsh and fish). Looking back at it, I think the options above weren't as clear as I could have made them. Sorry about that!
So, the two steps that need extra handling (numbers), and their possible solutions (letters):
bin/git-forgit
to the path
a. User manually sets their own path (and might be able to use an env var set by one of the options above)
b. The plugin manager adds the script to the user's path
c. Forgit adds to the path (either when forgit is loaded, when a particular env var is set, or have a separate function/command/alias to do it when requested)
d. Forgit copies/symlinks the script into an existing directory on the path (either when forgit is loaded, when a particular env var is set, or have a separate function/command/alias to do it when requested)So, as an example, 1b and 2c would look something like this:
# forgit.plugin.zsh
if [[ -z "$FORGIT_NO_STANDALONE" ]]; then
# the below line might need zsh & fish specific handling
export FORGIT_INSTALL_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
export PATH="${PATH}:${FORGIT_INSTALL_DIR}/bin"
fi
# bin/git-forgit
source "${FORGIT_INSTALL_DIR}/forgit.plugin.zsh"
cmd="$1"
shift
forgit::${cmd/-/::} "$@"
What do you think? Do prefer one of the other options, or should I just go with this example?
@wren Thank you!
potentially with some extra handling because I don't know if $BASH_SOURCE works on zsh and fish
You are right. We need different ways to find the installation directory for different shells:
# in forgit.plugin.zsh
test -n "$BASH" && export FORGIT_INSTALL_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
test -n "$ZSH_NAME" && export FORGIT_INSTALL_DIR=$(dirname "${(%):-%x}")
# in forgit.plugin.fish (unverified)
set -x FORGIT_INSTALL_DIR (cd (dirname (status -f)); and pwd)
What do you think? Do prefer one of the other options, or should I just go with this example?
I prefer 1b and 2b because users may not expected to see the PATH
env being modified while sourcing a plugin.
I prefer 1b and 2b because users may not expected to see the
PATH
env being modified while sourcing a plugin.
@wfxr Ok, sounds good. I'll code this up and send over a PR this weekend. 👍
@wfxr I just opened the PR. I'm happy to make any changes as needed, so just let me know what you think.
I think this would be pretty useful if it was possible to use as a sub-command of
git
, rather than as shell aliases.Check list
Environment info
Suggested solution
I've put together a shell script that does this, but a little better integration might be better, as well as bash completion. I name this
git-forgit
, and put it in my path, and I can do, for example,git forgit log
, instead offorgit::log
, orglo