use fzf to navigate through completion proposals #10
Conversation
note: code formatting using `black`
|
|
||
| if autojump.returncode == 0: | ||
| if autojump_stdout.rstrip("\n"): | ||
| fzf = self.fm.execute_command( |
There was a problem hiding this comment.
We're executing the same exact command a second time? Can't we just send the results from the first run to fzf?
There was a problem hiding this comment.
yes, there's probably a better way to do it
| directory = subprocess.check_output(["autojump", self.arg(1)]) | ||
| directory = directory.decode("utf-8", "ignore") | ||
| directory = directory.rstrip("\n") | ||
| self.fm.cd(directory) |
There was a problem hiding this comment.
Can we do an early return to save some indentation?
There was a problem hiding this comment.
I'm not sure, if fzf is absent, an action must still be performed.
| universal_newlines=True, | ||
| stdout=subprocess.PIPE, | ||
| ) | ||
| fzf_stdout, fzf_stderr = fzf.communicate() |
There was a problem hiding this comment.
Isn't there are more modern way to do this? The first branch uses subprocess directly, maybe we can align all of them and get rid of communicate? Or is this the recommended way?
There was a problem hiding this comment.
To be honest, I don't know much about python ;-) , but reading the documentation, there are ways to make things simpler.
| self.fm.cd(fzf_file_path) | ||
| else: | ||
| self.fm.select_file(fzf_file_path) | ||
| else: # sdtout is empty |
There was a problem hiding this comment.
Shouldn't we do the same handling in the first branch, without fzf?
There was a problem hiding this comment.
a priori no, because in this first case we don't try to navigate through the proposals, we jump directly to the path with the highest weight in the history
| ) | ||
| fzf_stdout, fzf_stderr = fzf.communicate() | ||
|
|
||
| if fzf.returncode == 0: |
There was a problem hiding this comment.
Good point, the user should be notified of the error
|
First of all, thank you for taking the time to provide this detailed feedback :-). There are a lot of little things to review. I'll update this first version as soon as possible. |
Hello,
I've extended the script to allow navigation through the possible completions proposed by
autojumpin casefzfis present in the PATH.