Skip to content
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

Autosuggestions suggest non-existing files #10414

Open
rickalex21 opened this issue Apr 4, 2024 · 10 comments
Open

Autosuggestions suggest non-existing files #10414

rickalex21 opened this issue Apr 4, 2024 · 10 comments

Comments

@rickalex21
Copy link

fish, version 3.7.1
Linux void 6.6.22_1 SMP PREEMPT_DYNAMIC Fri Mar 15 23:56:15 UTC 2024 x86_64 GNU/Linux
xterm-kitty
no

This is not a problem more of a feature?

Is there a way to make the fish shell smarter? For example, if file foo does not exist
do not suggest it.

When I do: cp /mn it will suggest files that do not exist anymore. Same thing with rmdir etc...

The output suggestion is this:

cp /mnt/an-old-file-I-deleted-last-year

I would perfer that it suggests files that actually do exist.

Thanks

@zanchey
Copy link
Member

zanchey commented Apr 4, 2024

So the problem is that fish doesn't know which arguments are files and which aren't (though paths with a / could be picked up), and whether the argument should exist or shouldn't. For example, mv existing nonexisting is a perfectly reasonable command to suggest, but mv nonexisting existing isn't.

Without adding special knowledge of particular commands I'm not sure if this would be possible. I thought we had considered and rejected the idea of programmable suggestions (as opposed to completions) in the past, but I can't find any discussion with a brief search.

@faho faho changed the title Fish suggests deleted files, smarter fish shell. Autosuggestions suggest non-existing files Apr 4, 2024
@rickalex21
Copy link
Author

rickalex21 commented Apr 4, 2024

I think instead of integrating into fish, perhaps a plugin system would be better to maniplulate the
way this is done. This way users can modify to thier liking. I'm not sure, I'm not familar with plugins.
Another thing to consider is how much would this affect speed. Suggestions are a fraction of a second
so every ms counts.

@mqudsi
Copy link
Contributor

mqudsi commented Apr 16, 2024

As at the time that the history is retrieved it is not possible to know if an argument is a path that previously existed or not, IMHO the correct way to do this (if we are to do this at all, read on) would be for the history file to include information about which arguments were paths (e.g. a "column" paths that contains something like 1,3,5 indicating that arguments 1, 3, and 5 are/were valid paths at the time the command was executed. At the time of history retrieval, it would be possible to see if these paths existed or not before showing the history entry.

This would also handle the case where commands are expected to take paths that don't yet exist (e.g. touch README.md should be suggested when README.md doesn't already exist because the command creates it).

But I'm not sure it is a universally good approach because there's absolutely no rule that says retrieved history lines are executed as-is. If I were to venture a guess, I would say that well over 50% of the time when I am retrieving something from history that involves a path, I am going to edit it to reflect my current needs before execution. If I have a complicated command that wraps over 12 lines that took me 5 minutes to come up with but the payload of the input file argument -i foo.in doesn't exist, I still absolutely want that history suggestion, I'm just going to complete from history then edit the command to change -i foo.in to -i bar.in instead.

(This applies for autosuggestions or history search suggestions equally.)

It might still be useful for the history file to include that information for other purposes, but not so that the autosuggestion can be hidden.

@krobelus
Copy link
Member

