From 5293c460dbc77b190dcb65e8b2edc31d5b688206 Mon Sep 17 00:00:00 2001 From: Yuri Pieters Date: Sun, 31 Jan 2021 16:51:12 +0000 Subject: [PATCH 1/3] Refactor doom--help-insert-button Changes: - Fixes a bug where opening a file which a buffer was already visiting didn't raise the buffer. - The function had unused functionality where it would split a string on '::' and then search for the text after the first '::' in the buffer; this has been removed. - The searching functionality has been replaced with the option to pass a line number, which the opened buffer will jump to. This is now used by the part of doom/help-packages that shows the places a package is configured. - It now fails earlier. If there's an invalid file, it fails at call time rather than when the button is pressed. - Add a docstring --- core/autoload/help.el | 64 ++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/core/autoload/help.el b/core/autoload/help.el index 909898d60..3ead167b1 100644 --- a/core/autoload/help.el +++ b/core/autoload/help.el @@ -404,33 +404,31 @@ variables, this only lists variables that exist to be customized (defined with ;; ;;; `doom/help-packages' -(defun doom--help-insert-button (label &optional path) - (cl-destructuring-bind (uri . qs) - (let ((parts (split-string label "::" t))) - (cons (string-trim (car parts)) - (string-join (cdr parts) "::"))) - (let ((path (or path label))) - (insert-text-button - uri - 'face 'link - 'follow-link t - 'action +(defun doom--help-insert-button (label &optional uri line) + "Helper function to insert a button at point. + +The button will have the text LABEL. If URI is given, the button will open it, +otherwise the LABEL will be used. If the uri to open is a url it will be opened +in a browser. If LINE is given (and the uri to open is not a url), then the file +will open with point on that line." + (let ((uri (or uri label))) + (insert-text-button + label + 'face 'link + 'follow-link t + 'action + (if (string-match-p "^https?://" uri) + (lambda (_) (browse-url uri)) + (unless (file-exists-p uri) + (user-error "Path does not exist: %S" uri)) (lambda (_) (when (window-dedicated-p) (other-window 1)) - (pcase (cond ((string-match-p "^https?://" qs) 'url) - ('file)) - ((or `file `nil) - (unless (file-exists-p path) - (user-error "Path does not exist: %S" path)) - (let ((buffer (or (get-file-buffer path) - (find-file path)))) - (when qs - (with-current-buffer buffer - (goto-char (point-min)) - (re-search-forward qs) - (recenter))))) - (`url (browse-url uri)))))))) + (find-file uri) + (when line + (goto-char (point-min)) + (forward-line (1- line)) + (recenter))))))) (defun doom--help-package-configs (package) (let ((default-directory doom-emacs-dir)) @@ -584,19 +582,11 @@ If prefix arg is present, refresh the cache." (insert "This package is configured in the following locations:") (dolist (location (doom--help-package-configs package)) (insert "\n" indent) - (insert-text-button - location - 'face 'link - 'follow-link t - 'action - `(lambda (_) - (cl-destructuring-bind (file line _match) - ',(split-string location ":") - (find-file (expand-file-name file doom-emacs-dir)) - (goto-char (point-min)) - (forward-line (1- (string-to-number line))) - (recenter))))) - + (cl-destructuring-bind (file line _match) + (split-string location ":") + (doom--help-insert-button location + (expand-file-name file doom-emacs-dir) + (string-to-number line)))) (insert "\n\n"))))) (defvar doom--package-cache nil) From b983929e826db6f2b20082faf2da2735e35ea72c Mon Sep 17 00:00:00 2001 From: Yuri Pieters Date: Mon, 1 Feb 2021 02:07:27 +0000 Subject: [PATCH 2/3] Refactor doom/help-packages --- core/autoload/help.el | 105 ++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 45 deletions(-) diff --git a/core/autoload/help.el b/core/autoload/help.el index 3ead167b1..09323957c 100644 --- a/core/autoload/help.el +++ b/core/autoload/help.el @@ -420,7 +420,7 @@ will open with point on that line." (if (string-match-p "^https?://" uri) (lambda (_) (browse-url uri)) (unless (file-exists-p uri) - (user-error "Path does not exist: %S" uri)) + (error "Path does not exist: %S" uri)) (lambda (_) (when (window-dedicated-p) (other-window 1)) @@ -461,44 +461,41 @@ If prefix arg is present, refresh the cache." (if (and doom--help-packages-list (null current-prefix-arg)) doom--help-packages-list (message "Generating packages list for the first time...") - (sit-for 0.1) + (redisplay) (setq doom--help-packages-list (delete-dups (append (mapcar #'car package-alist) (mapcar #'car package--builtins) - (mapcar #'intern (hash-table-keys straight--build-cache)) + (mapcar #'intern + (hash-table-keys straight--build-cache)) (mapcar #'car (doom-package-list 'all)) nil)))))) (unless (memq guess packages) (setq guess nil)) (list (intern - (completing-read (if guess - (format "Select Doom package to search for (default %s): " - guess) - (format "Describe Doom package (%d): " (length packages))) + (completing-read (format "Describe Doom package (%s): " + (concat (when guess + (format "default '%s', " guess)) + (format "total %d" (length packages)))) packages nil t nil nil - (if guess (symbol-name guess)))))))) + (when guess (symbol-name guess)))))))) ;; TODO Refactor me. (require 'core-packages) (doom-initialize-packages) - (if (or (package-desc-p package) - (and (symbolp package) - (or (assq package package-alist) - (assq package package--builtins)))) - (describe-package package) - (help-setup-xref (list #'doom/help-packages package) - (called-interactively-p 'interactive)) - (with-help-window (help-buffer))) - (save-excursion - (with-current-buffer (help-buffer) - (let ((inhibit-read-only t) - (indent (make-string 13 ? ))) - (goto-char (point-max)) - (if (re-search-forward "^ *Status: " nil t) - (progn - (end-of-line) - (insert "\n")) + (help-setup-xref (list #'doom/help-packages package) + (called-interactively-p 'interactive)) + (with-help-window (help-buffer) + (with-current-buffer standard-output + (when (or (package-desc-p package) + (and (symbolp package) + (or (assq package package-alist) + (assq package package--builtins)))) + (describe-package-1 package)) + (let ((indent (make-string 13 ? ))) + (goto-char (point-min)) + (if (re-search-forward " Status: .*$" nil t) + (insert "\n") (search-forward "\n\n" nil t)) (package--print-help-section "Package") @@ -513,38 +510,53 @@ If prefix arg is present, refresh the cache." pin "unpinned") "\n") + (package--print-help-section "Build") (let ((default-directory (straight--repos-dir (symbol-name package)))) - (insert (cdr (doom-call-process "git" "log" "-1" "--format=%D %h %ci")) - "\n" indent)) + (if + (file-exists-p default-directory) + (insert + (cdr + (doom-call-process "git" "log" "-1" "--format=%D %h %ci"))) + (insert "n/a"))) + (insert "\n" indent) + (package--print-help-section "Build location") (let ((build-dir (straight--build-dir (symbol-name package)))) (if (file-exists-p build-dir) (doom--help-insert-button (abbreviate-file-name build-dir)) (insert "n/a"))) (insert "\n" indent) + (package--print-help-section "Repo location") (let ((repo-dir (straight--repos-dir (symbol-name package)))) (if (file-exists-p repo-dir) (doom--help-insert-button (abbreviate-file-name repo-dir)) (insert "n/a")) (insert "\n")) + (let ((recipe (doom-package-build-recipe package))) (package--print-help-section "Recipe") - (insert (format "%s\n" (string-trim (pp-to-string recipe)))) - (package--print-help-section "Homepage") - (doom--help-insert-button (doom--package-url package)))) + (insert + (replace-regexp-in-string "\n" (concat "\n" indent) + (pp-to-string recipe)))) + + (package--print-help-section "Homepage") + (doom--help-insert-button (doom--package-url package))) + (`elpa (insert "[M]ELPA ") (doom--help-insert-button (doom--package-url package)) (package--print-help-section "Location") (doom--help-insert-button (abbreviate-file-name - (file-name-directory (locate-library (symbol-name package)))))) + (file-name-directory + (locate-library (symbol-name package)))))) (`builtin (insert "Built-in\n") (package--print-help-section "Location") (doom--help-insert-button (abbreviate-file-name - (file-name-directory (locate-library (symbol-name package)))))) + (file-name-directory + (locate-library (symbol-name package)))))) (`other (doom--help-insert-button (abbreviate-file-name (or (symbol-file package) @@ -564,7 +576,9 @@ If prefix arg is present, refresh the cache." (let* ((module-path (pcase (car m) (:core doom-core-dir) (:private doom-private-dir) - (category (doom-module-path category (cdr m))))) + (category + (doom-module-locate-path category + (cdr m))))) (readme-path (expand-file-name "README.org" module-path))) (insert indent) (doom--help-insert-button @@ -572,22 +586,23 @@ If prefix arg is present, refresh the cache." module-path) (insert " (") (if (file-exists-p readme-path) - (doom--help-insert-button - "readme" - readme-path) + (doom--help-insert-button "readme" readme-path) (insert "no readme")) (insert ")\n")))) (package--print-help-section "Configs") - (insert "This package is configured in the following locations:") - (dolist (location (doom--help-package-configs package)) - (insert "\n" indent) - (cl-destructuring-bind (file line _match) - (split-string location ":") - (doom--help-insert-button location - (expand-file-name file doom-emacs-dir) - (string-to-number line)))) - (insert "\n\n"))))) + (if-let ((configs (doom--help-package-configs package))) + (progn + (insert "This package is configured in the following locations:") + (dolist (location configs) + (insert "\n" indent) + (cl-destructuring-bind (file line _match) + (split-string location ":") + (doom--help-insert-button location + (expand-file-name file doom-emacs-dir) + (string-to-number line))))) + (insert "This package is not configured anywhere")) + (goto-char (point-min)))))) (defvar doom--package-cache nil) (defun doom--package-list (&optional prompt) From bf185edd918628e4ac8a33732258fb240ef208a4 Mon Sep 17 00:00:00 2001 From: Yuri Pieters Date: Mon, 1 Feb 2021 19:01:42 +0000 Subject: [PATCH 3/3] Prefer readability over a line length < 80 --- core/autoload/help.el | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/autoload/help.el b/core/autoload/help.el index 09323957c..c0efa60b9 100644 --- a/core/autoload/help.el +++ b/core/autoload/help.el @@ -513,11 +513,8 @@ If prefix arg is present, refresh the cache." (package--print-help-section "Build") (let ((default-directory (straight--repos-dir (symbol-name package)))) - (if - (file-exists-p default-directory) - (insert - (cdr - (doom-call-process "git" "log" "-1" "--format=%D %h %ci"))) + (if (file-exists-p default-directory) + (insert (cdr (doom-call-process "git" "log" "-1" "--format=%D %h %ci"))) (insert "n/a"))) (insert "\n" indent)