-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(_filedir): add -f
to manually suffix / to directory names
#847
base: main
Are you sure you want to change the base?
Conversation
bash_completion
Outdated
# Ignored with `-d`. | ||
# OPTIONS | ||
# -C dir Change to dir before generating completions | ||
# -d Complete only on directories | ||
_filedir() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem in this approach is that the -o filenames
at end of the function won't take effect with -C somewhere
as we are no longer in the dir we changed to at the end. So e.g. appending slashes to dir names does not happen. Not sure what to do about that.
(Commenting here because it seems the UI here won't let me comment that far away from changed lines.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing the compopt -o filenames
inside each compgen
subshell doesn't seem to make any difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this is a good point. But after another thinking of it, I guess this issue also existed in the other approach in #827, so I think this is not an issue specific to this approach.
If we are to solve this issue within this PR, maybe we need to manually suffix a slash in the shell code.
- One way is to test the filename in a
for
loop and suffix/
if it is a directory name. - Another way is to use
compgen -S / -d
for directory names, and get non-directory filenames using another way (find
, etc.). - Or we may switch between the above two based on the number of files in the specified directory.
Maybe we don't have to too much care about it, but the opposite issue is that when a filename in the specified directory has the same name as a directory in the current directory, it would be unexpectedly suffixed by /
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the problem existed in the other implementation in #827, I don't know why I didn't notice that. In my later tests I also noticed that it ends up changing the dir to /usr/share/mime in various scenarios, something I didn't notice originally either for some reason.
I'm going to try out yet another different approach for #827 before returning to this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweaked some, tests pass but there are still issues:
$ xx() { _filedir -C foo; }
$ mkdir -p foo/bar foo/quux
$ complete -F xx x
$ x <TAB>
bar/ quux/ # <-- this is all good
$ x ba<TAB>
bar/ quux/ # <-- this is not
The problem of filename elsewhere matching a dir in current dir you mentioned also persists.
Obsoleted by #973 |
Let me reopen this. The problem that the suffix slash is not added to directory names for the paths generated in another directory is not solved. Also, I'd like to use the feature to suffix slashes to directory names to use it in another PR. There are many cases where the suffix slash is not added because the generated word does not exactly match a directory name in the current directory. In addition to the cases we change the directory, this also happens when the generated words are prefixed by |
5a07301
to
1cce580
Compare
Rebased and added a fix. Now |
1cce580
to
3595595
Compare
-C
, use it-f
to manually suffix / to directory names
3595595
to
06e3f55
Compare
06e3f55
to
925defb
Compare
Co-authored-by: Koichi Murase <[email protected]>
If we are completing entries somewhere else besides the current dir, `compopt -o filenames` won't do its usual thing; append slashes ourselves. Co-authored-by: Koichi Murase <[email protected]>
This fixes test_15d in test/t/unit/test_unit_filedir.py.
925defb
to
d461b61
Compare
This attempts to implement
_filedir -C
as discussed in #827.