Allow overriding filter and search modes from CLI #635

pdecat · 2022-12-03T10:56:42Z

This PR allows overriding filter and search modes from CLI arguments:

# atuin search -h | grep overriding
      --filter-mode <FILTER_MODE>    Allow overriding filter mode over config [possible values: global, host, session, directory]
      --search-mode <SEARCH_MODE>    Allow overriding search mode over config [possible values: prefix, full-text, fuzzy]

The bash up key bindings are updated to always use session filter mode when invoking atuin, and that choice may not be acceptable by other.

TODO:

decide what to do regarding up key bindings
also update fish and zsh up key bindings

pdecat · 2022-12-03T12:13:55Z

src/shell/atuin.bash

-    bind -x '"\e[A": __atuin_history'
-    bind -x '"\eOA": __atuin_history'
+    bind -x '"\e[A": __atuin_history --filter-mode session'
+    bind -x '"\eOA": __atuin_history --filter-mode session'


On second thought, an alternative may be to have an additional configuration setting for filter mode when interactive mode is invoked via up key bindings.

Not sure which would be the best way to tell atuin that it invoked from the up key binding between:

adding a --shell-up-key-binding CLI argument:

Suggested change

bind -x '"\eOA": __atuin_history --filter-mode session'

bind -x '"\eOA": __atuin_history --shell-up-key-binding'

defining a ATUIN_SHELL_UP_KEY_BINDING environment variable:

Suggested change

bind -x '"\eOA": __atuin_history --filter-mode session'

bind -x '"\eOA": ATUIN_SHELL_UP_KEY_BINDING=1 __atuin_history'

I have a slight preference for hardcoding --filter-mode session in the init scripts, since up is for looking for a previous command and ctrl-r is for searching the history.

Using a --shell-up-key-binding argument is a good alternative if you want the configs to be in the config file instead of requiring users to customize the shell script. I don't think we should use an environment variable for this, you can always just make the CLI argument hidden from the help functions if that's the concern.

I'd rather configure this in our config file, and globally for all shells (not just bash). I'd also be pretty happy for --shell-up-key-binding to be how it's invoked!

I have a slight preference for hardcoding --filter-mode session in the init scripts, since up is for looking for a previous command and ctrl-r is for searching the history.

I think generally I'd agree with you there @patricksjackson, but I'm also a little cautious to change defaults at this point without a strong opinion for why. Currently I mostly feel lukewarm on the topic

arcuru

Thanks for writing this! I've had this on my backlog ever since I started using Atuin.

src/command/client/search.rs

arcuru · 2022-12-07T17:07:59Z

src/shell/atuin.bash

-    bind -x '"\e[A": __atuin_history'
-    bind -x '"\eOA": __atuin_history'
+    bind -x '"\e[A": __atuin_history --filter-mode session'
+    bind -x '"\eOA": __atuin_history --filter-mode session'


I have a slight preference for hardcoding --filter-mode session in the init scripts, since up is for looking for a previous command and ctrl-r is for searching the history.

Using a --shell-up-key-binding argument is a good alternative if you want the configs to be in the config file instead of requiring users to customize the shell script. I don't think we should use an environment variable for this, you can always just make the CLI argument hidden from the help functions if that's the concern.

ellie

Thanks for this 🙌 Generally happy with it, just one small suggested then it's good to go imo

ellie · 2022-12-16T17:09:47Z

src/shell/atuin.bash

-    bind -x '"\e[A": __atuin_history'
-    bind -x '"\eOA": __atuin_history'
+    bind -x '"\e[A": __atuin_history --filter-mode session'
+    bind -x '"\eOA": __atuin_history --filter-mode session'


I'd rather configure this in our config file, and globally for all shells (not just bash). I'd also be pretty happy for --shell-up-key-binding to be how it's invoked!

I have a slight preference for hardcoding --filter-mode session in the init scripts, since up is for looking for a previous command and ctrl-r is for searching the history.

I think generally I'd agree with you there @patricksjackson, but I'm also a little cautious to change defaults at this point without a strong opinion for why. Currently I mostly feel lukewarm on the topic

pdecat · 2022-12-17T14:55:48Z

Hi, I've implemented the discussed changes:

