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

bash-completion: use sqlite cache when available (BZ: 1815895) #1817

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

rjarry
Copy link
Contributor

@rjarry rjarry commented Mar 2, 2022

Use /var/cache/dnf/packages.db to make SQL requests instead of parsing the repodata files at each tab completion. This makes completing package names much snappier.

This is an inspiration from zsh dnf completion: https://github.com/zsh-users/zsh/blob/zsh-5.8.1/Completion/Redhat/Command/_dnf#L7-L33

= changelog =
msg: Use sqlite cache to make bash completion snappier
type: enhancement
resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1815895

Use /var/cache/dnf/packages.db to make SQL requests instead of parsing
the repodata files at each tab completion. This makes completing package
names much snappier.

This is an inspiration from zsh dnf completion:
https://github.com/zsh-users/zsh/blob/zsh-5.8.1/Completion/Redhat/Command/_dnf#L7-L33

= changelog =
msg: Use sqlite cache to make bash completion snappier
type: enhancement
resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1815895
@rjarry rjarry changed the title bash-completion: use sqlite cache when available bash-completion: use sqlite cache when available (BZ: 1815895) Mar 17, 2022
@rjarry
Copy link
Contributor Author

rjarry commented Apr 4, 2022

Gentle bump :)

@kontura kontura self-assigned this Apr 25, 2022
@kontura
Copy link
Contributor

kontura commented Apr 25, 2022

I like the change in general.
We have a plugin in dnf-plugins-core that creates and updates the packages.db cache but as far as I can tell we are not using it anywhere so it is kind of wasted.

It provides less correct completions though.
For example when running update it completes all installed packages but not all of then can have updates available.
Also when running with for example with --disable/enablerepo, --repo, --repofrompath or --installroot the result can be misleading. Maybe we could at least add a check for those options and not use the cache in that case?

Also also we already have a PR with similar approach: #1100

@rjarry
Copy link
Contributor Author

rjarry commented Apr 25, 2022

I can send a changed version that bypasses the completion cache when --disable/enablerepo, --repo, --repofrompath or --installroot are used. I can do the same thing for update if you want.

@rjarry
Copy link
Contributor Author

rjarry commented Apr 25, 2022

I had not seen #1100 which is indeed the exact same thing.

@rjarry
Copy link
Contributor Author

rjarry commented Apr 26, 2022

I did not find a way to detect if --disable/enablerepo, --repo, --repofrompath or --installroot are used. I am not sure if the current implementation takes these options into consideration when completing package names. I could be wrong though.

I have bypassed the completion cache for update and its related commands. However, from a user experience point of view, I wonder if having less accurate completion may be better than having the terminal freeze for several seconds at each tab completion. In the worst case, users may try to update a package which has no update candidates. To see which updates are available, they can always use list updates.

I am waiting for your feedback before proceeding. Thanks.

@kontura
Copy link
Contributor

kontura commented Apr 27, 2022

I did not find a way to detect if --disable/enablerepo, --repo, --repofrompath or --installroot are used. I am not sure if the current implementation takes these options into consideration when completing package names. I could be wrong though.

I see yes you are correct. It doesn't take them into account either so doing the fallback wouldn't help much.

I have bypassed the completion cache for update and its related commands. However, from a user experience point of view, I wonder if having less accurate completion may be better than having the terminal freeze for several seconds at each tab completion. In the worst case, users may try to update a package which has no update candidates. To see which updates are available, they can always use list updates.

I also prefer faster feedback.

Ok, thanks for the contributions!

@kontura kontura merged commit c1a407e into rpm-software-management:master Apr 27, 2022
grumpey added a commit to grumpey/dnf that referenced this pull request Sep 4, 2023
grumpey added a commit to grumpey/dnf that referenced this pull request Sep 18, 2023
grumpey added a commit to grumpey/dnf that referenced this pull request Sep 18, 2023
grumpey added a commit to grumpey/dnf that referenced this pull request Sep 18, 2023
grumpey added a commit to grumpey/dnf that referenced this pull request Sep 18, 2023
j-mracek pushed a commit that referenced this pull request Sep 20, 2023
#1817 added the ability to utilize sqlite cache when available.
https://discussion.fedoraproject.org/t/why-bash-auto-completion-is-so-slow-with-dnf/88944
and https://discussion.fedoraproject.org/t/dnf-autocompletion-is-too-slow/64038
Are users noting that bash completion is slow without sqlite installed.

= changelog =
msg:   [spec] Add Recommends %{_bindir}/sqlite3 for bash-completion for Fedora
type:  enhancement
related: #1817
related: https://discussion.fedoraproject.org/t/why-bash-auto-completion-is-so-slow-with-dnf/88944
related: https://discussion.fedoraproject.org/t/dnf-autocompletion-is-too-slow/64038
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants