Fixes bug #7 which allowed for multiple simultaneous pane monitoring #14
Conversation
| PANEID=$(tmux list-panes | grep "active" | awk -F\] '{print $3}' | awk '{print $1}') | ||
| PID_DIR=~/.tmux/notify | ||
| # Get current directory | ||
| CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" |
There was a problem hiding this comment.
Any reason for not using just using CURRENT_DIR=$(dirname $BASH_SOURCE)?
There was a problem hiding this comment.
@ChanderG Thanks for bringing that up I missed that myself. I reused that code from commit 5f7554e. When I did that commit, I use the
code of the tmux-continuum plugin of @bruno- as a template. I did some research myself, and it looks like it is the most reliable way to get the ABSOLUTE script path (See this stackoverflow question). I am however not sure if there are cases where the relative path fails while the absolute path works. Maybe @bruno- still knows why he added the extra cd and pwd arguments in his original commit.
There was a problem hiding this comment.
It also seems to be the recommended answer in this stackoverflow question.
There was a problem hiding this comment.
@ChanderG I quickly double-checked why the CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && PWD )" syntax is used in most bash scripts over the simpler CURRENT_DIR=$(dirname $BASH_SOURCE) syntax. The reason is explained in this blog post. The longer version always get the absolute path and therefore also works when the working directory is changed before you use the path. As this doesn't seem to be the case in our scripts, we can also use the shorter form. Nevertheless, I think it does not hurt the use of the longer version to prevent future possible errors if we change the scripts in the future. Let me know what you prefer so that I can merge this pull request.
| PANEID=$(tmux list-panes | grep "active" | awk -F\] '{print $3}' | awk '{print $1}') | ||
| PID_DIR=~/.tmux/notify | ||
| # Get current directory | ||
| CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" |
There was a problem hiding this comment.
@ChanderG Thanks for bringing that up I missed that myself. I reused that code from commit 5f7554e. When I did that commit, I use the
code of the tmux-continuum plugin of @bruno- as a template. I did some research myself, and it looks like it is the most reliable way to get the ABSOLUTE script path (See this stackoverflow question). I am however not sure if there are cases where the relative path fails while the absolute path works. Maybe @bruno- still knows why he added the extra cd and pwd arguments in his original commit.
| PANEID=$(tmux list-panes | grep "active" | awk -F\] '{print $3}' | awk '{print $1}') | ||
| PID_DIR=~/.tmux/notify | ||
| # Get current directory | ||
| CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" |
There was a problem hiding this comment.
It also seems to be the recommended answer in this stackoverflow question.
This commit fixes a bug which used the `$CURRENT_DIR` variable before it is used.
This commit fixes a bug which caused the cancel monitoring message to appear twice.
This commit removes the codacy badge as the upstream does not use it.
|
@ChanderG I fixed some last bugs and merged the changes into the master branch. Feel free to let me know if you want to use the simpler directory fetch syntax. |
|
I wasn't aware of the use of the longer version. Makes sense. |
I wasn't familiar with it either. There is also the |
This pull request fixes bug #7 which allowed the same pane to be monitored multiple times. Additionally with the new fix user are now also able to monitor multiple panes in different windows and sessions at the same time.