-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add gitstatus backend for git segment #104
base: master
Are you sure you want to change the base?
Conversation
da7aa73
to
6530e2f
Compare
sbp.bash
Outdated
|
||
PS1=$(bash "${SBP_PATH}/src/main.bash" "$command_status" "$command_duration") | ||
# gitstatus.plugin.sh requires an interactive shell | ||
PS1=$(bash --noediting --noprofile --norc -i "${SBP_PATH}/src/main.bash" "$command_status" "$command_duration" "$GITSTATUS_DIR" "$GITSTATUS_DAEMON_PID" "$_GITSTATUS_REQ_FD" "$_GITSTATUS_RESP_FD") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you verify that this is the case? I can't see the rationale for this looking at the repository at least as the gitstatus binary is doing the actual work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look at the script you can see the same return that the default .bashrc
has.
If we would vendor the script we could remove the check and remove this giant workaround. Not sure what to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah right. Would we be able to limit the interactive shell to just the invocation of the gitplugin that we are doing in the segment script though? I'm thinking that this could probably be a separate segment script, as it's using a completely different approach than the regular git segment. Does that make sense to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. We can't easily forward all variables we need without explicitly listing all of them.
Line 46 in f5d0b29
source "$segment_script" |
I wouldn't make this a different segment because it is still sharing some of the code.
This is a difficult one, I really like the work romkatv has done on getting the speed increase for git status. Which is by far the slowest segment in SBP. In the current suggestion though I think we are taking on too much responsibility for the gitstatus plugin. Would it be possible to instead link to instructions to setup the deamon, and have the segment use it directly? Or could we then query the daemon directly based on environment variables in the shell? |
🤔 I should have thought about that earlier. Instruct people to put it into their bashrc and we would make a lot of stuff easier and they can use it with other scripts. I try to do that and test it over the weekend. This would also mean we could remove the setting because if the PID variable of gitstatus is present we assume it should be used. This also solves the config loading issue. This is probably the way to go. Thanks for the idea!! |
Cool! 👍 |
2cb8ee6
to
8b71c4c
Compare
ea66bfb
to
9dbb232
Compare
@brujoand I tested this over the last weeks and I finally got the history fully working like it should. Also sometimes gitstatus does not show as much information as git-status. I think this is only the case if the remote branch does not exist but I am not sure if this a limitation of gitstatus or my code. Anyway I don't think this is a blocker right now. |
9dbb232
to
d87aa84
Compare
d87aa84
to
d5e34f6
Compare
@@ -48,6 +48,21 @@ truncation/compacting for that specific segment. | |||
shows the git branch and current status, set ´SEGMENTS_GIT_BRANCH_ONLY=true´ | |||
to speed up execution on large repos. The default value is ´false´. | |||
|
|||
Enother option to speed up execution is to use gitstatus as a backend. | |||
Install gitstatus and then add the following to your .bashrc_: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still require sbp to start an interactive shell with this approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d5e34f6
to
e10489a
Compare
Apologies again for taking forever to get back to you on this one. It really bugged me that gitstatus would have to become so interconnected into sbp to achieve this. And looking through the bash code it uses, it's quite extensive and not trivial to understand. At the same time, I've been working on a monorepo the last couple of years and the git segement has been really really slow. But today I discovered this https://github.blog/2022-06-29-improve-git-monorepo-performance-with-a-file-system-monitor/ By enabling untrackedcache and fsmonitor the git segment execution moved from 160ms to 45ms. Which is pretty much the same results I had with gitstatus.
This would be much easier for people to use, it's straight from git and doesn't require us to use an interactive shell. So I think this is the correct approach to solve this. |
I have untrackedcache enabled through feature.manyFiles since a long time but the fsmonitor is not available on linux and the alternative hook daemons I have tried before where all either not really faster or leaked hundreds of megabytes of RAM over a short amount of time per open shell.
|
gitstatus is faster especially on large repositories with many changes. This is an alternative to untracked cache and fsmonitor.
e10489a
to
f430804
Compare
Yeah, I’ve been using the fs-monitor for a while now on osx, and I’m not super happy with the performance. |
This PR adds gitstatus as an backend for the git segment. On my local copy of nixpkgs this improves the speed of the segment drastically. With normal git it takes ~58ms for the segment and with gitstatus only ~5 ms like their readme advertises.
I am happy to integrate feedback.