Make grab area configurable using an option

[ivanalejandro0]

So, this should tackle the proposed new option on #8.

Here are some thoughts about this implementation:

• We now have "default" values for options in two files: extrakto.tmux and helpers.sh
• When you use an horizontal extrakto you may end up shifting some lines upwards but I can't think of a reliable way of calculating how many extra lines we should add for that. This will vary on how long are the lines you're seeing by the time you open extrakto.
• Available configuration now can't be found in "just one file" as I liked to think before. This doesn't need to be a bad thing, it's just a nice thing we had before and now we don't. If we do want to have that we may need to think of a better way of managing configurations.

@maximbaz since you are our test user for this feature let me know how it feels. I've tested it trying to grab lines right on the edge of the visible screen and not grabing lines right on the edge of the non-visible screen and it seems to be working fine.

[ivanalejandro0]

To keep configurations together we could do something as tmux-yank does. See this:

Configurations are calculated and get from here: https://github.com/tmux-plugins/tmux-yank/blob/master/scripts/helpers.sh and used like this: https://github.com/tmux-plugins/tmux-yank/blob/master/yank.tmux#L44

We could have a scripts/options.sh file or something like that, which we would use as our "central configuration management place" thing.

That's the last idea that my brain is capable today, I hope it get us somewhere :)

[maximbaz]

Awesome!

I'm only not sure in the whole concept of "compensating for fzf window". It is becoming complicated, and there is simply no way to make it consistent. Like you noted yourself, with the horizontal split it is not clear how many lines it is needed to compensate. But vertical split has the same issue, the current compensation relies on the cursor being in the bottom of the screen. When this is not the case, you do get prompts from the invisible area:

• echo "LALALA"
• Ctrl+L to clear pane
• search for "LALALA" and you'll find it

Maybe we should treat "visible" not as "what was visible before you started extrakto", but "what is visible after you started extrakto"? Then you just pass "0" and it is always consistent, in both splits.

To mitigate the cases that sometimes you do need to search for the content that was scrolled above, why don't we introduce one more key binding to "toggle content [visible/all]", just like today we have the "toggle filter [w/pu]"? That way, instead of guessing how to compensate, we don't compensate anything, but have a quick ability to search everything.

[laktak]

@maximbaz I have to admit I don't quite understand why you want to limit the captured area. The results are fed into fzf so that the text at the bottom has a higher priority than the text at the top.

When you select you should get the most recent tokens first - is there a problem with the results you get? Is it a performance issue?

[maximbaz]

I guess it is just an engineering perfection, I tend to have quite large history in a tmux panel and I know that I never need to copy a word from the the invisible area, so it makes me a little sad to see extrakto having parsed and discovered a few thousand entries for me.

But there is also a problem with the results that I get: the sorting (most recent tokens below) is only applied in the beginning, fzf doesn't guarantee the order when you actually start typing the filter. See the example:

That means, if I had abc.txt printed 10000 lines above, even though I only have xyz.abc.txt right now on the screen, searching for abc will put abc.txt as the first result.

[laktak]

@maximbaz Ah I understand - I forgot to set fzf tiebreak. Can you try it again with master?

@ivanalejandro0 How about calling the options full and recent? recent would just use an arbitrary number like -10 that makes sure that usually everything works as expected.

Since we are then down to just one config for this feature why not pass it along as an argument to tmux-extrakto.sh like we are already doing with EXTRAKTO_OPT?

[maximbaz]

0 or -10 is good. What do you think about adding one more toggle, between visible and all?

With tiebreak defined the order is correct :+1:

[maximbaz]

To continue the discussion on the "why do this".

Found a good command for doing some benchmarking:

tr -dc a-z1-4 </dev/urandom | tr 1-2 ' \n' | awk 'length==0 || length>50' | tr 3-4 ' ' | sed 's/^ *//' | cat -s | sed 's/ / /g' | fmt

Assuming the capture_start is set to parse truly full history with -$(get_tmux_option "history-limit" "2000"), running that command even for a couple of seconds produces so much output (around 100k lines), that makes extrakto take quite a few noticeable seconds before it can even initialize the fzf window.

My current project at work produces ~6k lines in the log every time I build it. Reaching 100k is far from being impossible, especially if you don't reboot the build server every day 🙂

[laktak]

@maximbaz are you possibly running this in Bash on Windows? Because on my Linux VM/Macbook the results from your testcase are displayed without any delay. (my tmux history-limit is set to 10000)

[laktak]

Forgot about the toggle - I'm OK with it though the help text shouldn't get too long.

[maximbaz]

There are no issues on 10k, try 100k ;) I'm on ArchLinux.

ctrl+l=toggle area [visible/all] or ctrl+l=toggle area [recent/full]

[laktak]

There is no issues on 10k, try 100k ;) I'm on ArchLinux.

Why 100k? You said the default is 2000 while the script takes a max of 32768 lines.

So I restarted tmux with 100000 lines and the result is still instant (tmux says 97643 lines and fzf gives 103835 results). I'm also on Arch using Python 3.6.2 (default, Jul 20 2017, 03:52:27) and fzf 0.17.0