would be for the history file to include information about which arguments were paths (e.g. a "column" paths that contains something like 1,3,5 indicating that arguments 1, 3, and 5 are/were valid paths at the time the command was executed

fish already does this.

For example If I remove a directory, my history file will have

- cmd: rmdir some.path
  when: 1713298898
  paths:
    - some.path

and rmdir some.path will no longer be autosuggested until I recreate the directory.

Note sure why it's not working for @rickalex21. Can you check if your history file is missing the paths entry with /mnt/an-old-file-I-deleted-last-year?

If I have a complicated command that wraps over 12 lines that took me 5 minutes to come up with but the payload of the input file argument -i foo.in doesn't exist, I still absolutely want that history suggestion

I use multiline commands in that case so I always need to use history search as of today

@mqudsi
Copy link
Contributor

mqudsi commented Apr 22, 2024

fish already does this.

I thought that was the case but doubted myself from the conversation!

@johnr14
Copy link

johnr14 commented Apr 26, 2024

This is a valid issue that I am trying to fix on my end. Sometime, suggestions just don't make sens, and it's weird edge case that are hard to describe. Most of the time, suggestions are ok.

But I do agree with @mqudsi that history search is a separate thing and that you may want to edit an old command with invalid paths.

I believe that fish's autosuggestion is a core feature. I don't want to have AI add better suggestion, but making it smarter would certainly help.

Short

Autosuggestions fish suggests commands as you type, based on command history, completions, and valid file paths.

I think that we expect autosuggestion to test file paths from history to ensure they are valid when they need to be.

I would also expect autosuggestion to parse history and extract token for known completions and suggests them, see yt-dlp example.

Long

@krobelus I'll illustrate how this affects me and how I am trying to solve it. Also, I think that rmdir will not be suggested for a path that doesn't exist anymore, but fish will still suggest older commands that had that path as a token ? Like for cp.

An other example :

mkdir TEST/test -p
cd TEST
touch test/file.txt
cd ..

Fish history :

- cmd: mkdir TEST/test -p
  when: 1714138074
- cmd: cd TEST/
  when: 1714138077
  paths:
    - TEST/
- cmd: touch test/test.txt
  when: 1714138087
- cmd: cd ..
  when: 1714138089
  paths:
    - ..

Then writing touch, the first autosuggestion will be test/file.txt. This doesn't make sense as the folder test is non-existent in the current directory ! Also note that paths has .., but how can that be meaningful if we don't know the current path ?

@rickalex21 should say if it's consistent with what he mentioned. I may have over complexified the issue for a broader discussion while @rickalex21 may just want a simple test on the suggestion's token if it's a path to see if it still exist.
To do that, we may need more context in fish_history as @mqudsi mentionned.

An other example not related to paths :

yt-dlp has completion so it will suggest --audio-format if --au is typed. But after it will not lookup history and match that I usually set the next token to mp3. It will only do so if the previous part of the commandline is identical. This may be unrelated to file paths, but it's just annoying that autosuggestion doesn't work that way when it's what I would expect it to do.

Problematic situations

The worst cases are :

  • the usage of relative path for files or directories like : ../someother/dir and ../README.md
  • deleted files and directories
  • created files and directories.

When the current directory is changed the suggestion to someother/dir is not valid anymore but still suggested.

For example, if I use it nvim, it may suggest a file that doesn't exist in the current directory like laundry.md. However, it may make sens to sometime make such suggestion, for example to create a README.md in a new git directory as @mqudsi said.

How I tried to address this in a wrong way

To mitigate this, I was trying a few things :

I added a function commandline-paths-to-absolute from #3368 and left it binded to alt-a. I was thinking to add it my execute-or-preexec that's binded to \r.

It would

  • convert valid files and directory entered on the command line to absolute
  • reprocess the last history command to find if a file or a directory was created and convert it to absolute.
function execute-or-preexec
    # this is to merge history on exter with no arguments
    set -l cmd (commandline)
    if test -z "$cmd"
    #echo -n "Debug: history merge"
        history save
        history merge
    end
    #commandline-paths-to-absolute
    #history-last-cmd-paths-to-absolute # TODO
    commandline -f execute
end

bind \r execute-or-preexec

The new problem

The issue is handling special cases !
Let's say a file named push exist, in case I ran git push in that directory, it would rewrite push as a relative path ! /home/user/some/path/push.

So I was thinking of processing completions/ and extract --long ARG and --arguments ARG and have matching token be excluded from the -to-absolute. Then all this gets quite complex... I think this is the direction it should take, but not how it should be done. I think it would be better to extend fish_history and autosuggestion and not by parse the commandline like I'm trying to do.

An other more simple way with fish_history and autosuggestion test for validity

If fish history had the pwd of where a command was executed, it would be easy to test tokens for validity. As @mqudsi said :

is not possible to know if an argument is a path that previously existed or not

So we could look to resolve that with a cpath and dpath for created and deleted paths of tokens.
Also, it would leave the command line as typed, not rewriting path to absolute-path.

An old issue #120 is still opened to extend fish's history with additional information like PWD:

In summary

  • fish_history :
    • paths: contains relative paths to dir and files;
      • Should it always be absolute ?
      • Can there be cases that relative path may be needed, like /mnt/USB-DRIVE if it is remounted as /mnt/usb-drive having relative path would make autosuggestion work if in the root of the drive while absolute path will not.
      • And exclude .. as valid path !
    • missing PWD:, required for relative paths
    • missing information on tokens that are paths if they existed before/after the command was ran -> dpaths and cpaths ?
  • token:
    • edge case where a file exist in the current directory and a token on the commandline is the same like push
  • autosuggestion
    • how to identify
      • wanted valid suggestion of file to create ie. nvim README.md if it doesn't exist
      • unwanted suggestion of non-existing filenames ie nvim id_ed255519_@github but still keep id_ed255519 as valid suggestion for a file to create ?
    • validate if a command require a valid path token like cp and it's the issue mentioned by @rickalex21
    • check next token in history for a completion argument ie. mp3 for --audio-format of yt-dlp.

@rickalex21
Copy link
Author

the history file to include information about which arguments were paths

@mqudsi Extra metadata would be something to consider.

Can you check if your history file is missing the paths entry

@krobelus Ya, by the looks of it the paths are missing when the file does not exist.
I don't use hyprland but I tried it once and the file is still suggested:

 file /tmp/hyprpm/hyprland/subprojects/wlroots/build/meson-logs/meson-log.txt
/tmp/hyprpm/hyprland/subprojects/wlroots/build/meson-logs/meson-log.txt: cannot open `/tmp/hyprpm/hyprland/subprojects/wlroots/build/meson-logs/meson-log.txt' (No such file or directory)

Fish history:

- cmd: file /tmp/hyprpm/hyprland/subprojects/wlroots/build/meson-logs/meson-log.txt
  when: 1711668222

the suggestion's token if it's a path to see if it still exist

@johnr14 I think path is only one, I'm sure there are other commands like rmdir etc..

I'm not even in the directory that has na.fish and will suggest rm na.fish. In a sense
that is path. It's not in my path though.

Zoxide has a great approach to their system in which you can remove. However, it would be
too cumbersome to have to not only remove the directory but remove the erroneous suggestion
from fish if needed.

More metadata added to the history file could be an option as long as it does not interfere with
performance.

@krobelus
Copy link
Member

- cmd: file /tmp/hyprpm/hyprland/subprojects/wlroots/build/meson-logs/meson-log.txt
  when: 1711668222

This looks like when you ran that command, meson-log.txt did not exist.
In that case, fish should autosuggest it.

I'll make shift-delete delete the current autosuggestion from history.

mkdir TEST/test -p
cd TEST
touch test/file.txt
cd ..

Then writing touch, the first autosuggestion will be test/file.txt. This doesn't make sense as the folder test is non-existent

Yeah that can be improved.
I don't see what can be improved about the relative path case.

I would also expect autosuggestion to parse history and extract token for known completions and suggests them, see yt-dlp example.

yeah that's unrelated and it needs design work, best to move to a different ticket.

  • unwanted suggestion of non-existing filenames ie nvim id_ed255519_@github but still keep id_ed255519 as valid suggestion for a file to create ?

not sure what you mean, are not both valid suggestions?

@johnr14
Copy link

johnr14 commented Apr 27, 2024

I am looking for documentation about autosuggestion and can't seem to find much.
Is it all written in source code and not in fish script ?

Then writing touch, the first autosuggestion will be test/file.txt. This doesn't make sense as the folder test is non-existent

Yeah that can be improved.

Do a check on file path to check that all directories exist before suggesting it should fix it right ? If it's just a filename, it's ok as it will be created, but not many application will create an non-existing path to create a file.

unwanted suggestion of non-existing filenames ie nvim id_ed255519_@github but still keep id_ed255519 as valid suggestion for a file to create ?

not sure what you mean, are not both valid suggestions?

The example was that a previous file named id_ed255519_@github was created. The next time, it could be a other key like id_ed255519_@othersite. So it should not suggest id_ed255519_@github that was already created, as is would be accessible with a , but I would like that it suggest id_ed255519 so I can type the new command quickly.

For new files, it would be nice to have some regex matching rules that can be user customized.
so for instance, if I type :
nvim ~/.ssh/ I could specify that it should first show config then id_ed255519 then list files present in the directory.

Is there a way to do that ?

@rickalex21
Copy link
Author

rickalex21 commented Apr 27, 2024

This looks like when you ran that command, meson-log.txt did not exist.

@krobelus I can confirm that is the case, ls non-existent will continue to suggest non-existent.
At some point fish knows that non-existent does not exist because it's logged without a path
using that logic perhaps would be able to not suggest it in the future unless it's created later.
That's the tricky part.

Do a check on file path to check that all directories exist before suggesting it should fix it right ?

@johnr14 I'm not sure how well that would scale cause it sounds like O(n), a hashmap (key value) approach would prob
be better if there was a way to implement it.

@krobelus krobelus added this to the fish-future milestone May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants