Add pkg_detail RPC for packages pane detail editor#1291
Conversation
jennybc
left a comment
There was a problem hiding this comment.
I made a few inline suggestions.
I know this PR is not really about the shape and look of the package detail editor. But I'll go ahead and mention that I'd probably include some things that aren't here (e.g. link to package's main site, if available, or its CRAN landing page) and omit or refactor some things that are here (e.g. dependency count seems less useful than just revealing those DESCRIPTION fields or giving the typical dependency/reverse dependency treatment seen on CRAN and METACRAN).
| installed <- rownames(utils::installed.packages()) | ||
| if (!nzchar(name) || !(name %in% installed)) { | ||
| return(NULL) | ||
| } |
There was a problem hiding this comment.
It will be more efficient to just go directly to utils::packageDescription(), which will return NA if the package is not installed. This avoids reading and parsing DESCRIPTION of all installed packages for this existence check. installed.packages() docs say:
It will be slow when thousands of packages are installed, so do not use it to find out if a named package is installed (use
find.packageorsystem.file) nor to find out if a package is usable (callrequireNamespaceorrequireand check the return value) nor to find details of a small number of packages (usepackageDescription).
| first | ||
| } | ||
|
|
||
| base_pkgs <- rownames(utils::installed.packages(priority = "base")) |
There was a problem hiding this comment.
Given the downsides to calling installed.packages() when it can be avoided, I would just hardwire these. They don't change. I have a history of doing same:
If you really want to compute it, I'd suggest:
tools:::.get_standard_package_names()$base
| out <- list(name = name, dependencyCount = length(deps)) | ||
|
|
||
| # Prefer Maintainer for author display; fall back to Author. | ||
| author <- clean(d$Maintainer) |
There was a problem hiding this comment.
FWIW a CRAN package absolutely must have a Maintainer. But I suppose this pane could populate from a non-CRAN source? Just thought I'd mention.
| if (!is.null(title)) { | ||
| out$title <- title | ||
| } | ||
| if (!is.null(author)) { | ||
| out$author <- author | ||
| } | ||
| if (!is.null(license)) { | ||
| out$license <- license | ||
| } | ||
| if (!is.null(repo)) { | ||
| out$sourceRepository <- repo | ||
| } | ||
| if (!is.null(published)) { | ||
| out$publishedDate <- published |
There was a problem hiding this comment.
If you definitely want to drop the NULLs (I guess that's favorable on the other side?), something along these lines might be more readable:
out <- list(
name = name,
title = title,
author = author,
license = license,
sourceRepository = repo,
publishedDate = published
)
Filter(Negate(is.null), out)
Adds a
.ps.rpc.pkg_detail(name)RPC that returns cheap, kernel-local detailmetadata for a single installed package. Positron's new package detail editor
calls this when a package is opened.
The RPC reads the installed package's
DESCRIPTIONviautils::packageDescription(...)and returns a named list with:namedependencyCount- count ofDepends+Imports+LinkingTo, excludingbase packages and
Ritselftitle- theTitlefield (when present)author-Maintainer, falling back toAuthor(when present)license- theLicensefield (when present)sourceRepository- theRepositoryfield, e.g. "CRAN" (when present)publishedDate- theDate/Publicationfield (when present)It returns
NULLfor an empty name or a package that isn't installed. No Rustchange is needed - the generic
.ps.rpc.<method>dispatch incrates/ark/src/ui/ui_comm.rsalready routes it.Companion Positron work: posit-dev/positron#12926 (the package detail
editor, which adds the
getPackageDetailpackage-manager hook that calls thisRPC). The Positron PR will bump the ark submodule to this change once merged.
Positron PR: posit-dev/positron#14440
QA
In an R session:
(Exact accessor depends on how the module is exposed; the function is
.ps.rpc.pkg_detailincrates/ark/src/modules/positron/packages_pane.R.)