Fix Portage backend #24
Conversation
helm-system-packages-portage.el
Outdated
| (defun helm-system-packages-portage () | ||
| "Preconfigured `helm' for Portage." | ||
| (unless (helm-system-packages-missing-dependencies-p "eix" "qlist" "euse" "portageq" "genlop") | ||
| (helm :sources (list (helm-system-packages-portage-build-source) | ||
| 'helm-system-packages-portage-use-source) | ||
| (helm :sources (list (helm-system-packages-portage-build-source) helm-system-packages-portage-use-source) |
There was a problem hiding this comment.
I'll leave the linebreak here, it's a bit too long otherwise.
| @@ -373,7 +373,7 @@ Return the result as a string." | |||
|
|
|||
| With prefix argument, insert the output at point. | |||
| Otherwise display in `helm-system-packages-buffer'." | |||
| (let ((res (helm-system-packages-call command args))) | |||
| (let ((res (apply 'helm-system-packages-run (cons command args)))) | |||
There was a problem hiding this comment.
-print is deprecated. We should eventually move to -show-information. But that can wait for the next pull request :)
There was a problem hiding this comment.
Yes, this is just lack of (immediate) effort on my part.
| @@ -256,26 +232,13 @@ The caller can pass the list of EXPLICIT packages to avoid re-computing it." | |||
| (defvar helm-system-packages-portage-use-source | |||
| (helm-build-in-buffer-source "USE flags" | |||
| :init 'helm-system-packages-portage-use-init | |||
| :candidate-transformer 'helm-system-packages-portage-use-transformer | |||
There was a problem hiding this comment.
I see you moved the transformer to the -use-init function. Why?
As I understand it, the USE flags won't be updated on change.
Let me know if I'm wrong.
There was a problem hiding this comment.
I'm unsure of how to approach this one - ideally, we want the USE-flag properties to be re-cached in helm-system-packages-portage-refresh, since then they'll be updated after running commands etc. However, that will inextricably link the package list source and the USE-flag source, preventing then from potentially being used independently by a third party. Thoughts?
There was a problem hiding this comment.
OK, so if I understand correctly, by moving the transformer to the init function you are updating the USE source everytime?
If this is fast enough, then I think this is the right approach.
helm-system-packages-portage.el
Outdated
| (define-key map (kbd "M-N") 'helm-system-packages-portage-toggle-uninstalled) | ||
| (define-key map (kbd "M-D") 'helm-system-packages-portage-toggle-dependencies) | ||
| (define-key map (kbd "C-]") 'helm-system-packages-toggle-descriptions) | ||
| map)) |
There was a problem hiding this comment.
Did you only move this block? Unless really necessary, let's keep the diff short! :)
helm-system-packages-portage.el
Outdated
| (set-keymap-parent map helm-map) | ||
| (define-key map (kbd "M-I") 'helm-system-packages-portage-toggle-explicit) | ||
| (define-key map (kbd "M-N") 'helm-system-packages-portage-toggle-uninstalled) | ||
| (define-key map (kbd "M-D") 'helm-system-packages-portage-toggle-dependencies) |
There was a problem hiding this comment.
Those 3 -toggles can be replaced with the package-level -toggles, e.g. helm-system-packages-toggle-explicit.
helm-system-packages-portage.el
Outdated
| (defun helm-system-packages-portage-cache (display-list) | ||
| "Cache all package names with descriptions." | ||
| (let* ((raw (with-temp-buffer | ||
| (process-file "env" nil '(t nil) nil "EIX_LIMIT=0" "OVERLAYS_LIST=none" "PRINT_COUNT_ALWAYS=never" "eix" "--format" "<category>/<name>\t<description>\n") |
There was a problem hiding this comment.
I think now we should break this absurdly long line ;)
| ;; tell `eix' to use the same formatting as `apt-cache' so that we can | ||
| ;; re-use its code. | ||
| ;; TODO: Can eix pad in the format string just like `expac' does? | ||
| ;; TODO: Or output straight to Elisp? |
There was a problem hiding this comment.
What do you think of outputting straight to Elisp? Could be dangerous, but could also be simpler and faster (if speed is a problem).
If you think this is still relevant, please keep the "TODO" note.
There was a problem hiding this comment.
Although the eix documentation that exists is fairly sparse, I couldn't find any reference to padding in the format string anywhere. It would certainly help significantly if this was possible. As far as outputting directly to Elisp, I assume you mean writing a series of S-expressions and then using read? I agree that this is fairly dangerous, and we'd have to perform a post-processing step anyway, since eix doesn't have any ability to escape strings AFAIK.
There was a problem hiding this comment.
OK. Feel free to comment on the TODO in the code.
helm-system-packages-portage.el
Outdated
| (buffer-string))) | ||
| (paired (mapcar (lambda (x) (let ((l (split-string x "\t"))) (cons (car l) (cadr l)))) (split-string raw "\n"))) | ||
| (names (mapcar 'car paired)) | ||
| (descriptions (mapcar (lambda (x) (concat (car x) (make-string (max (- helm-system-packages-column-width (length (car x))) 0) ? ) (cdr x))) paired))) |
There was a problem hiding this comment.
Please break all the long lines, thanks!
There was a problem hiding this comment.
names and descriptions are supposed to be string buffers for helm-system-packages--cache-set. Is this really working?
There was a problem hiding this comment.
helm-init-candidates-in-buffer accepts either a string or a list of candidates according to its documentation.
|
Looks good to me! Let me know when this is ready for a merge. |
|
Should be ready now. |
As discussed in #23 .
The changes I've made here are pretty minimal, mostly just switching to the new caching system.