[maximbaz]

So first of all I should mention that I believe the capture_start needs to respect history-limit regardless of this PR, the current value of 32768 is an arbitrary value that doesn't represent anything. Otherwise the names all and full are simply not correct.

Now, when I described the benchmarking and said that the effect is visible at 100k lines, I implicitly assumed that (1) you set history-limit to at least 100k and (2) change capture_start from a hardcoded arbitrary value of 32768 to the same 100k - sorry if that was not clear. I'm not talking about defaults anymore, I'm showing a benchmark and providing values which on my hardware demonstrate when extrakto becomes slow.

I guess you didn't do the second step, as fzf providing only 103k words per the last 100k lines is a too small number, there is more than one word per generated line.

When I do these two changes, then run that tr command to produce ~100k lines, then fire up extrakto in the default "full" mode (the true full, not 32768), it takes a few seconds to initialize fzf window.

Maybe your hardware is faster and can easily handle 100k limit, try increasing the value. How about 1m lines?

I'm recording the video below with 1m records on a quite powerful laptop, my server at work where I compile the project and expect to see a lot of output in terminal is not that fast:

[ivanalejandro0]

Hey, sorry for the delay coming back to you. Here are my thoughts on this matter:

For the slowness on parsing output it may be due to mechanical vs solid state hard drives. Or whatever the reason would be, I'd like to give the option to a user with a particular setup to have faster responses.

[...] How about calling the options full and recent? recent would just use an arbitrary number like -10 that makes sure that usually everything works as expected. [...]

love it!

Since we are then down to just one config for this feature why not pass it along as an argument to tmux-extrakto.sh like we are already doing with EXTRAKTO_OPT?

Ohh, I missed that bit, I'll definitely change it to do it that way, thanks!

I believe the capture_start needs to respect history-limit regardless of this PR [...]. Otherwise the names all and full are simply not correct.

100% agree. I didn't know where the 32768 limit came either, I just assumed it was the limit of what we could capture.

ctrl+l=toggle area [recent/full]

I like this. I don't promise to add it to this PR, though. I'm a bit short of time this week. But if not, I definitely can handle in a different PR :)

Here's my todo list for this PR, I'll tackle them asap:

• [x] rename grab area to full and recent
• [x] pass capture_start as param to get all configs in one place
• [x] for full, capture_start needs to respect history-limit
• [x] add hotkey to toggle grab area... ctrl+l=toggle area [recent/full]

I'll read this more thoroughly when I sit down to code the changes to make sure I didn't miss something. Let me know if that's the case. Cheers!

[laktak]

@maximbaz OK, I get it now.

@ivanalejandro0 sounds great! If it's not too much trouble please allow numbers for the area option as well so that in case someone really runs into performance problems they can limit it.

[ivanalejandro0]

I'm getting the required options on the tmux-extrakto.sh file itself, otherwise we will end up passing more and more parameters over time. This time I needed to add 2 more for these features.

I moved all the configurations to helpers.sh, maybe we should call it config.sh?

This change allowed me to use configurations anywhere on the app without worrying about having duplicated "defaults" written in different places. Also, it looks to me like a good balance between usability and easiness.

I kept get_capture_pane_start since there was more logic involved on this feature than a couple of lines anyway.

Right now we are using the "@extrakto_default_opt" option, I propose renaming it to "@extrakto_args" it makes more sense to me since they are literally the arguments we pass to extrakto.py, naming it "default" sounds odd to me. Anyway, that's not for this PR and I'm ok with leaving it like it is.

Sorry for the long list of changes, this took a bit more work than I expected.

btw, if you were using this branch you may have problems pulling it, I had to force push this branch after rebased to lastest master and solved a conflict.

[laktak]

Looks good though I also thought we were going with one option like @maximbaz suggested.

Could you update that in this PR? I would also need the [CLIP-TOOL] option back as I have my own clipboard manager. Maybe move it into your new tidy config script?

[ivanalejandro0]

Oh, it looks like I misunderstood the idea then... sure, I'll merge those into one option, it sounds better.

@laktak sure, I'll add an option for a custom clipboard tool/command. I removed since I saw no path from where we were using it... how were you using that option?

[ivanalejandro0]

So, I've just merged the options, I had to add some nested if in order to support 2 or 3 option switch for grab area... I tested it and it seems to work with whatever you give it :)

I've also added the clipboard option to customize it... funny thing, to test if that was working I used the following line: set -g @extrakto_clip_tool "espeak", which basically speaks out loud any word I hit enter on :D, I can't think of a practical use of it, but is so cool to find hidden features like this one.

[laktak]

sure, I'll add an option for a custom clipboard tool/command. I removed since I saw no path from where we were using it... how were you using that option?

My primary prefix key takes me directy to copy-mode-vi. So I'm not using the built in tmux script but define the keybinding myself for that table. I don't think anyone else does it this way though so I didn't bother to add it for the plugin ;)

The PR looks good! Thank you!