emacs-eldev / eldev

Elisp development tool
https://emacs-eldev.github.io/eldev/
GNU General Public License v3.0
226 stars 17 forks source link

Fix source directory interaction with info command #98

Closed Zweihander-Main closed 6 months ago

Zweihander-Main commented 7 months ago

Ran into the following bug using the new source directories feature and eldev info:

  1. Create a project using source directories but without a README-elpa, README-elpa.md, README, README.rst or README.org file in the root of the project. See here for why those particular files.
  2. Run eldev -d info.

Result (Using Eldev's test/dependency-d directory as an example):

eldev --debug info
Debugger entered--Lisp error: (file-missing "Opening input file" "No such file or directory" "/home/zwei/dev/tmp/eldev/test/dependency-d/dependency-d.el")
  insert-file-contents("/home/zwei/dev/tmp/eldev/test/dependency-d/dependency-d.el" nil nil nil nil)
  lm-commentary("/home/zwei/dev/tmp/eldev/test/dependency-d/dependency-d.el")
  package--get-description(#s(package-desc :name dependency-d :version (1 0 99) :summary "Dependency test package D with a special source directory" :reqs nil :kind dir :archive nil :dir nil :extras nil :signed nil))
  (progn (package--get-description package))
  (if (fboundp 'package--get-description) (progn (package--get-description package)))
  (let* ((package (eldev-package-descriptor)) (description (if (fboundp 'package--get-description) (progn (package--get-description package))))) (if (> (length description) 0) nil (setq description (progn (or (progn (and (memq (type-of package) cl-struct-package-desc-tags) t)) (signal 'wrong-type-argument (list 'package-desc package))) (aref package 3)))) (eldev-output "%s %s" (eldev-colorize (progn (or (progn (and (memq (type-of package) cl-struct-package-desc-tags) t)) (signal 'wrong-type-argument (list 'package-desc package))) (aref package 1)) 'name) (eldev-message-version package)) (if description (progn (setq description (replace-regexp-in-string "\\`\\(?:[[:space:]]\\|\n\\)+\\|\\(?:[[:space:]]\\|\n\\)+\\'" "" description)) (if (string= description "") nil (eldev-output "\n%s" description)))))
  eldev-info()
  apply(eldev-info nil)
  (let ((eldev-message-rerouting-destination :stdout)) (apply handler command-line))
  (let ((hook (eldev-get handler :command-hook))) (if (and eldev-too-old (not (eldev-get handler :works-on-old-eldev))) (progn (signal 'eldev-too-old eldev-too-old))) (setq command-line (if (eldev-get handler :custom-parsing) (cdr command-line) (eldev-parse-options (cdr command-line) real-command))) (if (eq real-command command) (if (memq eldev-verbosity-level '(verbose trace)) (progn (eldev-output (eldev-colorize "Executing command `%s'..." 'verbose) command))) (if (memq eldev-verbosity-level '(verbose trace)) (progn (eldev-output (eldev-colorize "Executing command `%s' (alias for `%s')..." 'verbose) command real-command)))) (if eldev-executing-command-hook (progn (if (eq eldev-verbosity-level 'trace) (progn (eldev-output (eldev-colorize "Executing `eldev-executing-command-hook'..." 'trace)))) (run-hook-with-args 'eldev-executing-command-hook real-command))) (if (symbol-value hook) (progn (if (eq eldev-verbosity-level 'trace) (progn (eldev-output (eldev-colorize "Executing `%s'..." 'trace) hook))) (run-hooks hook))) (let ((eldev-message-rerouting-destination :stdout)) (apply handler command-line)))
  (if handler (let ((hook (eldev-get handler :command-hook))) (if (and eldev-too-old (not (eldev-get handler :works-on-old-eldev))) (progn (signal 'eldev-too-old eldev-too-old))) (setq command-line (if (eldev-get handler :custom-parsing) (cdr command-line) (eldev-parse-options (cdr command-line) real-command))) (if (eq real-command command) (if (memq eldev-verbosity-level '(verbose trace)) (progn (eldev-output (eldev-colorize "Executing command `%s'..." 'verbose) command))) (if (memq eldev-verbosity-level '(verbose trace)) (progn (eldev-output (eldev-colorize "Executing command `%s' (alias for `%s')..." 'verbose) command real-command)))) (if eldev-executing-command-hook (progn (if (eq eldev-verbosity-level 'trace) (progn (eldev-output (eldev-colorize "Executing `eldev-executing-command-hook'..." 'trace)))) (run-hook-with-args 'eldev-executing-command-hook real-command))) (if (symbol-value hook) (progn (if (eq eldev-verbosity-level 'trace) (progn (eldev-output (eldev-colorize "Executing `%s'..." 'trace) hook))) (run-hooks hook))) (let ((eldev-message-rerouting-destination :stdout)) (apply handler command-line))) (eldev--complain-about-missing-command command) (if (eq eldev-verbosity-level 'quiet) nil (eldev-output "Run `%s help' for a list of supported commands" (eldev-shell-command t))))
  (let* ((command (intern (car command-line))) (real-command (or (cdr (assq command eldev--command-aliases)) command)) (handler (cdr (assq real-command eldev--commands)))) (if handler (let ((hook (eldev-get handler :command-hook))) (if (and eldev-too-old (not (eldev-get handler :works-on-old-eldev))) (progn (signal 'eldev-too-old eldev-too-old))) (setq command-line (if (eldev-get handler :custom-parsing) (cdr command-line) (eldev-parse-options (cdr command-line) real-command))) (if (eq real-command command) (if (memq eldev-verbosity-level '(verbose trace)) (progn (eldev-output (eldev-colorize "Executing command `%s'..." 'verbose) command))) (if (memq eldev-verbosity-level '(verbose trace)) (progn (eldev-output (eldev-colorize "Executing command `%s' (alias for `%s')..." 'verbose) command real-command)))) (if eldev-executing-command-hook (progn (if (eq eldev-verbosity-level 'trace) (progn (eldev-output (eldev-colorize "Executing `eldev-executing-command-hook'..." 'trace)))) (run-hook-with-args 'eldev-executing-command-hook real-command))) (if (symbol-value hook) (progn (if (eq eldev-verbosity-level 'trace) (progn (eldev-output (eldev-colorize "Executing `%s'..." 'trace) hook))) (run-hooks hook))) (let ((eldev-message-rerouting-destination :stdout)) (apply handler command-line))) (eldev--complain-about-missing-command command) (if (eq eldev-verbosity-level 'quiet) nil (eldev-output "Run `%s help' for a list of supported commands" (eldev-shell-command t)))))
  eldev--execute-command(("info"))

This PR fixes that but two issues I want to flag --

  1. package--get-description first checks for README files and then if they aren't found, uses lm-commentary. Rather than attempt to advise/rewrite package--get-description with the source directory, this catches it if it errors with the assumption that the README wasn't found and the error was in the lm-commentary half of it. It then runs lm-commentary on the correct file (and doesn't bother trying to find a README in a source directory). The possible problem here is it won't catch any regressions package--get-description may turn up in the future -- it always assumes an error there is going to be this one source directory case.
  2. The second source directory test doesn't actually test the intended effect very well. The eldev-info-missing-dependency-1 test suggests that different versions of Emacs may turn up the commentary string or the package title. The second source directory test is trying to confirm that if the commentary is present in the main code file, it'll come up but since it has to test for both, it doesn't actually make any difference if you just catch the package--get-description error and then use the source directory to find the commentary string or if you just use (error nil) after catching it and letting the next block of code handle it.

If this is the wrong approach here, happy to try again or otherwise feel free to discard; mostly wanted to flag the problem!

doublep commented 7 months ago

Also, have a look at the CI failures. In general, this PR looks good.

Zweihander-Main commented 7 months ago

I've added (eval-and-compile (require 'lisp-mnt)) for the CI failure related to byte compilation and updated/rebased the branch to avoid copyright year errors from the eldev doctor part of the pipeline -- think that should pass now.

doublep commented 6 months ago

Thank you for the fix!