arl / tmux-gitbar

Git in your tmux status bar
GNU General Public License v3.0
170 stars 16 forks source link

Moved tmux-gitbar.conf out of the repo #38

Closed alexkornitzer closed 7 years ago

alexkornitzer commented 7 years ago

Right so the bash version took me about 4 seconds to knock out but the tmux version took ages (I still dont get it!).

So the tmux version will work the same way as before, there are now to variables that can be set:

TMUX_GITBAR_CONF
TMUX_GITBAR_DIR

The bash version is in scripts and can just be used by putting

TMUX_GITBAR_DIR/scripts/setup.sh

in .bashrc

Feel free to remove the bash version, but I just feel that this is much cleaner and as this project is bash dependent I would suggest this may be the better way to go!

I haven't updated the docs yet incase any of this implementation changes during the PR.

Hopefully travis is ready to run the tests.

alexkornitzer commented 7 years ago

Solves #38

alexkornitzer commented 7 years ago

Isnt ready just quite yet have found an overwrite bug... bare with me.

alexkornitzer commented 7 years ago

Right should be okay now if travis doesn't complain.

arl commented 7 years ago

I'm not against having another way to install tmux-gitbar, with .bashrc. Some, as myself, would prefer to not have to edit their .bashrc in order for a third party tool to function, that's why I proposed the all from tmux method, also if installing the $PROMPT_COMMAND from a tmux script requires an awful double escaping, hopefully hidden to the end-user ;-) I believe it's great to propose the method you are implementing, doing the things in .bashrc. Just be sure to write two words about it the README and overall to add some integration tests for that. If you need help with TCL expect, tell me. Thanks again

alexkornitzer commented 7 years ago

Okay so I'll update the README to explain both methods then. While I would prefer the tmux only method it is a tad to hacky for me and if you are happy to have both then I will add to the readme and additional tests.

It also looks like the tests may be failing due to hardcoded path assumptions in the travis tests?

If you hold of on merging this PR, I'll add the readme and extra tests required (if any).

alexkornitzer commented 7 years ago

Yeah looks like it is due to an issue in the tmux-gitbar.tmux

arl commented 7 years ago

Yes, it would be nice to multiplex the integration tests in such a way that the whole suite is tested against both methods (the bashrc and the tmux method)

arl commented 7 years ago

About my last comment, I mean, I don't ask you to code that in this PR. You or me may develop that in a further PR. The only thing i ask, before merging is that the current test suite should pass on travis.

alexkornitzer commented 7 years ago

Yeah that is fair enough, I'll get the current tests and the README done on this one.

arl commented 7 years ago

Ok great I'll have a look at the tests then, tonight or tomorrow when they ll pass I'll merge your PR. Thanks again for the good work

Le 17 nov. 2016 23:39, "Alex Kornitzer" notifications@github.com a écrit :

@AlexKornitzer commented on this pull request.

In tmux-gitbar.tmux https://github.com/aurelien-rainone/tmux-gitbar/pull/38:

@@ -3,15 +3,22 @@

Created by Aurélien Rainone

github.com/aurelien-rainone/tmux-gitbar

-# generate tmux-gitbar.conf if it doesn't exist -if-shell 'test -z ${TMUX_GITBAR_DIR}' \

  • "if-shell '! stat ~/.tmux-gitbar/tmux-gitbar.conf' \
  • 'run-shell \"~/.tmux-gitbar/lib/generate-config.sh ~/.tmux-gitbar/tmux-gitbar.conf\"'" \
  • "if-shell '! stat $TMUX_GITBAR_DIR/tmux-gitbar.conf' \
  • 'run-shell \"$TMUX_GITBAR_DIR/lib/generate-config.sh $TMUX_GITBAR_DIR/tmux-gitbar.conf\"'" +# Set defaults +# FIXME: Why can I only get this to work if I double nest? +if-shell 'test True' \
  • "if-shell 'test -z \"${TMUX_GITBAR_DIR}\"' \
  • 'TMUX_GITBAR_CONF=\"$HOME/.tmux-gitbar.conf\"' \
  • 'TMUX_GITBAR_CONF=\"$TMUX_GITBAR_CONF\"'"

