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.

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

Add options #7

Merged
merged 3 commits into from
Oct 1, 2017
Merged

Add options #7

merged 3 commits into from
Oct 1, 2017

Conversation

ivanalejandro0
Copy link
Contributor

Changes list:

  • Group options together
  • Make split direction and size configurable

I wanted to have more than the default 6 lines of height on the tmux window and I thought that it may be useful to have extrakto as a sidebar as well on some cases, so I went ahead and added configuration options.

This is how it looks with more height:

set -g @extrakto_window_split_direction 'v'
set -g @extrakto_window_lines "20"

screenshot_20171001_150400

This is how it looks as an horizontal sidebar:

set -g @extrakto_window_split_direction 'h'
set -g @extrakto_window_lines "50"

screenshot_20171001_150238

@maximbaz
Copy link
Contributor

maximbaz commented Oct 1, 2017

Oh that is a good change, love it!

Would it be too much of an ask to add one more configuration option, for the number of lines outside the visible content in the pane? In other words, parameterize the 32768 part in the line below:

tmux set-buffer -- `tmux capture-pane -pJS -32768 -t !|$DIR/extrakto.py $1|fzf`

It was briefly discussed in #4. It is especially useful in combination with the extrakto_window_lines that you introduce here, e.g. if I expect extrakto pane to hide half of the content, I want to increase the number, but if I make extrakto pane very narrow, I would probably prefer to decrease that number to something around 0.

@ivanalejandro0
Copy link
Contributor Author

@maximbaz interesting idea, I like it

I don't mind adding it, but since I think it needs some discussion I'd rather handle this in a different PR/ticket for it.

I think that instead of adding a config option for "how many lines you want to grab" it may be easier to grasp for a user to pick different presets, like: "grab from all the contents" or "grab only from visible area".

Proposed option name: @extrakto_grab_from
Options: visible, all
Default: I think that all for default makes more sense, since fzf is really fast and I think that trying to grab something from the screen that just got pushed out of sight is a pretty common case.

Grabbing from visible will require to capture with -X being X the vertical lines, or 0 if horizontal.

@maximbaz
Copy link
Contributor

maximbaz commented Oct 1, 2017

I think @laktak also had presets in mind, and I agree that such concept is easier to grasp for a user. I like seeing it how you describe this, it all makes sense to me: name, options, the default. 👍

@ivanalejandro0
Copy link
Contributor Author

I've created #8 to handle that option, please take a look to see if I missed something.

@laktak
Copy link
Owner

laktak commented Oct 1, 2017

I was a bit confused at first beause a vertical split in vim is horizontal in tmux. I'm not a native english speaker so I found this a bit strange to say the least.

Anyways ;) thanks for the PR! Looks good but could you change the option names to

set -g @extrakto_split_direction 'v'
set -g @extrakto_split_size '6'

(because @extrakto_window_lines is not a good match with 'h')?

@ivanalejandro0
Copy link
Contributor Author

Yeah, while writing this PR I thought something was wrong because of that vim/tmux difference in naming. I'm not a native speaker either :), I guess it may be because you can refer to "horizontal" as "to split horizontally" or "to arrange horizontally".

I like your names better, shorter and more meaningful.
Changes pushed.

@laktak laktak merged commit 6b4e96b into laktak:master Oct 1, 2017
@laktak
Copy link
Owner

laktak commented Oct 1, 2017

Great, thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants