Allow quick open with pre-populated query and non-directories #76

kidonng · 2021-01-07T10:32:08Z

This PR improves both #72 and #73.

Also prepends ./ to the only selected path if it is a file. This has two benefits:
- Prepending ./ allows you to open a executable, just like entering a directory. (But the latter is Fish magic, while the former is not)
- This makes the feature more consistent. The user doesn't have to remember that they had to select a folder to make it work, now they only needs to know "If I only made one selection, I can quickly open it!".
Use current token to pre-populate the fzf query, replace it with selection #73 doesn't play nice with Prepend ./ to single directory selections, allowing for quick cd #72, because we assume the commandline is empty. We are now using a smarter check: if the length of the whole commandline matches the length of the first token of the commandline, we can assume the user is making use of the quick open feature. So Prepend ./ to single directory selections, allowing for quick cd #72 will work with pre-populated query.

PatrickF1 · 2021-01-07T18:56:23Z

This makes the feature more consistent. The user doesn't have to remember that they had to select a folder to make it work, now they only needs to know "If I only made one selection, I can quickly open it!".

Not quite. Well, as you said it only works with executables. What about for text files which is probably the most common use case? I'm indifferent though, will probably still merge this after some more thought.

kidonng · 2021-01-08T02:05:20Z

Well, as you said it only works with executables. What about for text files which is probably the most common use case?

That's why we are lifting the limit while still being generic: we don't prepend $EDITOR or something like that.

My example of executables may not be good, but I hope you can get what I mean. It's literally just, if you only selected one path, we prepend ./ for you, which you can make use of. There may be more use cases though.

PatrickF1 · 2021-01-08T21:27:00Z

functions/__fzf_search_current_dir.fish

+            set first_token_length 0
+        end
+
+        if test (string length (commandline)) = $first_token_length && test (count $file_paths_selected) = 1


Can you reverse the order of the if statement for performance reasons?

Also, can't you just test (count (commandline --tokenize) = 1) to tell if the user only inputted one token?

Can you reverse the order of the if statement for performance reasons?

As you wish, but two builtins vs one builtin doesn't make any difference.

just test (count (commandline --tokenize) = 1) to tell if the user only inputted one token

This doesn't allow it being triggered with an empty command line

<command> | (| indicates cursor position) is considered one token by commandline --tokenize, which is not desirable.

Oh good point, forgot commandline was a builtin too.

I still find the logic convoluted though. If all we want to know is if the user is still typing the first token, can't we just compare string trim (commandline) to string trim (commandline --current-token)?

Or even test (commandline --tokenize) = (commandline --current-token)

test (commandline --tokenize) = (commandline --current-token)

Brilliant! We can even drop --tokenize.

Actually, we should keep tokenize for situations where the user accidentally included a whitespace in the front or back for whatever reason but still has the cursor on the current token. I'll add it in and test.

Oh darn, now it doesn't work if the input is empty. Hmm...

Oh it's because if the commandline is empty, then (commandline --tokenize) is empty and test doesn't work

test: Missing argument at index 2 ~/.config/fish/functions/__fzf_search_current_dir.fish (line 29): if test (count $file_paths_selected) = 1 && test ""(commandline --tokenize) = ""(commandline --current-token) ^ in function '__fzf_search_current_dir'

PatrickF1 · 2021-01-13T06:49:32Z

@kidonng want me to finish this for you?

PatrickF1 · 2021-01-13T16:05:36Z

functions/__fzf_search_current_dir.fish

+        #   attempt to cd implicitly if a directory name starts with a dot)
+        if test (count $file_paths_selected) = 1
+            set commandline_tokens (commandline --tokenize)
+            set current_token (commandline --current-token)


@kidonng please LMK if you have a better solution to command substitution resulting in an empty string. Seems fish doesn't allow you to do "(commandline --tokenize)" so I have to use variables.

After further research I think this is the only way.

Yes, you have to use a variable (which IMO is not bad), see https://fishshell.com/docs/current/index.html#cartesian-products.

kidonng · 2021-01-14T03:22:31Z

Thanks for your work on this! ✨

PatrickF1 · 2021-01-14T17:45:32Z

Thanks for getting it started @kidonng!

…kF1#76) Make the quick cd behavior trigger even if the user was starting to type a token and even if the path is not a directory. Before, it would only prepend ./ if the command line was empty and the selected path is a directory. Now, if the user was in the middle of inputting the first token and only one path is selected, then prepend ./ to the selected path so that - if the path is an executable, the user can hit Enter one more time to immediately execute it - if the path is a directory, the user can hit Enter one more time to immediately cd into it

kidonng added 2 commits January 7, 2021 18:12

Allow quick cd with pre-populated query

90197de

Add comments and allow files

fd0f859

kidonng added 3 commits January 8, 2021 14:22

Update doc

5565df4

Merge branch 'main' into cd-with-query

4abb78f

Revise comments

aecac82

PatrickF1 reviewed Jan 8, 2021

View reviewed changes

kidonng and others added 3 commits January 9, 2021 13:48

Flip if statement

a958eee

Merge branch 'main' into cd-with-query

6aea3a3

Merge branch 'main' into cd-with-query

8317723

kidonng and others added 3 commits January 13, 2021 22:53

Simplify code

b807c7d

improve docs, use --tokenize

a6b8237

fix empty input bug

6467bc1

PatrickF1 reviewed Jan 13, 2021

View reviewed changes

PatrickF1 added 2 commits January 13, 2021 09:51

grammar

5d26504

qualify more

00e2773

PatrickF1 approved these changes Jan 13, 2021

View reviewed changes

PatrickF1 added 2 commits January 13, 2021 09:59

remove period

17f168d

immediately

49eaa65

PatrickF1 merged commit 5e15aeb into PatrickF1:main Jan 13, 2021

PatrickF1 deleted the cd-with-query branch January 13, 2021 18:02

PatrickF1 changed the title ~~Allow quick open with pre-populated query~~ Allow quick open with pre-populated query and non-directories Jan 13, 2021

kidonng mentioned this pull request Jul 4, 2021

Auto prepend ./ for quick directory change doesn't work if directory starts with . #171

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow quick open with pre-populated query and non-directories #76

Allow quick open with pre-populated query and non-directories #76

kidonng commented Jan 7, 2021 •

edited

Loading

PatrickF1 commented Jan 7, 2021

kidonng commented Jan 8, 2021 •

edited

Loading

PatrickF1 Jan 8, 2021

PatrickF1 Jan 9, 2021

kidonng Jan 9, 2021

kidonng Jan 9, 2021

PatrickF1 Jan 10, 2021 •

edited

Loading

PatrickF1 Jan 10, 2021

kidonng Jan 13, 2021

PatrickF1 Jan 13, 2021

PatrickF1 Jan 13, 2021

PatrickF1 Jan 13, 2021

PatrickF1 commented Jan 13, 2021

PatrickF1 Jan 13, 2021

PatrickF1 Jan 13, 2021

kidonng Jan 14, 2021

kidonng commented Jan 14, 2021

PatrickF1 commented Jan 14, 2021

Allow quick open with pre-populated query and non-directories #76

Allow quick open with pre-populated query and non-directories #76

Conversation

kidonng commented Jan 7, 2021 • edited Loading

PatrickF1 commented Jan 7, 2021

kidonng commented Jan 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PatrickF1 Jan 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PatrickF1 commented Jan 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kidonng commented Jan 14, 2021

PatrickF1 commented Jan 14, 2021

kidonng commented Jan 7, 2021 •

edited

Loading

kidonng commented Jan 8, 2021 •

edited

Loading

PatrickF1 Jan 10, 2021 •

edited

Loading