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

[Laptop765]

Describe the bug
When selecting a directory that begins with ., the result is not prepended with ./ and fish gives an error: fish: Unknown command: .config.

To Reproduce

1. set -x fzf_fd_opts -H
2. cd to a folder with folders beginning with .
3. C-f to trigger __fzf_search_current_dir
4. Select a folder such as .config that begins with a . and press enter to select it
5. Press enter again

Expected behavior
Prepend with ./ so that the second enter does a cd into the folder

Screenshots
N/A

Environment

• Fish: 3.1.2
• fzf.fish: not sure how to check but I did run fisher update prior to posting this bug so it should be latest as of this bug report
• plugin manager: fisher 4.3.0
• terminal: kitty
• OS: debian-based

Additional context
This appears to have been introduced by b19e3f8 which has a regex check for a path used as the base directory and as a side effect explicitly does not add the ./ to folders beginning with ..

[Laptop765]

I'd be happy to take a shot at fixing this with some guidance unless it's a quick fix for someone else to do.

[PatrickF1]

Hi Paul! Welcome and thanks for reporting the bug.
I've been able to repro this. But just to clarify, when you said

fish gives an error: fish: Unknown command: .config.

You mean if you try to hit enter right after selecting .config from fzf, right?

[PatrickF1]

@kidonng got any ideas on how to fix this?

[Laptop765]

Hi Paul! Welcome and thanks for reporting the bug.
I've been able to repro this. But just to clarify, when you said

fish gives an error: fish: Unknown command: .config.

You mean if you try to hit enter right after selecting .config from fzf, right?

That is correct!

Edited the original for clarity.

[kidonng]

@kidonng got any ideas on how to fix this?

I guess just adjust the regex to only exclude leading /. There is no reason for excluding leading ., from what I can tell.

[Laptop765]

It also looks like the test added in that commit only covers / and not ..

[Laptop765]

One fix could be to remove the . from the regex but another could be to change it so that it skips if it starts with / or ./ (as opposed to just .) so that a pre-typed token of ./afolder wouldn't become ././afolder which I think would fit the intention of the original fix.

[Laptop765]

I'm not a regex expert but I think the original fix:
^[.|/]
should have been
^(.|/)
for alternation, whereas the original is a character class.

The new regex could be simplified to something like:
^\.?/ i.e. start with an optional literal . and then followed by a / so that both /whatever and ./whatever would be exempted from the ./ prepending but not .whatever.

[PatrickF1]

The new regex could be simplified to something like:
^.?/ i.e. start with an optional literal . and then followed by a / so that both /whatever and ./whatever would be exempted from the ./ prepending but not .whatever.

That's very creative and great in-depth analysis. I think the regex ^(.|/) doesn't quite work. Upon further analysis, the correct regex is ^/|(./).

However, all of this thinking in this area made me realize a better, cleaner solution

• since prepending ./ is causing issues when the path starts with . or /, why not just append /? fd never appends / to its output so this is safe to do (see Display directories with trailing slashes sharkdp/fd#436)
• of course, we can't just append / to everything; we need to test if the path is actually a directory, but since we only do this when there's one path selected, it's not performance hindering
• a / at the end of only dirs is cleaner, natural, and has a more comprehensible than always prepending ./ when there is one selection
• I prototyped the code and the documentation and they are much more straightforward
• this will remove support for executables, which I think is not a feature I wanted to maintain anyway

The PR for this is #185.

[kidonng]

I think the regex ^(.|/) doesn't quite work.

Can you give some failing examples?

Upon further analysis, the correct regex is ^/|(./).

I think you mean ^(/|./) or ^/|^./. But isn't they already covered by ^(.|/)?

since prepending ./ is causing issues when the path starts with . or /

Only when path starts with / because prepending ./ will make it invalid. ././ is valid, just looks not nice.

fd never appends / to its output so this is safe to do

You quoted "I think I'm inclined to add this to fd by default. I don't really see any drawbacks." and that's in contrary to "this is safe to do".

but since we only do this when there's one path selected, it's not performance hindering

The current way doesn't need checking and hence has no performance hindering.

a / at the end of only dirs is cleaner, natural, and has a more comprehensible than always prepending ./ when there is one selection

That's exactly the thoughts behind #76, i.e. the current behavior.

this will remove support for executables, which I think is not a feature I wanted to maintain anyway

I can't say I use that often, but occasionally it's very nice. I will miss it if you really decided to drop it.

---

TL;DR I'm opposed to reverting to appending / because

• It doesn't really reduce maintenance burden
• It doesn't support executables
• It has the potential to alter other program's behavior

[PatrickF1]

Can you give some failing examples?

It excludes hidden files. In fact, it's no different from ^[.|/] other than that it captures.

I think you mean ^(/|./) or ^/|^./. But isn't they already covered by ^(.|/)?

No I tested them using a regex tester on different paths.

You quoted "I think I'm inclined to add this to fd by default. I don't really see any drawbacks." and that's in contrary to "this is safe to do".

You're right. But if or when fd appends / to directories outputted, then I can just remove all these checks to append / to directories, because they'll already be there. Search directory's code will become much simpler.

The current way doesn't need checking and hence has no performance hindering.

Right, but we're talking probably less than 1 ms difference.

That's exactly the thoughts behind #76, i.e. the current behavior.

Are you saying you think prepending ./ is more natural than appending /?

It has the potential to alter other program's behavior

True, it affects symlinks. But remember that if you already start typing something into the command line, e.g. rm -rf and then execute this function, it won't append /.

I can't say I use that often, but occasionally it's very nice. I will miss it if you really decided to drop it.

That's good to hear you've been using it. But I think you represent one of the more advanced power users of fzf.fish. I need to consider all fzf.fish users, and for many of them I suspect this feature is not used and prepending ./ is more confusing and unnatural than appending /. However, the main reason I want to remove it, as I've stated before, I deeply regret how much feature creep has gone into search directory and I think it's time to cut back. I am losing time and motivation because of maintaining a feature I don't use very much (look up the commit history to see how many bugs we've had to fix in search directory; I'm really sick of it). By removing one small feature, it should reduce the surface area and complexity that is causing bugs. If you look at my PR, the code and documentation shrinks and becomes easier to read. I'm really sorry I'm going to remove this feature you originally worked on. I'm still grateful for all the work you've poured into this and hope you will continue to do so. I hope you can understand. However, I will await your reply before proceeding because I find that 50% of the time, you convince me to change course anyway--I respect that your judgment, experience, and shell prowess is generally greater than my own.

[kidonng]

However, I will await your reply before proceeding because I find that 50% of the time, you convince me to change course anyway

I just convinced myself this is not in line with what Fish does: they append / at end of the directory in tab completions. Also, I think prepending ./ for executables is not very safe either. More importantly, it is causing trouble here and there. So I'm happy to accept the change as-is.

I respect that your judgment, experience, and shell prowess is generally greater than my own.

While I do not consider myself to be of anything you described, thanks for your kind words. I love your carefulness, inclusive thoughts as detailed documentation as well.

[PatrickF1]

I just convinced myself this is not in line with what Fish does: they append / at end of the directory in tab completions. Also, I think prepending ./ for executables is not very safe either.

Great! Excellent points. Even if you didn't agree, I hope you know that I very much value your insight and contributions and it hurts me as well to reject your excellent ideas. It's usually not an easy call.