Optional dependencies are (or should be) listed in a PKGBUILD in order of importance. We previously rejected outputting them in alphabetical order because of this. Given this patch just groups the installed/not installed nature of the optdepend, I suppose this is more reasonable, but I am still reluctant. I can see the concern, but I think the importance of having a package installed disappears once you already have it. Also, surely uninstalled optdepends are the more important ones and should be listed first. The idea was getting them closer to the end of the output, where your caret will be prepared to type them in and install.
Drew DeVault On 07/11/2013 08:43 PM, Allan McRae wrote:
On 12/07/13 04:43, Drew DeVault wrote:
Not sure how expensive alpm_db_get_pkg is, so it might be worthwhile to use something other than make_optstring to accomplish this if we want to avoid calling it four times per package. Optional dependencies are (or should be) listed in a PKGBUILD in order of importance. We previously rejected outputting them in alphabetical order because of this. Given this patch just groups the installed/not installed nature of the optdepend, I suppose this is more reasonable, but I am still reluctant.
Also, surely uninstalled optdepends are the more important ones and should be listed first.
Allan
Signed-off-by: Drew DeVault <sir@cmpwn.com> --- src/pacman/util.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 23c4009..c1d3dab 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1243,10 +1243,19 @@ void display_optdepends(alpm_pkg_t *pkg)
optdeps = alpm_pkg_get_optdepends(pkg);
- /* turn optdepends list into a text list */ + /* turn optdepends list into a text list, with uninstalled dependencies last */ for(i = optdeps; i; i = alpm_list_next(i)) { alpm_depend_t *optdep = i->data; - optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) != 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } + } + + for(i = optdeps; i; i = alpm_list_next(i)) { + alpm_depend_t *optdep = i->data; + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) == 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } }
if(optstrings) {