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

feat: add an optional second parameter to trigger extrakto with a spe… #118

Merged
merged 3 commits into from
May 9, 2024

Conversation

marcdeop
Copy link
Contributor

@marcdeop marcdeop commented May 5, 2024

This allow us to trigger extrakto with a specific filter in mind.

For example running open.sh 0 url will open with the url filter (and only that one):
image

Thanks to the restore_original() function, our settings for extrakto will remain the same.

This opens the possibility of having custom key-bindings like:

tmux bind-key u run-shell "\"$extrakto_open\" \"#{pane_id}\" url"

To trigger extracto with our needed filter in mind :-)

@laktak
Copy link
Owner

laktak commented May 6, 2024

Thanks for your PR!

I like your idea but could you change the implementation to pass the filter along instead of temporarily changing the option?

For example tmux allows you to pass an env. variable to the new process:

 -e takes the form ‘VARIABLE=value’ and sets an environment variable 
 for the popup; it may be specified multiple times.

You could pass along extrakto_inital_mode and return that in the get_next_mode() function for if [ $next == "initial" ].

@marcdeop
Copy link
Contributor Author

marcdeop commented May 9, 2024

Thanks for your PR!

I like your idea but could you change the implementation to pass the filter along instead of temporarily changing the option?

For example tmux allows you to pass an env. variable to the new process:

 -e takes the form ‘VARIABLE=value’ and sets an environment variable 
 for the popup; it may be specified multiple times.

You could pass along extrakto_inital_mode and return that in the get_next_mode() function for if [ $next == "initial" ].

I will have a look!

@marcdeop
Copy link
Contributor Author

marcdeop commented May 9, 2024

I think your proposal is WAY more elegant than what I did.

Please have a look at how I implemented it this time @laktak

The previous implementation of this functionallity temporary replaced
the @extrakto_filter_order and restored the original value when
finished.

This is a much more elegant solution
@laktak
Copy link
Owner

laktak commented May 9, 2024

Looking good! Just one thing - please change EXTRAKTO_INITAL_MODE to lowercase (shell naming conventions) before I merge it.

We also have a new branch (https://github.com/laktak/extrakto/tree/python-posix) if you are interested, but I can update that one later.

@marcdeop
Copy link
Contributor Author

marcdeop commented May 9, 2024

Looking good! Just one thing - please change EXTRAKTO_INITAL_MODE to lowercase (shell naming conventions) before I merge it.

I made it lowercase

We also have a new branch (https://github.com/laktak/extrakto/tree/python-posix) if you are interested, but I can update that one later.

Is this a long term thing or are you planning on merging this soon?

@laktak laktak merged commit a3d76b8 into laktak:master May 9, 2024
1 check passed
@laktak
Copy link
Owner

laktak commented May 9, 2024

Thank you! :)

Is this a long term thing or are you planning on merging this soon?

Depends on how stable it is but I'd rather switch to it soon.

marcdeop added a commit to marcdeop/extrakto that referenced this pull request May 9, 2024
marcdeop added a commit to marcdeop/extrakto that referenced this pull request May 9, 2024
laktak pushed a commit that referenced this pull request May 9, 2024
laktak added a commit that referenced this pull request Jun 20, 2024
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.

2 participants