Yeah, just tried it again to double check. Deleted the conf restarted tmux and it's happy.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/aurelien-rainone/tmux-gitbar/pull/38, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdF6hATEa3oROrzI2D0-aaiom9c8kK6ks5q_NeLgaJpZM4K10Ua .

alexkornitzer commented 7 years ago

That seems reasonable, it is working locally but hopefully that isn't due to my distro. If you wouldn't mind investigating my branch that would be great. No worries, really like the project it is very cool.

arl commented 7 years ago

Ok, after having played a bit with your branch, i found 2 things:

.bashrc should not always source tmux-gitbar.sh, it should source only when bash runs inside tmux. tmux-gitbar.sh modifies the $PROMPT_COMMAND, the $PROMPT_COMMAND perform various tmux-specific calls that fail when not run inside tmux. Error messages are printed after every command entered on the terminal... Even if nothing was printed (i.e failing silently), calling update-gitbar is not without cost as it calls git on various occasions so as to obtain the working tree status and compute the tmux status bar string out of it.

Checking if $TMUX does the trick:

if [ -n "$TMUX" ] && [ -f "${HOME}/.tmux-gitbar/tmux-gitbar.sh" ]; then
    source-file "$HOME/.tmux-gitbar/tmux-gitbar.tmux"
fi

There is a naming problem with the env vars in tmux.gitbar.tmux. Probably a copy-paste, because it's the same logic than the one you implemented in tmux-gitbar.sh file, and it works. This is probably the cause of (hopefully all) errors seen on travis. I managed to reproduce the garbage output locally. Could it be that it didn't happen to you because $TMUX_GITBAR_xxx variables were still in your .bashrc, or at least still in your environment, thus hiding the problem?

Anyway, here is the diff. I didn't have the time to create another branch and to PR your PR :roll_eyes: :laughing: so don't know what travis will say about that, but on my machine it works.

diff.txt

I'll let you update your PR

alexkornitzer commented 7 years ago

Ah awesome, thanks for doing that. I swear I had tmux-gitbar.tmux looking like that at one stage and it just wasn't working, clearly I had been staring at the computer too long. As it is looking super elegant and not hacky, I have removed the bash version as that is just additional stuff that would have to be maintained :) Apologies for not getting it right first time.

Not sure about the failing left right test, I have tried those manually and they are fine but again that isn't saying much with my previous attempts 🙄

arl commented 7 years ago

Ok, by looking at the new failed Travis tests, we see that the garbage output test pass, that is a good thing! Still some failures, but I don't think they are due to the PR. I'll have a look at that later on.

Yeah the run-shell, if-shell commands... it can get really hairy. Do not think that I got it right at the first time!

If you feel to remove the bashrc version, up to you. I would not have used it as long as it can all be done in tmux.conf.

So the important point to be added to the README is that, now, tmgb.conf is by default in $HOME, and that one can have it anywhere else by setting TMUX_GITBAR_CONF in tmux.conf. If you didn't do it, don't worry, I will, after merging, as this PR is a semi-breaking change, and I will bump the minor version tag.

Le 18 nov. 2016 08:08, "Alex Kornitzer" notifications@github.com a écrit :

Ah awesome, thanks for doing that. I swear I had tmux-gitbar.tmux looking like that at one stage and it just wasn't working, clearly I had been staring at the computer too long. As it is looking super elegant and not hacky, I have removed the bash version as that is just additional stuff that would have to be maintained :) Apologies for not getting it right first time (v. annoyed with myself).

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub, or mute the thread.

alexkornitzer commented 7 years ago

With the new and clean tmux one I was not going to use the bash one either :P I updated the README with how to change the default location.

arl commented 7 years ago

Hi, when and if you accept my PR on your fork (AlexKornitzer/tmux-gitbar#1), I'll merge this.

alexkornitzer commented 7 years ago

Done :)

arl commented 7 years ago

fingers crossed ;-)

alexkornitzer commented 7 years ago

Success 👍

arl commented 7 years ago

we like our friend travis when he's green!