unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [ELPA] New package: hcel
@ 2022-09-10 15:01 Yuchen Pei
  2022-09-17  0:40 ` Yuchen Pei
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Yuchen Pei @ 2022-09-10 15:01 UTC (permalink / raw)
  To: Emacs Devel mailing list

Hello,

I'd like to submit my little package called hcel[1] to ELPA.

It is a Haskell codebase explorer, using the server program provided by
haskell-code-explorer[2], which I ported to recent GHC version[3] and
I named my port / fork hcel.

[1] https://g.ypei.me/hcel.git/tree/lisp
[2] https://github.com/alexwl/haskell-code-explorer
[3] https://g.ypei.me/hcel.git/about/

I'm not sure whether it is unusual to submit a package that relies on a
self-hosted free software server program, but I am also not aware of
packages doing anything similar.  There are packages offering related
functionalities, like eglot, which can work with
haskell-language-server.  However, haskell-language-server requires more
ram and cpu, and does not support cross-package find-definition and
references.

Basically, this is an emacs version of the web client that is included
in the original haskell-code-explorer.  In general, I think emacs as an
alternative client to the web browser is a way to avoid nonfree
javascript, similar to alternative web frontends (e.g. invidious,
teddit, haketilo), but better (because emacs).  Ideally there should be
an "emacs app store" offering (GPL covered) clients to all sorts of web
services, which could be a subset of ELPA packages, to advance the user
freedom of at least emacs users.  But I digress.

The server and client programs are currently in the same repo, and I
hold the copyright of the emacs package, but not the server.  I can
assign the copyright of the emacs package and separate it out into its
own git repo of course, and I can also ask the original authors for
copyright assignment if needed.

I have been using this package for a week or two, and expect to use it
as aid for writing Haskell programs.  So as long as I write Haskell and
the package has not been superceded by other tools like
haskell-language-server, I am motivated to make sure it works well.

What do you think?

Best,
Yuchen

-- 
PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
          <https://ypei.org/assets/ypei-pubkey.txt>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-10 15:01 [ELPA] New package: hcel Yuchen Pei
@ 2022-09-17  0:40 ` Yuchen Pei
  2022-09-19  0:15   ` How Do I submit a package to ELPA? (was Re: [ELPA] New package: hcel) Yuchen Pei
  2022-09-20 21:26 ` [ELPA] New package: hcel Stefan Monnier
  2022-09-20 21:42 ` Stefan Monnier
  2 siblings, 1 reply; 11+ messages in thread
From: Yuchen Pei @ 2022-09-17  0:40 UTC (permalink / raw)
  To: Emacs Devel mailing list

Given there's no comments and no approval process[1], my next step will
be to prepare the package further for elpa publication, including
separating the emacs package out from the repo due to copyright
assignment, and ask here for someone with commit access to push my
package to an hcel branch on elpa.git.  I will try to do that over the
weekend.

[1] https://git.savannah.gnu.org/cgit/emacs/elpa.git/plain/README

Let me know if there's anything else I need to do.

Best,
Yuchen

-- 
PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
          <https://ypei.org/assets/ypei-pubkey.txt>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* How Do I submit a package to ELPA? (was Re: [ELPA] New package: hcel)
  2022-09-17  0:40 ` Yuchen Pei
@ 2022-09-19  0:15   ` Yuchen Pei
  0 siblings, 0 replies; 11+ messages in thread
From: Yuchen Pei @ 2022-09-19  0:15 UTC (permalink / raw)
  To: Emacs Devel mailing list

On Sat 2022-09-17 10:40:51 +1000, Yuchen Pei wrote:

> Given there's no comments and no approval process[1], my next step will
> be to prepare the package further for elpa publication, including
> separating the emacs package out from the repo due to copyright
> assignment, and ask here for someone with commit access to push my
> package to an hcel branch on elpa.git.  I will try to do that over the
> weekend.
>
> [1] https://git.savannah.gnu.org/cgit/emacs/elpa.git/plain/README

I just noticed that the instructions on adding a package to ELPA is
below the following text:

* Text below this marker is OUTDATED and still needs to be reviewed/rewritten!!

So I'm not sure what to do now.  What is the diff between the current
process and the instructions in the README?

Best,
Yuchen

-- 
PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
          <https://ypei.org/assets/ypei-pubkey.txt>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-10 15:01 [ELPA] New package: hcel Yuchen Pei
  2022-09-17  0:40 ` Yuchen Pei
