Skip to content
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.

Sign up for GitHub

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

Auto-creating folders in logging path #2 #21

Merged
merged 2 commits into from
Mar 2, 2018
Merged

Auto-creating folders in logging path #2 #21

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5ed764
Changing variable working way. Reducing 'get_tmux_option' uses. Addin…
selfuryon Feb 12, 2018
3059d52
Refactoring code based on #21 PR comments
selfuryon Mar 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions logging.tmux
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,21 @@ CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
source "$CURRENT_DIR/scripts/variables.sh"
source "$CURRENT_DIR/scripts/shared.sh"


setup_logging_key_binding() {
local key="$(get_tmux_option "$logging_key_option" "$default_logging_key")"
tmux bind-key "$key" run-shell "$CURRENT_DIR/scripts/toggle_logging.sh"
tmux bind-key "$logging_key" run-shell "$CURRENT_DIR/scripts/toggle_logging.sh"
}

setup_screen_capture_key_binding() {
local key="$(get_tmux_option "$pane_screen_capture_key_option" "$default_pane_screen_capture_key")"
tmux bind-key "$key" run-shell "$CURRENT_DIR/scripts/screen_capture.sh"
tmux bind-key "$pane_screen_capture_key" run-shell "$CURRENT_DIR/scripts/screen_capture.sh"
}

setup_save_complete_history_key_binding() {
local key="$(get_tmux_option "$save_complete_history_key_option" "$default_save_complete_history_key")"
tmux bind-key "$key" run-shell "$CURRENT_DIR/scripts/save_complete_history.sh"
tmux bind-key "$save_complete_history_key" run-shell "$CURRENT_DIR/scripts/save_complete_history.sh"
}

setup_clear_history_key_binding() {
local key="$(get_tmux_option "$clear_history_key_option" "$default_clear_history_key")"
tmux bind-key "$key" run-shell "$CURRENT_DIR/scripts/clear_history.sh"
tmux bind-key "$clear_history_key" run-shell "$CURRENT_DIR/scripts/clear_history.sh"
Copy link
Member

Choose a reason for hiding this comment

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

Given that all the above functions are just one-liners now, how do you feel about just having them in the main function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good suggestion! I will make it in the second commit :)

}