add a --shell-up-key-binding hidden command argument,
add a filter_mode_shell_up_key_binding configuration option to allow customizing the filter mode used when atuin is invoked from a shell up-key binding,
update the fish and zsh scripts too (could not test them though).

I've also kept the default to be global to avoid messing with other people preferences.

It would be great if fish and/or zsh user could validate the changes, as I only use bash.

… properly passed

…ell_up_key_binding configuration option to allow customizing the filter mode used when atuin is invoked from a shell up-key binding

ellie

One last thing! Then I'm happy to get this merged

src/shell/atuin.zsh

pdecat commented Dec 3, 2022

View reviewed changes

pdecat marked this pull request as ready for review December 3, 2022 15:52

arcuru suggested changes Dec 7, 2022

View reviewed changes

pdecat force-pushed the set_filter_and_search_mode_from_cli branch from c29d493 to 30511bc Compare December 12, 2022 23:18

ellie reviewed Dec 16, 2022

View reviewed changes

pdecat requested review from ellie and arcuru and removed request for ellie and arcuru December 17, 2022 14:56

pdecat added 3 commits December 17, 2022 16:38

Allow overriding filter and search modes from CLI arguments

5dbf8ce

Use session filter mode for bash up key binding

629a928

We precisely do not want to add quotes here so that all arguments are…

cdc51e7

… properly passed

pdecat force-pushed the set_filter_and_search_mode_from_cli branch from 1461768 to 303cc99 Compare December 17, 2022 15:40

Add --shell-up-key-binding hidden command argument and filter_mode_sh…

3cea079

…ell_up_key_binding configuration option to allow customizing the filter mode used when atuin is invoked from a shell up-key binding

pdecat force-pushed the set_filter_and_search_mode_from_cli branch from 303cc99 to 3cea079 Compare December 17, 2022 15:58

ellie reviewed Dec 18, 2022

View reviewed changes

src/shell/atuin.zsh Outdated Show resolved Hide resolved

Correct up binding for zsh

36a2864

ellie approved these changes Dec 18, 2022

View reviewed changes

ellie enabled auto-merge (squash) December 18, 2022 18:17

ellie merged commit ed394af into atuinsh:main Dec 18, 2022

pdecat deleted the set_filter_and_search_mode_from_cli branch December 18, 2022 18:26

ellie mentioned this pull request Mar 5, 2023

filter_mode configuration does not seem to have any effect #756

Closed

tpoliaw mentioned this pull request Jan 19, 2024

One cursor-up history per session #468

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow overriding filter and search modes from CLI #635

Allow overriding filter and search modes from CLI #635

pdecat commented Dec 3, 2022 •

edited

pdecat Dec 3, 2022

pdecat Dec 3, 2022

arcuru Dec 7, 2022

ellie Dec 16, 2022

pdecat Dec 17, 2022

arcuru left a comment

arcuru Dec 7, 2022

ellie left a comment

ellie Dec 16, 2022

pdecat commented Dec 17, 2022

ellie left a comment

	bind -x '"\eOA": __atuin_history --filter-mode session'
	bind -x '"\eOA": __atuin_history --shell-up-key-binding'

	bind -x '"\eOA": __atuin_history --filter-mode session'
	bind -x '"\eOA": ATUIN_SHELL_UP_KEY_BINDING=1 __atuin_history'

Allow overriding filter and search modes from CLI #635

Allow overriding filter and search modes from CLI #635

Conversation

pdecat commented Dec 3, 2022 • edited

pdecat Dec 3, 2022

Choose a reason for hiding this comment

pdecat Dec 3, 2022

Choose a reason for hiding this comment

arcuru Dec 7, 2022

Choose a reason for hiding this comment

ellie Dec 16, 2022

Choose a reason for hiding this comment

pdecat Dec 17, 2022

Choose a reason for hiding this comment

arcuru left a comment

Choose a reason for hiding this comment

arcuru Dec 7, 2022

Choose a reason for hiding this comment

ellie left a comment

Choose a reason for hiding this comment

ellie Dec 16, 2022

Choose a reason for hiding this comment

pdecat commented Dec 17, 2022

ellie left a comment

Choose a reason for hiding this comment

pdecat commented Dec 3, 2022 •

edited