feat: add option for overwriting indexes #2
Conversation
also refactored add method to avoid code duplication
readme.md
Outdated
| To map panes or session to a specific index, use the -o/O flag: | ||
|
|
||
| ```conf | ||
| bind -n M-q run 'harpoon -s 1' # alt-q goes to index 1 |
There was a problem hiding this comment.
we can merge this example tmux configuration with the one above. add shorter comments at end of line to explain what the configuration is about. At the same time make the keybinds consistent. I like your approach of have qwerty denote the position.
There was a problem hiding this comment.
Make keybinds consistent, as in change the existing binds to qwerty?
harpoon
Outdated
| base_info='#{session_name}=#{session_path}' | ||
| pane_info=':#{window_index}.#{pane_index}' | ||
| tmux display -p "$base_info$([ "$extended" = 1 ] && echo "$pane_info")" >>"$cachefile" | ||
| bookmark=$(tmux display -p "$base_info$([ "$extended" = true ] && echo "$pane_info")") |
There was a problem hiding this comment.
unnecessary variable assignment, can directly echo the output of subcommand
consisten naming according to flags used
|
@Chaitanyabsprip I liked all your suggestions. Also noticed that I hadn't updated the help command. I made an assumption based on my reply to one of your comments, if that's not what you meant then I'll change it up. |
harpoon
Outdated
| base_info='#{session_name}=#{session_path}' | ||
| pane_info=':#{window_index}.#{pane_index}' | ||
| tmux display -p "$base_info$([ "$extended" = 1 ] && echo "$pane_info")" >>"$cachefile" | ||
| echo tmux display -p "$base_info$([ "$extended" = true ] && echo "$pane_info")" |
There was a problem hiding this comment.
this echo is a bug, I'm sure this is going to cause an error, it will echo "tmux" and then complain that display is not a command. You can completely get rid of the leading echo here.
readme.md
Outdated
| bind M-q run 'harpoon -r 1' # prefix+alt+q adds pane to index 1 | ||
|
|
||
| bind -n M-w run 'harpoon -s 2' | ||
| bind M-w run 'harpoon -R 2' # adds session instead of pane to index 2 |
There was a problem hiding this comment.
reading this explanation, the behavior of -a, -A, and -r, -R seem counterintuitive and opposite. -r should replace with session and -R should replace with pane to be consistent with -a, -A.
There was a problem hiding this comment.
So I noticed that the behavior of the implementation is consistent, just the documentation is incorrect. Can you please make the documentation in the readme and the help command consistent with the implementation. Thanks
readme.md
Outdated
|
|
||
| # Map panes or sessions to specific by replacing entires | ||
| # If there is no pane/session in the given index, it will be appended to the list instead. | ||
| bind -n M-q run 'harpoon -s 1' # alt+q goes to index 1 |
There was a problem hiding this comment.
this is repetition of the command above, either remove this entry or the one above. I would suggest the following
bind -n M-i run 'harpoon -e'
bind -n M-q run 'harpoon -s 1' # alt+q goes to index 1
bind M-q run 'harpoon -r 1' # prefix alt+q replace entry index 1 with current session
bind -n M-w run 'harpoon -s 2'
bind M-w run 'harpoon -R 2' # replace entry at index 2 with current pane within session
bind -n M-e run 'harpoon -s 3'
bind -n M-r run 'harpoon -s 4'
# Note: If there is no entry at the given index, it is appended to the list instead.There was a problem hiding this comment.
Good suggestion!
|
Sorry for the mistakes, I had originally made my changes and tested them, and accidentally restored my changes in git. Then after recreating my changes I thought I had tested them again, but I think I might have forgotten to refresh my tmux config or something. I pushed new updates now according to your comments @Chaitanyabsprip |
|
Thanks for your contribution and prompt replies and changes. I really appreciate it, hope this tool caters to you better with these changes. |
Absolutely, thanks for working with me! This tool was really a missing piece in my workflow, so hope I can help out more as it grows. |
also refactored add method to avoid code duplication
The idea is to be able to set a bookmark for a given keybind, so that i.e:
I use alt+q to go to index 1
I use alt+w to go to index 2
When I want to pin a pane/session to alt+q, I can press prefix+alt+q to store that pane on index 1, which alt+q is bound to.
This avoids manually maintaining the index list when all you want is to put a bookmark for a given index