Bug 45625 - command_not_found_handle improvements
Summary: command_not_found_handle improvements
Status: RESOLVED NOTABUG
Alias: None
Product: PackageKit
Classification: Unclassified
Component: General (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Richard Hughes
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-04 10:30 UTC by Dan Nicholson
Modified: 2018-08-21 15:52 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
[PATCH] cnf: Use local variables in the shell handler (913 bytes, patch)
2012-02-04 10:38 UTC, Dan Nicholson
Details | Splinter Review
[PATCH] cnf: Quote the arguments passed to pk-command-not-found (1.01 KB, patch)
2012-02-04 10:38 UTC, Dan Nicholson
Details | Splinter Review
[PATCH] cnf: Inform the user of the missing command from the shell (2.44 KB, patch)
2012-02-04 10:38 UTC, Dan Nicholson
Details | Splinter Review
[PATCH] cnf: Only search for packages when shell is interactive (1.15 KB, patch)
2012-02-04 10:38 UTC, Dan Nicholson
Details | Splinter Review
[PATCH] cnf: Return errors immediately instead of storing return values (1.71 KB, patch)
2012-02-04 10:38 UTC, Dan Nicholson
Details | Splinter Review
[PATCH] cnf: Ask the user before searching the package database (1.65 KB, patch)
2012-02-04 10:38 UTC, Dan Nicholson
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Nicholson 2012-02-04 10:30:59 UTC
Here's a set of patches that tries to improve the handling in command_not_found_handler prior to handing off to pk-command-not-found. Some of the patches are just cleanup and correctness fixes, but the final one adds a check with the user prior to launching pk-command-not-found. I think that's the right thing to do since a lot of times you've just entered a typo and don't want to wait while your package database is searched.
Comment 1 Dan Nicholson 2012-02-04 10:38:29 UTC
Created attachment 56602 [details] [review]
[PATCH] cnf: Use local variables in the shell handler


Since this is a bash specific feature, we can use local variables to
not pollute the user's environment.
---
 contrib/command-not-found/PackageKit.sh.in |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Comment 2 Dan Nicholson 2012-02-04 10:38:33 UTC
Created attachment 56603 [details] [review]
[PATCH] cnf: Quote the arguments passed to pk-command-not-found


"$@" is a special variable in bash that expands to all the positional
parameters properly quoted. Essentially it means that any parameters
with spaces will be passed correctly as a single argument.
---
 contrib/command-not-found/PackageKit.sh.in |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Comment 3 Dan Nicholson 2012-02-04 10:38:37 UTC
Created attachment 56604 [details] [review]
[PATCH] cnf: Inform the user of the missing command from the shell


The shell missing command handler is the correct place to inform the
user of the missing command, and not from the tool doing the
replacement lookup.

This also has the benefit that the message can exactly match what bash
itself would have sent. It uses the actual interpreter name from $0
and the existing bash translations. Here's an example showing the
handler vs bare bash:

[dan@buster ~] su -
Password:
[root@buster ~]# LANG=de_DE
[root@buster ~]# blah
-bash: blah: Kommando nicht gefunden.
[root@buster ~]# unset -f command_not_found_handle
[root@buster ~]# blah
-bash: blah: Kommando nicht gefunden.
---
 contrib/command-not-found/PackageKit.sh.in       |    7 +++++--
 contrib/command-not-found/pk-command-not-found.c |    5 -----
 2 files changed, 5 insertions(+), 7 deletions(-)
Comment 4 Dan Nicholson 2012-02-04 10:38:42 UTC
Created attachment 56605 [details] [review]
[PATCH] cnf: Only search for packages when shell is interactive


If we're inside a script, we can't ask a user to install a package, so
just return with an error. Example:

$ banshee
-bash: banshee: command not found
Install package 'banshee' to provide command 'banshee'? [N/y]
$ bash -c '. /etc/profile.d/PackageKit.sh; banshee'
bash: banshee: command not found
---
 contrib/command-not-found/PackageKit.sh.in |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Comment 5 Dan Nicholson 2012-02-04 10:38:49 UTC
Created attachment 56606 [details] [review]
[PATCH] cnf: Return errors immediately instead of storing return values


Instead of storing return values and logic in local variables, just
return immediately if we can't search the database. If we can run the
search command, let its exit code be the return code for the function.
---
 contrib/command-not-found/PackageKit.sh.in |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)
Comment 6 Dan Nicholson 2012-02-04 10:38:54 UTC
Created attachment 56607 [details] [review]
[PATCH] cnf: Ask the user before searching the package database


Often the user has simply mistyped the command when the shell can't
find it and would rather not wait while PackageKit roots around
looking for the missing package. Ask the user to continue before
searching the package database. This defaults to no and will assume no
after a 5 second timeout. If the user doesn't want to be bothered, they
can just hit return to get back to their prompt.
---
 contrib/command-not-found/PackageKit.sh.in |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
Comment 7 Daniel Miranda 2014-04-01 17:27:52 UTC
I've started getting spurious attempts of package installation after upgrading to GNOME 3.12 (unnoficial packages from http://copr.fedoraproject.org/coprs/rhughes/f20-gnome-3-12/) and getting a PackageKit update coupled with it.

I get installation prompts even for non-interactive shells, which is very annoying. It seems Dan Nicholson's patches improve the situation significantly, so applying them would be very helpful from an UX perspective.
Comment 8 Richard Hughes 2014-04-05 11:22:48 UTC
Comment on attachment 56602 [details] [review]
[PATCH] cnf: Use local variables in the shell handler

Review of attachment 56602 [details] [review]:
-----------------------------------------------------------------

Looks good, pushed; thanks.
Comment 9 Richard Hughes 2014-04-05 11:22:51 UTC
Comment on attachment 56603 [details] [review]
[PATCH] cnf: Quote the arguments passed to pk-command-not-found

Review of attachment 56603 [details] [review]:
-----------------------------------------------------------------

Looks good, pushed; thanks.
Comment 10 Richard Hughes 2014-04-05 11:22:54 UTC
Comment on attachment 56605 [details] [review]
[PATCH] cnf: Only search for packages when shell is interactive

Review of attachment 56605 [details] [review]:
-----------------------------------------------------------------

Looks good, pushed; thanks.
Comment 11 Richard Hughes 2014-04-05 11:23:48 UTC
I've pushed 1,2,4 -- but I'm not so sure about the rest. I'll review more when I get home. Thanks.
Comment 12 Richard Hughes 2018-08-21 15:52:45 UTC
We moved the upstream bugtracker to GitHub a long time ago. If this issue still affects you please re-create the issue here: https://github.com/hughsie/PackageKit/issues
 
Sorry for the impersonal message, and fingers crossed your issue no longer happens. Thanks.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.