Skip to content
This repository has been archived by the owner on Jun 23, 2020. It is now read-only.

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

Merged
merged 6 commits into from
Nov 20, 2016
Merged

Conversation

alexkornitzer
Copy link
Contributor

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
Copy link
Contributor Author

Solves #38

@alexkornitzer
Copy link
Contributor Author

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

@alexkornitzer
Copy link
Contributor Author

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

@arl
Copy link
Owner

arl commented Nov 17, 2016

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
Copy link
Contributor Author

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).

# Load the config file
load_config() {
source "${SCRIPT_DIR}/../tmux-gitbar.conf"
source "${TMUX_GITBAR_CONF}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems the no_extra_output.tcl integration test fails because the config file is not found, and the string /home/travis/.tmux-gitbar.conf: No such file or directory\r\,amongst others, are produced when $PROMPT_COMMAND is invoked.

@alexkornitzer
Copy link
Contributor Author

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

@arl
Copy link
Owner

arl commented Nov 17, 2016

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
Copy link
Owner

arl commented Nov 17, 2016

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
Copy link
Contributor Author

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

if-shell 'test True' \
"if-shell 'test -z \"${TMUX_GITBAR_DIR}\"' \
'TMUX_GITBAR_CONF=\"$HOME/.tmux-gitbar.conf\"' \
'TMUX_GITBAR_CONF=\"$TMUX_GITBAR_CONF\"'"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pseudo code i have in mind for tmux-gitbar.tmux too bad it involves a new level of nesting... :

""" 1st step: generate configuration """
if "$tmgb_conf" env var is set:
    if "$tmgb_conf" file not exist:
        if "$tmgb_dir" envvar is set:   
            "$tmgb_dir/lib/generate_config.sh" $(tmgb_conf)
        else:
            "$home/.tmgb/lib/generate_config.sh" $(tmgb_conf)
else:
    if "$tmgb_dir" envvar is set:   
        "$tmgb_dir/lib/generate_config.sh" ~/.tmgb.conf
    else:
        "$home/.tmgb/lib/generate_config.sh" ~/.tmgb.conf

""" 2nd step: install prompt_command
(THIS SHOULDNT BE DIFFERENT THAN CURRENT VERSION"""
if update-gitbar not in $prompt_cmd:
    if "$tmgb_dir" is set:
        "$prompt_cmd" += "$tmgb_dir/update-gitbar"
    else:
        "$prompt_cmd" += "~/.tmgb/update-gitbar"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it may have to end up looking like that as the tests still seem to be failing. Its running fine on my local machine which is odd.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, you mean the tmux method works on your machine? Even after having killed the tmux server in order to clear the tmux env?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to have more time tonight, but if that's only a question of tests not passing while all is working locally, i'm gonna fix the tests myself after pulling your branch. When travis will be happy i'm gonna merge

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@arl
Copy link
Owner

arl commented Nov 17, 2016

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
#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
#38, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAdF6hATEa3oROrzI2D0-aaiom9c8kK6ks5q_NeLgaJpZM4K10Ua
.

@alexkornitzer
Copy link
Contributor Author

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
Copy link
Owner

arl commented Nov 18, 2016

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

  • .bashrc method

.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
  • tmux method:

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 🙄 😆 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
Copy link
Contributor Author

alexkornitzer commented Nov 18, 2016

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 🙄

The new default location for the conf file is ~. This can be changed
using an envar if required. The README has also been updated to relfect
the changes.
@arl
Copy link
Owner

arl commented Nov 18, 2016

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
Copy link
Contributor Author

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
Copy link
Owner

arl commented Nov 20, 2016

Hi,
when and if you accept my PR on your fork (alexkornitzer#1), I'll merge this.

@alexkornitzer
Copy link
Contributor Author

Done :)

@arl
Copy link
Owner

arl commented Nov 20, 2016

fingers crossed ;-)

@arl arl merged commit 3e0cf93 into arl:master Nov 20, 2016
@alexkornitzer
Copy link
Contributor Author

Success 👍

@arl
Copy link
Owner

arl commented Nov 20, 2016

we like our friend travis when he's green!

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

Successfully merging this pull request may close these issues.

None yet

2 participants