main() {
Expand Down
23 changes: 6 additions & 17 deletions scripts/capture_pane_helpers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,15 @@
# Variables in this helper should be set in the file sourcing the helper.
# Required variables:
# - path_option & default_path
# - name_option & default_name

get_path() {
get_tmux_option "$path_option" "$default_path"
}

# `tmux save-buffer` command does not perform interpolation, so we're doing it
# "manually" with `display-message`
get_filename() {
local name_template=$(get_tmux_option "$name_option" "$default_name")
tmux display-message -p "$name_template"
}

Copy link
Member

Choose a reason for hiding this comment

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

Big win!

capture_pane() {
local file="$(get_path)/$(get_filename)"
local capture_scope=$1
if [ $capture_scope == "History" ]; then
local file=$(tmux display-message -p "$save_complete_history_full_filename")
local path=$(tmux display-message -p "$save_complete_history_path")
mkdir -p "$path"
local history_limit="$(tmux display-message -p -F "#{history_limit}")"
tmux capture-pane -J -S "-${history_limit}" -p > "$file"
elif [[ $capture_scope == "Screen capture" ]]; then
local file=$(tmux display-message -p "$screen_capture_full_filename")
local path=$(tmux display-message -p "$screen_capture_path")
mkdir -p "$path"
Copy link
Member

Choose a reason for hiding this comment

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

After your refactoring it seems as this function is unnecessarily doing 2 things: capturing history and doing screen capture.

Would it make things simpler if this one was split into two functions? With that we could get rid of this whole file because the functions would be kept in files where they're used..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it will be simpler than the current state. I will refactor this part

tmux capture-pane -J -p > "$file"
else
# error
Expand Down
7 changes: 0 additions & 7 deletions scripts/save_complete_history.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@ source "$CURRENT_DIR/variables.sh"
source "$CURRENT_DIR/shared.sh"
source "$CURRENT_DIR/capture_pane_helpers.sh"

# Functions defined in capture_pane_helpers are customized via below variables.
default_path="$default_save_complete_history_path"
default_name="$default_save_complete_history_filename"

path_option="$save_complete_history_path_option"
name_option="$save_complete_history_filename_option"

main() {
if supported_tmux_version_ok; then
capture_pane "History"
Expand Down
7 changes: 0 additions & 7 deletions scripts/screen_capture.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@ source "$CURRENT_DIR/variables.sh"
source "$CURRENT_DIR/shared.sh"
source "$CURRENT_DIR/capture_pane_helpers.sh"

# Functions defined in capture_pane_helpers are customized via below variables.
default_path="$default_screen_capture_path"
default_name="$default_screen_capture_filename"

path_option="$screen_capture_path_option"
name_option="$screen_capture_filename_option"

main() {
if supported_tmux_version_ok; then
capture_pane "Screen capture"
Expand Down
15 changes: 6 additions & 9 deletions scripts/toggle_logging.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,18 @@ CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
source "$CURRENT_DIR/variables.sh"
source "$CURRENT_DIR/shared.sh"

get_filename() {
local logging_path="$(get_tmux_option "$logging_path_option" "$default_logging_path")"
local logging_filename="$(get_tmux_option "$logging_filename_option" "$default_logging_filename")"
echo "${logging_path}/${logging_filename}"
}

start_pipe_pane() {
local filename="$(get_filename)"
"$CURRENT_DIR/start_logging.sh" "$filename"
display_message "Started logging to $filename"
local file=$(tmux display-message -p "$logging_full_filename")
local path=$(tmux display-message -p "$logging_path")
Copy link
Member

Choose a reason for hiding this comment

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

Seing these 2 lines with tmux display-message -p again tripped me, despite getting your clarification in #20.
Chances are anyone else reading the source will be puzzled as well. How about extracting tmux display-message -p to a helper function? The purpose would be just to clarify intention. Example names: expand_value, expand_tmux_formats.. or something more fitting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think about this part... Maybe you are right and extracting this code to external function will be a good decision

mkdir -p "$path"
"$CURRENT_DIR/start_logging.sh" "$file"
display_message "Started logging to $logging_full_filename"
}

stop_pipe_pane() {
tmux pipe-pane
display_message "Ended logging to $(get_filename)"
display_message "Ended logging to $logging_full_filename"
}

# returns a string unique to current pane
Expand Down
36 changes: 26 additions & 10 deletions scripts/variables.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,54 @@ SUPPORTED_VERSION="1.9"

# Key binding options and defaults

logging_key_option="@logging-key"
default_logging_key="P" # Shift-p
logging_key=$(tmux show-option -gqv "@logging_key")
logging_key=${logging_key:-$default_logging_key}

pane_screen_capture_key_option="@screen-capture-key"
default_pane_screen_capture_key="M-p" # Alt-p
pane_screen_capture_key=$(tmux show-option -gqv "@screen-capture-key")
pane_screen_capture_key=${pane_screen_capture_key:-$default_pane_screen_capture_key}

save_complete_history_key_option="@save-complete-history-key"
default_save_complete_history_key="M-P" # Alt-Shift-p
save_complete_history_key=$(tmux show-option -gqv "@save-complete-history-key")
save_complete_history_key=${save_complete_history_key:-$default_save_complete_history_key}

clear_history_key_option="@clear-history-key"
default_clear_history_key="M-c" # Alt-c
clear_history_key=$(tmux show-option -gqv "@clear-history-key")
clear_history_key=${clear_history_key:-$default_clear_history_key}

# General options
filename_suffix="#{session_name}-#{window_index}-#{pane_index}-%Y%m%dT%H%M%S.log"

# Logging options
logging_path_option="@logging-path"
default_logging_path="$HOME"
logging_path=$(tmux show-option -gqv "@logging-path")
logging_path=${logging_path:-$default_clear_history_key}

logging_filename_option="@logging-filename"
default_logging_filename="tmux-${filename_suffix}"
logging_filename=$(tmux show-option -gqv "@logging-filename")
logging_filename=${logging_filename:-$default_logging_filename}

logging_full_filename="${logging_path}/${logging_filename}"

# Screen capture options
screen_capture_path_option="@screen-capture-path"
default_screen_capture_path="$HOME"
screen_capture_path=$(tmux show-option -gqv "@screen-capture-path")
screen_capture_path=${screen_capture_path:-$default_screen_capture_path}

screen_capture_filename_option="@screen-capture-filename"
default_screen_capture_filename="tmux-screen-capture-${filename_suffix}"
screen_capture_filename=$(tmux show-option -gqv "@screen-capture-filename")
screen_capture_filename=${screen_capture_filename:-$default_screen_capture_filename}

screen_capture_full_filename="${screen_capture_path}/${screen_capture_filename}"

# Save complete history options
save_complete_history_path_option="@save-complete-history-path"
default_save_complete_history_path="$HOME"
save_complete_history_path=$(tmux show-option -gqv "@save-complete-history-path")
save_complete_history_path=${save_complete_history_path:-$default_save_complete_history_path}

save_complete_history_filename_option="@save-complete-history-filename"
default_save_complete_history_filename="tmux-history-${filename_suffix}"
save_complete_history_filename=$(tmux show-option -gqv "@save-complete-history-filename")
save_complete_history_filename=${save_complete_history_filename:-$default_save_complete_history_filename}

save_complete_history_full_filename="${save_complete_history_path}/${save_complete_history_filename}"