@ 2022-09-20 21:26 ` Stefan Monnier
  2022-09-21  6:18   ` Yuchen Pei
  2022-09-20 21:42 ` Stefan Monnier
  2 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2022-09-20 21:26 UTC (permalink / raw)
  To: Yuchen Pei; +Cc: Emacs Devel mailing list

Hi Yuchen, sorry I didn't get back to you earlier,

> It is a Haskell codebase explorer, using the server program provided by
> haskell-code-explorer[2], which I ported to recent GHC version[3] and
> I named my port / fork hcel.
>
> [1] https://g.ypei.me/hcel.git/tree/lisp
> [2] https://github.com/alexwl/haskell-code-explorer
> [3] https://g.ypei.me/hcel.git/about/
>
> I'm not sure whether it is unusual to submit a package that relies on a
> self-hosted free software server program,

Indeed, in many cases such modes start their life alongside the
associated tool, but it can be nicer for users if it's available in
GNU ELPA.

Often part of the issue is co-evolution (e.g. does it make sense to use
`hcel.el` from last year with the current `hcel` tool and vice versa).

IIUC the code you suggest we add to GNU ELPA is the one at
https://g.ypei.me/hc.el.git, right?

Given the fact that this tool has a fairly narrow focus, I must admit
that I'd prefer it doesn't eat up a two-letter file name like `hc`.
Any chance you can rename that main file to `hcel.el`?

Also, I see that your package has:

    ;; Package-Requires: ((emacs "28") (haskell-mode))

but `haskell-mode` is currently neither in GNU ELPA nor in NonGNU ELPA
so we can't include it in neither of them.  Is the dependency
unavoidable (so we should first include `haskell-mode` in (Non)GNU
ELPA), or can it be removed (and replaced with a runtime test, possibly
allowing the use of other haskell modes such as haskell-tng-mode, which
*is* in NonGNU ELPA)?


        Stefan




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-10 15:01 [ELPA] New package: hcel Yuchen Pei
  2022-09-17  0:40 ` Yuchen Pei
  2022-09-20 21:26 ` [ELPA] New package: hcel Stefan Monnier
@ 2022-09-20 21:42 ` Stefan Monnier
  2022-09-21  5:58   ` Yuchen Pei
  2 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2022-09-20 21:42 UTC (permalink / raw)
  To: Yuchen Pei; +Cc: Emacs Devel mailing list

[-- Attachment #1: Type: text/plain, Size: 221 bytes --]

BTW, here's a patch with various cosmetic changes.
I suggest you read it at least as much as apply it (it contains a few
FIXMEs as well).  I also attach the many warnings I got when trying to
compile it.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: hcel.patch --]
[-- Type: text/x-diff, Size: 20798 bytes --]

diff --git a/.gitignore b/.gitignore
index e4e5f6c8b2..bd5709fcb0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,4 @@
-*~
\ No newline at end of file
+*~
+*.elc
+/hcel-autoloads.el
+/hcel-pkg.el
diff --git a/hcel-client.el b/hcel-client.el
index ab64ea3293..5ec605574b 100644
--- a/hcel-client.el
+++ b/hcel-client.el
@@ -138,7 +138,7 @@
     (goto-char (point-max))
     (insert "[" (current-time-string) "] Request: " url "\n"))
   (with-current-buffer (url-retrieve-synchronously url t)
-    (let ((header) (status) (fields) (json))
+    (let ((header) (status) (fields))
       (delete-http-header)
       (setq header (hcel-parse-http-header (car kill-ring))
             status (alist-get 'status header)
diff --git a/hcel-haddorg.el b/hcel-haddorg.el
index ad797e0b10..be97e423a2 100644
--- a/hcel-haddorg.el
+++ b/hcel-haddorg.el
@@ -29,7 +29,7 @@
            (module-name (car splitted))
            (entity (if (equal "v" (cadr splitted)) "Val" "Typ"))
            (name (caddr splitted))
-           (package) (unparsed) (package-id))
+           (package) (package-id))
       (goto-char (point-min))
       (setq package (org-entry-get (point) "ITEM"))
       (setq package-id
diff --git a/hcel-outline.el b/hcel-outline.el
index a46db147f6..78aad54156 100644
--- a/hcel-outline.el
+++ b/hcel-outline.el
@@ -17,31 +17,33 @@
 ;; You should have received a copy of the GNU Affero General Public
 ;; License along with hcel.  If not, see <https://www.gnu.org/licenses/>.
 
+(require 'hcel-source)
+
 (defvar hcel-outline-buffer-name "*hcel-outline*")
 (defvar hcel-outline-indentation 2)
 
 (defvar hcel-outline-mode-map
   (let ((kmap (make-sparse-keymap)))
-    (define-key kmap "n" 'outline-next-visible-heading)
-    (define-key kmap "p" 'outline-previous-visible-heading)
-    (define-key kmap "f" 'outline-forward-same-level)
-    (define-key kmap "F" 'hcel-outline-follow-mode)
-    (define-key kmap "b" 'outline-backward-same-level)
-    (define-key kmap "u" 'outline-up-heading)
-    (define-key kmap "\t" 'hcel-outline-toggle-children)
-    (define-key kmap "\r" 'hcel-outline-open-thing-at-point)
-    (define-key kmap "o" 'hcel-outline-open-thing-at-point-other-window)
-    (define-key kmap "q" 'quit-window)
+    (define-key kmap "n"  #'outline-next-visible-heading)
+    (define-key kmap "p"  #'outline-previous-visible-heading)
+    (define-key kmap "f"  #'outline-forward-same-level)
+    (define-key kmap "F"  #'hcel-outline-follow-mode)
+    (define-key kmap "b"  #'outline-backward-same-level)
+    (define-key kmap "u"  #'outline-up-heading)
+    (define-key kmap "\t" #'hcel-outline-toggle-children)
+    (define-key kmap "\r" #'hcel-outline-open-thing-at-point)
+    (define-key kmap "o"  #'hcel-outline-open-thing-at-point-other-window)
+    (define-key kmap "q"  #'quit-window)
     kmap))
 (define-derived-mode hcel-outline-mode outline-mode "hcel-outline"
-  "Major mode for browsing Haskell codebases"
+  "Major mode for browsing Haskell codebases."
   (setq-local package-filter nil
               module-filter nil
               outline-regexp "\\( *\\)."
               outline-level (lambda () (1+ (/ (length (match-string 1))
                                               hcel-outline-indentation)))
-              buffer-read-only t))
-(add-hook 'hcel-outline-mode-hook 'hcel-minor-mode)
+              buffer-read-only t)
+  (hcel-minor-mode 1))
 
 (defun hcel ()
   (interactive)
@@ -61,7 +63,7 @@
          (hcel-api-packages)))
       (hcel-outline-mode))))
 
-(define-key hcel-mode-map "o" 'hcel)
+(define-key hcel-mode-map "o" #'hcel)
 
 ;; TODO: maybe remove
 (defun hcel-outline-update-opened (package-id module-path)
@@ -208,8 +210,8 @@ update in the outline mode too."
       (if (not (eq major-mode 'hcel-outline-mode))
           (error "Not in hcel-outline mode!")
         (add-hook 'post-command-hook
-                  'hcel-outline-open-thing-at-point-other-window nil t))
+                  #'hcel-outline-open-thing-at-point-other-window nil t))
     (remove-hook 'post-command-hook
-                 'hcel-outline-open-thing-at-point-other-window t)))
+                 #'hcel-outline-open-thing-at-point-other-window t)))
 
 (provide 'hcel-outline)
diff --git a/hcel-results.el b/hcel-results.el
index a0ce11b049..ff19d26799 100644
--- a/hcel-results.el
+++ b/hcel-results.el
@@ -24,6 +24,7 @@
 
 ;;; Code:
 (require 'hcel-utils)
+(eval-when-compile (require 'compile))
 
 (defun hcel-results-next-error-no-open (n)
   (interactive "p")
@@ -40,7 +41,7 @@
         (goto-char (point-min))
         (hcel-results-next-error-internal 1 nil))
     (if (> n 0)
-        (dotimes (unused n)
+        (dotimes (_unused n)
           (condition-case nil
               (progn
                 (goto-char (next-single-property-change (point) 'match-line))
@@ -48,7 +49,7 @@
                   (goto-char
                    (next-single-property-change (point) 'match-line))))
             (error (hcel-results-next-page))))
-      (dotimes (unused (- n))
+      (dotimes (_unused (- n))
         (condition-case nil
             (progn
               (goto-char (previous-single-property-change (point) 'match-line))
@@ -68,12 +69,15 @@
   (compilation-set-overlay-arrow (selected-window))
   (hcel-load-module-location-info (get-text-property (point) 'location-info)))
 
+(defvar-local hcel-results-page-number nil)
+
 (defun hcel-results-next-page ()
   (interactive)
+  ;; FIXME: Using `major-mode' is a code smell.
   (unless (memq major-mode '(hcel-refs-mode hcel-ids-mode))
     (error "Not in hcel-refs or hcel-ids mode: %S" major-mode))
   (when (= hcel-results-page-number hcel-results-max-page-number)
-    (error "Already on the last page."))
+    (error "Already on the last page"))
   (setq hcel-results-page-number (1+ hcel-results-page-number))
   (cond ((eq major-mode 'hcel-refs-mode) (hcel-refs-update-references))
         ((eq major-mode 'hcel-ids-mode) (hcel-ids-update))
@@ -100,17 +104,17 @@
 
 (define-compilation-mode hcel-refs-mode "hcel-refs"
   "Major mode for showing references"
-  (setq-local next-error-function 'hcel-results-next-error
+  (setq-local next-error-function #'hcel-results-next-error
               hcel-refs-id nil
               hcel-refs-package-id nil
               hcel-results-page-number nil
               hcel-results-max-page-number nil))
 
 (define-key hcel-refs-mode-map (kbd "M-n")
-  'hcel-results-next-error-no-open)
+  #'hcel-results-next-error-no-open)
 
 (define-key hcel-refs-mode-map (kbd "M-p")
-  'hcel-results-previous-error-no-open)
+  #'hcel-results-previous-error-no-open)
 
 (defun hcel-refs-update-references ()
   "Find references and update the current hcel-refs-mode buffer."
@@ -159,11 +163,11 @@
 (defun hcel-refs-reload ()
   (interactive)
   (hcel-refs-update-references))
-(define-key hcel-refs-mode-map "g" 'hcel-refs-reload)
+(define-key hcel-refs-mode-map "g" #'hcel-refs-reload)
 
-(define-key hcel-refs-mode-map "f" 'hcel-results-next-page)
+(define-key hcel-refs-mode-map "f" #'hcel-results-next-page)
 
-(define-key hcel-refs-mode-map "b" 'hcel-results-previous-page)
+(define-key hcel-refs-mode-map "b" #'hcel-results-previous-page)
 
 (defun hcel-refs-buffer-name (id)
   (format "*hcel-refs %s*" (hcel-refs-format-id id)))
@@ -199,14 +203,14 @@ Start by choosing a package."
           hcel-results-page-number 1
           hcel-results-max-page-number max-page-number)
     (hcel-refs-update-references)))
-(define-key hcel-refs-mode-map "P" 'hcel-refs-update-references-package)
+(define-key hcel-refs-mode-map "P" #'hcel-refs-update-references-package)
 
 (defun hcel-find-references-at-point ()
   "Find references of the identifier at point."
   (interactive)
   (hcel-find-references-internal hcel-package-id hcel-module-path
                                  (hcel-text-property-near-point 'identifier)))
-(define-key hcel-mode-map (kbd "M-?") 'hcel-find-references-at-point)
+(define-key hcel-mode-map (kbd "M-?") #'hcel-find-references-at-point)
 
 (defun hcel-minor-find-references-at-point ()
   (interactive)
@@ -249,13 +253,13 @@ Start by choosing a package."
   :group 'hcel-ids)
 (define-compilation-mode hcel-ids-mode "hcel-ids"
   "Major mode for showing identifiers"
-  (setq-local next-error-function 'hcel-results-next-error
+  (setq-local next-error-function #'hcel-results-next-error
               hcel-ids-scope nil
               hcel-ids-query nil
               hcel-ids-package-id nil
               hcel-results-page-number nil
               hcel-results-max-page-number nil))
-(add-hook 'hcel-ids-mode-hook 'hcel-minor-mode)
+(add-hook 'hcel-ids-mode-hook #'hcel-minor-mode)
 
 (defun hcel-ids-update ()
   (unless (eq major-mode 'hcel-ids-mode)
@@ -305,14 +309,14 @@ Start by choosing a package."
 (defun hcel-ids-reload ()
   (interactive)
   (hcel-ids-update))
-(define-key hcel-ids-mode-map "g" 'hcel-ids-reload)
+(define-key hcel-ids-mode-map "g" #'hcel-ids-reload)
 
 (define-key hcel-ids-mode-map (kbd "M-n")
-  'hcel-results-next-error-no-open)
+  #'hcel-results-next-error-no-open)
 (define-key hcel-ids-mode-map (kbd "M-p")
-  'hcel-results-previous-error-no-open)
-(define-key hcel-ids-mode-map "f" 'hcel-results-next-page)
-(define-key hcel-ids-mode-map "b" 'hcel-results-previous-page)
+  #'hcel-results-previous-error-no-open)
+(define-key hcel-ids-mode-map "f" #'hcel-results-next-page)
+(define-key hcel-ids-mode-map "b" #'hcel-results-previous-page)
 
 (defun hcel-ids-update-query (query)
   "Search for identities matching query."
@@ -323,7 +327,7 @@ Start by choosing a package."
   (setq hcel-ids-query query
         hcel-results-page-number 1)
   (hcel-ids-update))
-(define-key hcel-ids-mode-map "s" 'hcel-ids-update-query)
+(define-key hcel-ids-mode-map "s" #'hcel-ids-update-query)
 
 (defun hcel-ids-buffer-name (scope query)
   (format "*hcel-ids-%S %s*" scope query))
@@ -375,9 +379,9 @@ Start by choosing a package."
   (interactive (list
                 (let ((minibuffer-allow-text-properties t))
                   (completing-read "Search for identifier globally: "
-                                   'hcel-global-ids-minibuffer-collection))))
+                                   #'hcel-global-ids-minibuffer-collection))))
   (hcel-ids 'global query))
-(define-key hcel-mode-map "I" 'hcel-global-ids)
+(define-key hcel-mode-map "I" #'hcel-global-ids)
 
 (defun hcel-package-ids (query)
   (interactive (list
@@ -390,6 +394,6 @@ Start by choosing a package."
                            (hcel-format-package-id package-id "-"))
                    (hcel-package-ids-minibuffer-collection package-id)))))
   (hcel-ids 'package query hcel-package-id))
-(define-key hcel-mode-map "i" 'hcel-package-ids)
+(define-key hcel-mode-map "i" #'hcel-package-ids)
 
 (provide 'hcel-results)
diff --git a/hcel-source.el b/hcel-source.el
index 7a793c6ade..2d51672b00 100644
--- a/hcel-source.el
+++ b/hcel-source.el
@@ -20,17 +20,20 @@
 (require 'hcel-client)
 (define-derived-mode hcel-mode special-mode "hcel"
   "Major mode for exploring Haskell codebases"
-  (setq-local eldoc-documentation-strategy 'eldoc-documentation-compose-eagerly
+  (setq-local eldoc-documentation-strategy #'eldoc-documentation-compose-eagerly
               eldoc-documentation-functions
               '(hcel-eldoc-id-type hcel-eldoc-expression-type hcel-eldoc-docs)
-              imenu-create-index-function 'hcel-imenu-create-index
+              imenu-create-index-function #'hcel-imenu-create-index
               imenu-space-replacement " "
               hcel-identifiers nil
               hcel-declarations nil
               hcel-occurrences nil
               hcel-package-id nil
               hcel-module-path nil
-              hcel-highlight-id nil))
+              hcel-highlight-id nil)
+  (cursor-sensor-mode 1)
+  (add-hook 'xref-backend-functions #'hcel--xref-backend nil t))
+
 (defun hcel-buffer-name (package-id module-path)
   (concat "*hcel " (hcel-format-package-id package-id "-")
           "/" module-path "*"))
@@ -65,7 +68,7 @@ When FORCE is non-nil, kill existing source buffer if any."
       (switch-to-buffer
        (hcel-load-module-source hcel-package-id hcel-module-path t))
     (error "Not in hcel-mode!")))
-(define-key hcel-mode-map "g" 'hcel-reload-module-source)
+(define-key hcel-mode-map "g" #'hcel-reload-module-source)
 
 (defun hcel-load-module-location-info (location-info &optional no-jump)
   "Load a module from exact location info.
@@ -118,7 +121,7 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
         'major-mode
         (get-buffer (if (stringp buffer) buffer (car buffer))))
 		   'hcel-mode)))))
-(define-key hcel-mode-map "b" 'hcel-switch-buffer)
+(define-key hcel-mode-map "b" #'hcel-switch-buffer)
 
 (defun hcel-lookup-occurrence-at-point ()
   (when-let ((occurrence (get-text-property (point) 'occurrence)))
@@ -224,7 +227,7 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
     (hcel-outline-load-modules-at-point)
     (hcel-outline-goto-module module-path)
     (hcel-outline-load-identifiers-at-point)))
-(define-key hcel-mode-map "O" 'hcel-outline-package-module)
+(define-key hcel-mode-map "O" #'hcel-outline-package-module)
 
 ;; eldoc
 (defun hcel-eldoc-id-type (cb)
@@ -312,7 +315,7 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
 (defface hcel-highlight-id '((t (:inherit underline)))
   "Face for highlighting hcel identifier at point.")
 
-(defun hcel-highlight-update (unused unused unused)
+(defun hcel-highlight-update (&rest _)
   ;; if mark is active, change of face will deactivate the mark in transient
   ;; mark mode
   (unless mark-active
@@ -345,8 +348,6 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
            (prop-match-beginning match)
            (prop-match-end match) 'hcel-highlight-id))))))
 
-(add-hook 'hcel-mode-hook 'cursor-sensor-mode)
-
 ;; utilities
 (defun hcel-write-source-line-to-buffer (line)
   (mapc
@@ -360,7 +361,7 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
                     'identifier (unless (string= id "") id)
                     'occurrence (unless (string= occ "") occ)
                     'cursor-sensor-functions
-                    (when id (list 'hcel-highlight-update))))))
+                    (when id (list #'hcel-highlight-update))))))
    line))
 
 (defun hcel-write-source-to-buffer (lines)
@@ -381,13 +382,13 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
                     'identifier (unless (string= id "") id)
                     'occurrence (unless (string= occ "") occ)
                     'cursor-sensor-functions
-                    (when id (list 'hcel-highlight-update))))))
+                    (when id (list #'hcel-highlight-update))))))
    (dom-by-tag line 'span))
   (insert "\n"))
 
 (defun hcel-write-html-source-to-buffer (lines)
   (mapc
-   'hcel-write-html-source-line-to-buffer
+   #'hcel-write-html-source-line-to-buffer
    lines))
 
 (defun hcel-source-html (json)
@@ -410,13 +411,9 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
        (alist-get 'name decl))
       (progn (goto-line (alist-get 'lineNumber decl)) (point))))
    hcel-declarations))
-(define-key hcel-mode-map "j" 'imenu)
+(define-key hcel-mode-map "j" #'imenu)
 
 ;; xref
-(add-hook 'hcel-mode-hook
-          (lambda ()
-            (add-hook 'xref-backend-functions
-                      #'hcel--xref-backend nil t)))
 (defun hcel--xref-backend () 'hcel-xref)
 (cl-defmethod xref-backend-definitions ((_backend (eql hcel-xref)) _identifiers)
   (hcel-find-definition))
@@ -427,10 +424,6 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
    (hcel-text-property-near-point 'identifier)
    (hcel-text-property-near-point 'occurrence)))
 
-(add-hook 'hcel-minor-mode-hook
-          (lambda ()
-            (add-hook 'xref-backend-functions
-                      #'hcel-minor--xref-backend nil t)))
 (defun hcel-minor--xref-backend () 'hcel-minor-xref)
 (cl-defmethod xref-backend-definitions ((_backend (eql hcel-minor-xref)) _identifiers)
   (hcel-minor-find-definition-at-point))
@@ -466,8 +459,7 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
                   (alist-get (intern occurrence) hcel-occurrences)))))
           (when (string= (hcel-location-tag location-info) "ApproximateLocation")
             (setq location-info (hcel-approx-to-exact-location location-info)))
-          (let ((module-path (alist-get 'modulePath location-info))
-                (line-beg (alist-get 'startLine location-info))
+          (let ((line-beg (alist-get 'startLine location-info))
                 (col-beg (alist-get 'startColumn location-info))
                 (line-end (alist-get 'endLine location-info))
                 (col-end (alist-get 'endColumn location-info)))
@@ -494,27 +486,29 @@ If NO-JUMP is non-nil, just open the source and does not jump to the location wi
 
 (defvar hcel-minor-mode-map
   (let ((kmap (make-sparse-keymap)))
-    (define-key kmap (kbd "M-?") 'hcel-minor-find-references-at-point)
+    (define-key kmap (kbd "M-?") #'hcel-minor-find-references-at-point)
     kmap))
 
 (define-minor-mode hcel-minor-mode
   "A minor mode for exploring haskell codebases."
   :lighter " hcel-minor"
-  :after-hook
-  (if hcel-minor-mode
-      (if (and (not (memq major-mode hcel-minor-major-modes))
-               (not (eq (current-buffer) eldoc--doc-buffer)))
-          (progn
-            (hcel-minor-mode 0)
-            (error "Not in one of the supported modes (%s) or the eldoc buffer."
-                   (string-join (mapcar 'prin1-to-string hcel-minor-major-modes)
-                                ", ")))
-        (add-hook
-         'eldoc-documentation-functions #'hcel-minor-eldoc-docs nil t)
-        (add-hook
-         'eldoc-documentation-functions #'hcel-minor-eldoc-id-type nil t)
-        (setq-local eldoc-documentation-strategy 'eldoc-documentation-compose))
+  (add-hook 'xref-backend-functions
+            #'hcel-minor--xref-backend nil t)
+  (cond
+   ((null hcel-minor-mode)
     (remove-hook 'eldoc-documentation-functions #'hcel-minor-eldoc-id-type t)
-    (remove-hook 'eldoc-documentation-functions #'hcel-minor-eldoc-docs t)))
+    (remove-hook 'eldoc-documentation-functions #'hcel-minor-eldoc-docs t))
+   ((not (or (memq major-mode hcel-minor-major-modes)
+             (eq (current-buffer) eldoc--doc-buffer)))
+    (setq hcel-minor-mode nil)
+    (error "Not in one of the supported modes (%s) or the eldoc buffer."
+           (mapconcat #'prin1-to-string hcel-minor-major-modes
+                      ", ")))
+   (t
+    (add-hook
+     'eldoc-documentation-functions #'hcel-minor-eldoc-docs nil t)
+    (add-hook
+     'eldoc-documentation-functions #'hcel-minor-eldoc-id-type nil t)
+    (setq-local eldoc-documentation-strategy 'eldoc-documentation-compose))))
 
 (provide 'hcel-source)
diff --git a/hcel-utils.el b/hcel-utils.el
index e5a82e7a1b..1e1afea1eb 100644
--- a/hcel-utils.el
+++ b/hcel-utils.el
@@ -77,6 +77,7 @@ Example of an idSrcSpan:
               (col-end (alist-get 'column (alist-get 'end span))))
     (buffer-substring-line-column line-beg (1- col-beg) line-end (1- col-end))))
 
+;; FIXME: Make sure all your definitions have an `hcel-' prefix!
 ;; buffers and strings manipulation
 (defun goto-line-column (line column)
   (goto-line line)
diff --git a/hc.el b/hcel.el
similarity index 86%
rename from hc.el
rename to hcel.el
index f6239f6dbc..bdfe65cbe6 100644
--- a/hc.el
+++ b/hcel.el
@@ -1,11 +1,11 @@
-;;; hc.el --- Haskell codebase explorer -*- lexical-binding: t; -*-
+;;; hcel.el --- Haskell codebase explorer -*- lexical-binding: t; -*-
 
 ;; Author: Yuchen Pei <id@ypei.org>
 ;; Maintainer: Yuchen Pei <id@ypei.org>
 ;; Created: 2022
 ;; Version: 0
 ;; Keywords: haskell
-;; Package-Requires: ((emacs "28") (haskell-mode))
+;; Package-Requires: ((emacs "28"))
 ;; Package-Type: multi
 ;; Homepage: https://g.ypei.me/hcel.git
 
@@ -36,9 +36,10 @@
   (interactive
    (list
     (completing-read "Select package: "
-                     (mapcar 'hcel-format-package-id
+                     (mapcar #'hcel-format-package-id
                              (hcel-api-packages)))))
-  (call-interactively (hcel-module-selector (hcel-parse-package-id package-id))))
+  (call-interactively
+   (hcel-module-selector (hcel-parse-package-id package-id))))
 
 (defun hcel-module ()
   "Select a module to display source."
@@ -56,3 +57,4 @@
      (hcel-load-module-source package-id module-path))))
 
 (provide 'hc)
+;;; hcel.el ends here.

[-- Attachment #3: hcel.warnings --]
[-- Type: text/plain, Size: 13425 bytes --]

Byte compiling packages/hcel/hcel-client.el
Byte compiling packages/hcel/hcel.el
Byte compiling packages/hcel/hcel-haddorg.el
Byte compiling packages/hcel/hcel-outline.el

In toplevel form:
packages/hcel/hcel-client.el:20:12: Warning: defcustom for ‘hcel-host’ fails to specify type
packages/hcel/hcel-client.el:23:12: Warning: defcustom for ‘hcel-indexed-dir’ fails to specify type

In end of data:
packages/hcel/hcel-client.el:142:8: Warning: the function ‘delete-http-header’ is not known to be defined.
packages/hcel/hcel-client.el:59:19: Warning: the function ‘hcel-location-tag’ is not known to be defined.
packages/hcel/hcel-client.el:47:43: Warning: the function ‘hcel-format-package-id’ is not known to be defined.
Byte compiling packages/hcel/hcel-results.el

In end of data:
packages/hcel/hcel-haddorg.el:27:17: Warning: the function ‘org-entry-get’ is not known to be defined.
packages/hcel/hcel-haddorg.el:26:6: Warning: the function ‘org-back-to-heading’ is not known to be defined.
Byte compiling packages/hcel/hcel-source.el

In end of data:
packages/hcel/hcel-outline.el:125:29: Warning: the function ‘outline-toggle-children’ is not known to be defined.
packages/hcel/hcel-outline.el:58:23: Warning: the function ‘hcel-format-package-id’ is not known to be defined.
packages/hcel/hcel-outline.el:32:29: Warning: the function ‘outline-up-heading’ is not known to be defined.
packages/hcel/hcel-outline.el:31:29: Warning: the function ‘outline-backward-same-level’ is not known to be defined.
packages/hcel/hcel-outline.el:29:29: Warning: the function ‘outline-forward-same-level’ is not known to be defined.
packages/hcel/hcel-outline.el:28:29: Warning: the function ‘outline-previous-visible-heading’ is not known to be defined.
packages/hcel/hcel-outline.el:27:29: Warning: the function ‘outline-next-visible-heading’ is not known to be defined.
Byte compiling packages/hcel/hcel-utils.el

In hcel-module:
packages/hcel/hcel.el:48:26: Warning: reference to free variable ‘hcel-package-id’

In hcel-module-selector:
packages/hcel/hcel.el:55:43: Warning: reference to free variable ‘package-id’

In end of data:
packages/hcel/hcel.el:55:25: Warning: the function ‘hcel-list-modules’ is not known to be defined.

In goto-line-column:
packages/hcel/hcel-utils.el:83:4: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.

In hcel-results-next-page:
packages/hcel/hcel-results.el:79:37: Warning: reference to free variable ‘hcel-results-max-page-number’
packages/hcel/hcel-results.el:101:12: Warning: defcustom for ‘hcel-refs-per-page’ fails to specify type

In hcel-load-module-source:
packages/hcel/hcel-source.el:56:17: Warning: assignment to free variable ‘hcel-declarations’
packages/hcel/hcel-source.el:57:17: Warning: assignment to free variable ‘hcel-identifiers’
packages/hcel/hcel-source.el:58:17: Warning: assignment to free variable ‘hcel-occurrences’
packages/hcel/hcel-source.el:59:17: Warning: assignment to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:60:17: Warning: assignment to free variable ‘hcel-module-path’

In hcel-reload-module-source:
packages/hcel/hcel-source.el:69:33: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:69:49: Warning: reference to free variable ‘hcel-module-path’

In hcel-load-module-location-info:
packages/hcel/hcel-source.el:73:2: Warning: docstring wider than 80 characters

In end of data:
packages/hcel/hcel-utils.el:109:8: Warning: the function ‘haskell-mode’ is not known to be defined.
packages/hcel/hcel-utils.el:49:15: Warning: the function ‘hcel-definition-site-location-info’ is not known to be defined.

In hcel-refs-update-references:
packages/hcel/hcel-results.el:126:11: Warning: reference to free variable ‘hcel-refs-package-id’
packages/hcel/hcel-results.el:126:32: Warning: reference to free variable ‘hcel-refs-id’
packages/hcel/hcel-results.el:133:46: Warning: reference to free variable ‘hcel-results-max-page-number’

In hcel-refs-update-references-package:
packages/hcel/hcel-results.el:187:51: Warning: reference to free variable ‘hcel-refs-id’
packages/hcel/hcel-results.el:202:11: Warning: assignment to free variable ‘hcel-refs-package-id’
packages/hcel/hcel-results.el:204:11: Warning: assignment to free variable ‘hcel-results-max-page-number’

In hcel-find-references-at-point:
packages/hcel/hcel-results.el:211:34: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-results.el:211:50: Warning: reference to free variable ‘hcel-module-path’
packages/hcel/hcel-results.el:213:13: Warning: reference to free variable ‘hcel-mode-map’

In hcel-find-references-internal:
packages/hcel/hcel-results.el:240:57: Warning: reference to free variable ‘hcel-identifiers’
packages/hcel/hcel-results.el:244:19: Warning: assignment to free variable ‘hcel-refs-id’
packages/hcel/hcel-results.el:248:12: Warning: defcustom for ‘hcel-ids-per-page’ fails to specify type
packages/hcel/hcel-results.el:251:12: Warning: defcustom for ‘hcel-ids-live-per-page’ fails to specify type

In hcel-ids-update:
packages/hcel/hcel-results.el:267:18: Warning: reference to free variable ‘hcel-ids-scope’
packages/hcel/hcel-results.el:267:48: Warning: reference to free variable ‘hcel-ids-package-id’
packages/hcel/hcel-results.el:272:27: Warning: reference to free variable ‘hcel-ids-query’
packages/hcel/hcel-results.el:277:11: Warning: assignment to free variable ‘hcel-results-max-page-number’
packages/hcel/hcel-results.el:284:21: Warning: reference to free variable ‘hcel-results-max-page-number’

In hcel-ids-update-query:
packages/hcel/hcel-results.el:326:47: Warning: reference to free variable ‘hcel-ids-query’
packages/hcel/hcel-results.el:327:9: Warning: assignment to free variable ‘hcel-ids-query’

In hcel-global-ids-minibuffer-collection:
packages/hcel/hcel-results.el:357:60: Warning: repeated variable unused in lambda-list
packages/hcel/hcel-results.el:357:53: Warning: Unused lexical argument `unused'
packages/hcel/hcel-results.el:357:60: Warning: Unused lexical argument `unused'

In hcel-package-ids-minibuffer-collection:
packages/hcel/hcel-results.el:361:25: Warning: repeated variable unused in lambda-list
packages/hcel/hcel-results.el:361:18: Warning: Unused lexical argument `unused'
packages/hcel/hcel-results.el:361:25: Warning: Unused lexical argument `unused'

In hcel-ids:
packages/hcel/hcel-results.el:373:15: Warning: assignment to free variable ‘hcel-ids-scope’
packages/hcel/hcel-results.el:374:15: Warning: assignment to free variable ‘hcel-ids-package-id’

In hcel-package-ids:
packages/hcel/hcel-results.el:389:35: Warning: reference to free variable ‘hcel-package-id’

In end of data:
packages/hcel/hcel-results.el:292:39: Warning: the function ‘hcel-definition-site-location-info’ is not known to be defined.
packages/hcel/hcel-results.el:271:12: Warning: the function ‘hcel-api-identifiers’ is not known to be defined.
packages/hcel/hcel-results.el:262:33: Warning: the function ‘hcel-minor-mode’ is not known to be defined.
packages/hcel/hcel-results.el:235:23: Warning: the function ‘hcel-load-module-source’ is not known to be defined.
packages/hcel/hcel-results.el:232:25: Warning: the function ‘hcel-buffer-name’ is not known to be defined.
packages/hcel/hcel-results.el:187:24: Warning: the function ‘hcel-api-global-references’ is not known to be defined.
packages/hcel/hcel-results.el:125:11: Warning: the function ‘hcel-api-references’ is not known to be defined.
packages/hcel/hcel-results.el:70:4: Warning: the function ‘hcel-load-module-location-info’ is not known to be defined.
packages/hcel/hcel-results.el:69:4: Warning: the function ‘compilation-set-overlay-arrow’ might not be defined at runtime.

In hcel-lookup-occurrence-at-point:
packages/hcel/hcel-source.el:128:36: Warning: reference to free variable ‘hcel-occurrences’

In hcel-type-at-point:
packages/hcel/hcel-source.el:145:30: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:145:46: Warning: reference to free variable ‘hcel-module-path’

In hcel-render-type-internal:
packages/hcel/hcel-source.el:153:59: Warning: reference to free variable ‘hcel-identifiers’

In hcel-id-docs-at-point:
packages/hcel/hcel-source.el:174:26: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:174:42: Warning: reference to free variable ‘hcel-module-path’

In hcel-id-docs-internal:
packages/hcel/hcel-source.el:183:51: Warning: reference to free variable ‘hcel-identifiers’

In hcel-expressions-type:
packages/hcel/hcel-source.el:215:45: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:216:45: Warning: reference to free variable ‘hcel-module-path’

In hcel-outline-package-module:
packages/hcel/hcel-source.el:223:21: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:224:22: Warning: reference to free variable ‘hcel-module-path’

In hcel-eldoc-id-type:
packages/hcel/hcel-source.el:239:28: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:240:29: Warning: reference to free variable ‘hcel-module-path’

In hcel-highlight-update:
packages/hcel/hcel-source.el:324:27: Warning: reference to free variable ‘hcel-highlight-id’
packages/hcel/hcel-source.el:327:15: Warning: assignment to free variable ‘hcel-highlight-id’

In hcel-imenu-create-index:
packages/hcel/hcel-source.el:412:15: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
packages/hcel/hcel-source.el:413:4: Warning: reference to free variable ‘hcel-declarations’

In hcel-find-definition:
packages/hcel/hcel-source.el:423:4: Warning: reference to free variable ‘hcel-package-id’
packages/hcel/hcel-source.el:423:20: Warning: reference to free variable ‘hcel-module-path’

In hcel-find-definition-internal:
packages/hcel/hcel-source.el:457:50: Warning: reference to free variable ‘hcel-identifiers’
packages/hcel/hcel-source.el:459:50: Warning: reference to free variable ‘hcel-occurrences’

In end of data:
packages/hcel/hcel-source.el:489:36: Warning: the function ‘hcel-minor-find-references-at-point’ is not known to be defined.
packages/hcel/hcel-source.el:477:30: Warning: the function ‘xref-make-buffer-location’ is not known to be defined.
packages/hcel/hcel-source.el:475:29: Warning: the function ‘xref-make-match’ is not known to be defined.
packages/hcel/hcel-source.el:461:34: Warning: the function ‘hcel-approx-to-exact-location’ is not known to be defined.
packages/hcel/hcel-source.el:408:8: Warning: the function ‘hcel-render-components’ is not known to be defined.
packages/hcel/hcel-source.el:397:6: Warning: the function ‘dom-by-class’ is not known to be defined.
packages/hcel/hcel-source.el:386:5: Warning: the function ‘dom-by-tag’ is not known to be defined.
packages/hcel/hcel-source.el:379:23: Warning: the function ‘dom-text’ is not known to be defined.
packages/hcel/hcel-source.el:377:18: Warning: the function ‘dom-attr’ is not known to be defined.
packages/hcel/hcel-source.el:252:29: Warning: the function ‘text-property-search-backward’ is not known to be defined.
packages/hcel/hcel-source.el:229:6: Warning: the function ‘hcel-outline-load-identifiers-at-point’ is not known to be defined.
packages/hcel/hcel-source.el:228:6: Warning: the function ‘hcel-outline-goto-module’ is not known to be defined.
packages/hcel/hcel-source.el:227:6: Warning: the function ‘hcel-outline-load-modules-at-point’ is not known to be defined.
packages/hcel/hcel-source.el:226:6: Warning: the function ‘hcel-outline-goto-package’ is not known to be defined.
packages/hcel/hcel-source.el:225:6: Warning: the function ‘hcel’ is not known to be defined.
packages/hcel/hcel-source.el:219:14: Warning: the function ‘hcel-expression-and-type’ is not known to be defined.
packages/hcel/hcel-source.el:208:25: Warning: the function ‘current-line’ is not known to be defined.
packages/hcel/hcel-source.el:199:14: Warning: the function ‘hcel-render-html’ is not known to be defined.
packages/hcel/hcel-source.el:159:15: Warning: the function ‘hcel-render-id-type’ is not known to be defined.
packages/hcel/hcel-source.el:141:6: Warning: the function ‘buffer-substring-line-column’ is not known to be defined.
packages/hcel/hcel-source.el:136:21: Warning: the function ‘hcel-text-property-near-point’ is not known to be defined.
packages/hcel/hcel-source.el:105:8: Warning: the function ‘goto-line-column’ is not known to be defined.
packages/hcel/hcel-source.el:94:21: Warning: the function ‘hcel-location-tag’ is not known to be defined.
packages/hcel/hcel-source.el:52:12: Warning: the function ‘fontify-with-haskell-mode’ is not known to be defined.
packages/hcel/hcel-source.el:38:21: Warning: the function ‘hcel-format-package-id’ is not known to be defined.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-20 21:42 ` Stefan Monnier
@ 2022-09-21  5:58   ` Yuchen Pei
  2022-09-21  6:21     ` Stefan Kangas
  2022-09-21  7:19     ` Michael Heerdegen
  0 siblings, 2 replies; 11+ messages in thread
From: Yuchen Pei @ 2022-09-21  5:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Emacs Devel mailing list

On Tue 2022-09-20 17:42:43 -0400, Stefan Monnier wrote:

> BTW, here's a patch with various cosmetic changes.

Thanks for the patch - it is very educational.  I have applied it and
made many other changes in a similar fashion.

A few remaining questions:

>  (defun hcel-results-next-page ()
>    (interactive)
> +  ;; FIXME: Using `major-mode' is a code smell.
>    (unless (memq major-mode '(hcel-refs-mode hcel-ids-mode))
>      (error "Not in hcel-refs or hcel-ids mode: %S" major-mode))

What do you mean, should I avoid using the variable `major-mode'? Why?

> In end of data:
> packages/hcel/hcel-client.el:142:8: Warning: the function
> ‘delete-http-header’ is not known to be defined.
> packages/hcel/hcel-client.el:59:19: Warning: the function
> ‘hcel-location-tag’ is not known to be defined.
> packages/hcel/hcel-client.el:47:43: Warning: the function
> ‘hcel-format-package-id’ is not known to be defined.
> Byte compiling packages/hcel/hcel-results.el

How do I get these "end of data" warnings?  When I do M-x byte-compile
or byte-recompile-directory I don't get them.

>
> In hcel-module-selector:
> packages/hcel/hcel.el:55:43: Warning: reference to free variable
> ‘package-id’

I thought this was OK because of lexical-binding?  For reference, here's
the defun:

(defun hcel-module-selector (package-id)
  (lambda (module-path)
    (interactive
     (list
      (completing-read "Select module: "
                       (hcel-list-modules package-id))))
    (switch-to-buffer
     (hcel-load-module-source package-id module-path))))

It seems to me to be similar to the problem described in
<https://lists.gnu.org/archive/html/help-gnu-emacs/2012-08/msg00296.html>.

Best,
Yuchen

-- 
PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
          <https://ypei.org/assets/ypei-pubkey.txt>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-20 21:26 ` [ELPA] New package: hcel Stefan Monnier
@ 2022-09-21  6:18   ` Yuchen Pei
  2022-09-21 12:03     ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Yuchen Pei @ 2022-09-21  6:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Emacs Devel mailing list

Hi Stefan,

Thanks for your reply.
On Tue 2022-09-20 17:26:34 -0400, Stefan Monnier wrote:

> Hi Yuchen, sorry I didn't get back to you earlier,
>
>> It is a Haskell codebase explorer, using the server program provided by
>> haskell-code-explorer[2], which I ported to recent GHC version[3] and
>> I named my port / fork hcel.
>>
>> [1] https://g.ypei.me/hcel.git/tree/lisp
>> [2] https://github.com/alexwl/haskell-code-explorer
>> [3] https://g.ypei.me/hcel.git/about/
>>
>> I'm not sure whether it is unusual to submit a package that relies on a
>> self-hosted free software server program,
>
> Indeed, in many cases such modes start their life alongside the
> associated tool, but it can be nicer for users if it's available in
> GNU ELPA.

One solution is to add downloading and installing the server program
hcel to the package install process.  But I'm not sure if this is a good
idea because 1) the server program is written in Haskell not elisp and I
don't hold the copyright and 2) anyone could set up a public instance
(like the reference instance by the maintainer of the original
haskell-code-explorer) for others to use.

I think the situation is similar to say a mediawiki client that talks to
any wiki including Wikipedia, or an emacs version of h-client[1] that talks
to any h-source[2] servers, including the "official" instance
h-node.org.

[1] https://savannah.nongnu.org/projects/h-client
[2] https://savannah.nongnu.org/projects/h-source

>
> Often part of the issue is co-evolution (e.g. does it make sense to use
> `hcel.el` from last year with the current `hcel` tool and vice versa).

Right.  hc.el the emacs client talks to hcel the server using http
requests.  If the server API ever changes, hc.el could check the API
version in an API response against its supported versions and throw an
error for incompatibility.  I have not implemented that yet because the
API has not changed so far.

>
> IIUC the code you suggest we add to GNU ELPA is the one at
> https://g.ypei.me/hc.el.git, right?

Yes, I separated it out of the server program.

>
> Given the fact that this tool has a fairly narrow focus, I must admit
> that I'd prefer it doesn't eat up a two-letter file name like `hc`.
> Any chance you can rename that main file to `hcel.el`?

I applied your patch that does so.

>
> Also, I see that your package has:
>
>     ;; Package-Requires: ((emacs "28") (haskell-mode))
>
> but `haskell-mode` is currently neither in GNU ELPA nor in NonGNU ELPA
> so we can't include it in neither of them.  Is the dependency
> unavoidable (so we should first include `haskell-mode` in (Non)GNU
> ELPA), or can it be removed (and replaced with a runtime test, possibly
> allowing the use of other haskell modes such as haskell-tng-mode, which
> *is* in NonGNU ELPA)?

It seems to me haskell-mode is in NonGNU ELPA[3]?  The mode is used for
piggybacking their font-locking for the code syntax highlight.  There's
probably sufficient info in the server response for hc.el to apply its
own syntax highlighting, but it has not been a priority to implement
this functionality.  Is this needed for inclusion in ELPA?

[3] https://elpa.nongnu.org/nongnu/

Best,
Yuchen

-- 
PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
          <https://ypei.org/assets/ypei-pubkey.txt>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-21  5:58   ` Yuchen Pei
@ 2022-09-21  6:21     ` Stefan Kangas
  2022-09-21  7:19     ` Michael Heerdegen
  1 sibling, 0 replies; 11+ messages in thread
From: Stefan Kangas @ 2022-09-21  6:21 UTC (permalink / raw)
  To: Yuchen Pei, Stefan Monnier; +Cc: Emacs Devel mailing list

Yuchen Pei <id@ypei.org> writes:

> What do you mean, should I avoid using the variable `major-mode'? Why?

Use `derived-mode-p' instead.

> How do I get these "end of data" warnings?  When I do M-x byte-compile
> or byte-recompile-directory I don't get them.

Try compiling with "emacs -Q".



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-21  5:58   ` Yuchen Pei
  2022-09-21  6:21     ` Stefan Kangas
@ 2022-09-21  7:19     ` Michael Heerdegen
  2022-09-21  7:44       ` Emanuel Berg
  1 sibling, 1 reply; 11+ messages in thread
From: Michael Heerdegen @ 2022-09-21  7:19 UTC (permalink / raw)
  To: emacs-devel

Yuchen Pei <id@ypei.org> writes:

> (defun hcel-module-selector (package-id)
>   (lambda (module-path)
>     (interactive
>      (list
>       (completing-read "Select module: "
>                        (hcel-list-modules package-id))))
>     (switch-to-buffer
>      (hcel-load-module-source package-id module-path))))
>
> It seems to me to be similar to the problem described in
> <https://lists.gnu.org/archive/html/help-gnu-emacs/2012-08/msg00296.html>.

Hmm - no, I think the problem is the reference in the interactive form -
AFAIK, variables in interactive forms can't refer to the lexical
environment.  See Bug#51695.  I don't recall a nice solution for this
kind of problem (anyone?).

Michael.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-21  7:19     ` Michael Heerdegen
@ 2022-09-21  7:44       ` Emanuel Berg
  0 siblings, 0 replies; 11+ messages in thread
From: Emanuel Berg @ 2022-09-21  7:44 UTC (permalink / raw)
  To: emacs-devel

Michael Heerdegen wrote:

> Hmm - no, I think the problem is the reference in the
> interactive form - AFAIK, variables in interactive forms
> can't refer to the lexical environment. See Bug#51695.
> I don't recall a nice solution for this kind of problem
> (anyone?).

Yours was good (get the spec from a function) but it is still
a workaround.

;;; -*- lexical-binding: t -*-
;;
;; this file:
;;   https://dataswamp.org/~incal/emacs-init/w3m/w3m-survivor.el

(require 'w3m-search)
(require 'cl-lib)

(let ((opts   "torrent magnet 720p")
      (show   "Survivor")
      (prompt "episode: ") )
  (let ((next 1))
    (defun australian-survivor-interactive ()
      (list (read-number prompt next)) )
    (declare-function australian-survivor-interactive nil)

    (defun australian-survivor (ep)
      (interactive (australian-survivor-interactive))
      (w3m-search w3m-search-default-engine
        (format "\"s10e%02d\" Australian %s %s" ep show opts) )
      (setq next (1+ ep)) )
    (declare-function australian-survivor nil) )
  (defalias 'aus #'australian-survivor)

  (let ((next 1))
    (defun us-survivor-interactive ()
      (list (read-number prompt next)) )
    (declare-function us-survivor-interactive nil)
    (defalias 'us #'us-survivor-interactive)

    (defun us-survivor (ep)
      (interactive (us-survivor-interactive))
      (w3m-search w3m-search-default-engine
        (format "\"s43e%02d\" %s %s" ep show opts) )
      (setq next (1+ ep)) )
    (declare-function us-survivor nil)
    (defalias 'usa #'us-survivor) ))

-- 
underground experts united
https://dataswamp.org/~incal




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [ELPA] New package: hcel
  2022-09-21  6:18   ` Yuchen Pei
@ 2022-09-21 12:03     ` Stefan Monnier
  0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier @ 2022-09-21 12:03 UTC (permalink / raw)
  To: Yuchen Pei; +Cc: Emacs Devel mailing list

>> Given the fact that this tool has a fairly narrow focus, I must admit
>> that I'd prefer it doesn't eat up a two-letter file name like `hc`.
>> Any chance you can rename that main file to `hcel.el`?
>
> I applied your patch that does so.

Perfect, thanks.

>> Also, I see that your package has:
>>
>>     ;; Package-Requires: ((emacs "28") (haskell-mode))
>>
>> but `haskell-mode` is currently neither in GNU ELPA nor in NonGNU ELPA
>> so we can't include it in neither of them.  Is the dependency
>> unavoidable (so we should first include `haskell-mode` in (Non)GNU
>> ELPA), or can it be removed (and replaced with a runtime test, possibly
>> allowing the use of other haskell modes such as haskell-tng-mode, which
>> *is* in NonGNU ELPA)?
>
> It seems to me haskell-mode is in NonGNU ELPA[3]?

Oh, right, sorry.  It's currently not being tracked (see
https://github.com/haskell/haskell-mode/issues/1755), but it's still in
NonGNU ELPA, yes.

Sorry 'bout the noise, the dependency on `haskell-mode` is fine.

>>  (defun hcel-results-next-page ()
>>    (interactive)
>> +  ;; FIXME: Using `major-mode' is a code smell.
>>    (unless (memq major-mode '(hcel-refs-mode hcel-ids-mode))
>>      (error "Not in hcel-refs or hcel-ids mode: %S" major-mode))
>
> What do you mean, should I avoid using the variable `major-mode'? Why?

Experience over the years taught me that.  It's a bit like explicit type
tests in OO languages, I think.  Stefan Kangas's suggestion to use
`derived-mode-p` is a bit better, but it's still not great.
It's often better to "do something else".  Depending on the situation
the "something else" can be quite varied, but one common option is to
have the major mode set a buffer-local variable that gives the
needed info.

>> In end of data:
>> packages/hcel/hcel-client.el:142:8: Warning: the function
>> ‘delete-http-header’ is not known to be defined.
>> packages/hcel/hcel-client.el:59:19: Warning: the function
>> ‘hcel-location-tag’ is not known to be defined.
>> packages/hcel/hcel-client.el:47:43: Warning: the function
>> ‘hcel-format-package-id’ is not known to be defined.
>> Byte compiling packages/hcel/hcel-results.el
>
> How do I get these "end of data" warnings?  When I do M-x byte-compile
> or byte-recompile-directory I don't get them.

When you `M-x byte-compile`, the compilation is done in the running Emacs,
which already has lot of packages loaded an thus won't be able to notice
if your code relies on those packages without making sure they'll be
loaded before using them.

The above warnings come from a "clean compilation" where every file is
compiled in a separate dedicated Emacs session.

>> In hcel-module-selector:
>> packages/hcel/hcel.el:55:43: Warning: reference to free variable
>> ‘package-id’
> I thought this was OK because of lexical-binding?  For reference, here's
> the defun:

As Micheal points out, this is Bug#51695 :-(
And indeed there is no nice solution for it currently.

In Emacs-29, there is a (cumbersome) solution which involves the use of
an OClosure.  I'm hoping to find the time before Emacs-29.1 to write the
code so the bytecompiler can use the OClosure behind the scenes for you.


        Stefan




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-09-21 12:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-10 15:01 [ELPA] New package: hcel Yuchen Pei
2022-09-17  0:40 ` Yuchen Pei
2022-09-19  0:15   ` How Do I submit a package to ELPA? (was Re: [ELPA] New package: hcel) Yuchen Pei
2022-09-20 21:26 ` [ELPA] New package: hcel Stefan Monnier
2022-09-21  6:18   ` Yuchen Pei
2022-09-21 12:03     ` Stefan Monnier
2022-09-20 21:42 ` Stefan Monnier
2022-09-21  5:58   ` Yuchen Pei
2022-09-21  6:21     ` Stefan Kangas
2022-09-21  7:19     ` Michael Heerdegen
2022-09-21  7:44       ` Emanuel Berg

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).