add the ability to use fzf on unknown points. #12
Conversation
|
@schorsch3000 thanks for this! I wasn't aware of I would appreciate a test or two and some documentation, but I definitely like the sentiment behind this PR. Good luck, and let me know if you have any questions! |
|
Also, 👍 👍 👍 on the comprehensive PR description 😍 |
|
One other thought I had: if the goal is just to provide 'fuzzy' completion (i.e. not just from the beginning), that might be accomplished just by changing the regular expressions etc. in the |
|
I've wrapped a lot of shellscripts around fzf since i learned about it, i'm quite fast with it, maybe that's why i've choose to use fzf instead of a fuzzy-regex for autocompletion. So, i've removed the ENV-var that enabled fzf and use it whenever a target-point is not availible and fzf is in $PATH. I've added a message if the target is nonexistent and fzd i not in PATH. I'm not sure if i as a user not using fzf would be happy to be pointed to it every time i mistyped a point or not. I've also added some test-cases using shpec, and man is that awesome, i'm going to use that from now on. And finally, i've documented that there is a fzf-feature. |
|
Hi, |
|
Sorry @schorsch3000, feedback incoming. |
rylnd
left a comment
rylnd
left a comment
There was a problem hiding this comment.
@schorsch3000 this is looking great, thanks. There're just a few style quibbles and some portability issues (OSX in particular).
sd
Outdated
|
|
||
| type fzf >/dev/null 2>&1 || { | ||
| echo "Can't shift to point '$requested_point' because it doesn't exist." | ||
| echo "Also fzf issn't in path so we can't fuzzy find a match, sorry." |
There was a problem hiding this comment.
I'd leave the messaging as it was; there's no need to remind the user of the features they haven't opted into.
There was a problem hiding this comment.
Sure, i'll just remove tha extra line :)
sd
Outdated
| echo "Also fzf issn't in path so we can't fuzzy find a match, sorry." | ||
| return 1 | ||
| } | ||
| point_path="$sdd/$(find "$sdd" -maxdepth 1 -type l -printf "%f\n"|fzf -q "$*")" |
There was a problem hiding this comment.
The OSX version of find does not have the printf option. Can you do this another way?
There was a problem hiding this comment.
Sure, -exec should do the trick.
sd
Outdated
| if [ "$?" -ne "0" ] ; then | ||
| return 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
Please delete unnecessary whitespace here.
shpec/sd_shpec.sh
Outdated
| end | ||
| describe "using fzf" | ||
| it "should use fzf if availible" | ||
| cd /tmp |
There was a problem hiding this comment.
Indentation should be two spaces, please.
- remove extra message about fzf not existent - osx find not havong -printf, using -exec basename instead - multiple whitespace issue
Conflicts: sd shpec/sd_shpec.sh
|
having |
|
should we do anything about the warning while running tests? |
3 participants
What?
This pullrequest allows
sdto run fzf on a non-found target.Why?
i use sd on several boxes with auto-generated targets.
E.g.: a webserver with a folder per vhost.
Often i don't remember if the folder is called example.com or www.example.com or even www.example.net (and the others are aliases).
The
sdautocompletion can't help here.with that feature i'm able to call
sd exampleand getfzfprefiltered.How was it tested?
At this point in time: only by using it.
Additional work for this pull request
I don't expect you to merge this pull request in it's current state.
This is the code i'm using, it's not tested and not documented.
If this change is okay for you to have, i would prefer to get familiar with
shpec, write some tests, document that feature before that gets merged.So, what do you think?