Auto-creating folders in logging path #20
Conversation
| @@ -4,7 +4,10 @@ | |||
| # - name_option & default_name | |||
|
|
|||
| get_path() { | |||
| get_tmux_option "$path_option" "$default_path" | |||
| local path_template=$(get_tmux_option "$path_option" "$default_path") | |||
| local check_folder=$(tmux display-message -p "$path_template") | |||
There was a problem hiding this comment.
What does this line do? Why can't you just use path_template variable on the 2 lines below?
There was a problem hiding this comment.
The reason of this is that path_template contains "%Y-%m-%d" which we need to convert real values. It can be done only through tmux display-message because bash doesn't know about this anything
| @@ -4,15 +4,17 @@ CURRENT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" | |||
|
|
|||
| source "$CURRENT_DIR/variables.sh" | |||
| source "$CURRENT_DIR/shared.sh" | |||
| source "$CURRENT_DIR/capture_pane_helpers.sh" | |||
There was a problem hiding this comment.
The helpers in this file should only be included in files that handle "capture pane" features. If get_path helper is now needed in more places then maybe:
- rename
capture_pane_helpers.shto be more generic - or create a separate helpers file just for
get_pathfunction
There was a problem hiding this comment.
After looking at this more I think this would be clean:
- create a separate helpers file for
get_path - this new helpers file would be sourced in
scripts/toggle_logging.shand inscripts/capture_pane_helpers.sh
There was a problem hiding this comment.
Ok, I will think about it
| default_name="$default_logging_filename" | ||
|
|
||
| path_option="$logging_path_option" | ||
| name_option="$logging_filename_option" |
There was a problem hiding this comment.
I just reviewed how and why are these variables defined. I must say it was a lousy decision on my part to set this "global state" that is then used implicitly in functions defined in capture_pane_helpers.sh.
If you have a suggestion how to refactor this, that would be welcome. I think we just want to pass all arguments to functions explicitly. For example, instead of just get_path it would be ok to have get_path "$logging_path_option" "$default_logging_path"
There was a problem hiding this comment.
If you wanna refactor this, let's do it in a separate PR. No need to do all at once.
There was a problem hiding this comment.
Thanks for the advice. I will create separate PR for refactoring this
Hello!
I created PR for auto-creating folders in logging path. So you can just use something like this:
And this path will be created automatically.
Based on this issue #19